-
-
Notifications
You must be signed in to change notification settings - Fork 81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rewrite/rotation task #683
Conversation
Neat necessary restructuring of this task. I think it would also be beneficial to move the subcraft specific code into it's own method too, makes it cleaner. Also you might want to look at your condition for the continue in rotateEntitiesOnCraft() again, probably you are just missing a negation to the contains check. Like it is now you won't move any tnt entity if ONLY_MOVE_PLAYERS is set, iirc previously movecraft always moved tnt entities and player entities regardless of that setting. Might also be worth it changing that hardcoded list to a entity tag or a whitelist in movecraft's main config. |
Double checking that part of the code, btw this is a rewrite to make it more understandable for now, if needed we can make another PR once the rewrite is done. Also to be fair that wasn't even a list to begin with, I just made it look like one xD |
Now that you mention a separate PR: I have plans to abstract the rotation task more to also add things like a Roll task (mainly for Subcraft Roll to make things like a garage door). For the list, you might want to make that as a constant here to avoid unnecessary object creation. Same thing for the cruise task btw, cruise direction could be changed to a vector that defines the direction and the cruise step length. But that also belongs in a separate PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I appreciate the work and improvement to the codebase this PR (and it's twin) brings, I would like to avoid PRs that refactor classes without any performance or feature improvements (especially for classes that will need to be rewritten in the near future).
Describe in detail what your pull request accomplishes
Refactoring some methods and moved/added some methods to CruiseDirection.java.
Checklist