Fix/Add tracked locations rotating on subcrafts #728
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DerToaster98
changed the title
(cherry picked from commit 353711b)
(cherry picked from commit a320241)
(cherry picked from commit 65e1209)
Old logic rotated the vector around the wrong point. Its safer to recalculate the new absolute location and calculate the offset from that (cherry picked from commit 771094b)
…ons around the rotation point, REGARDLESS of if they are part of the subcraft or not (cherry picked from commit 7e55636)
(cherry picked from commit 501dcd3)
…crafts such as squadrons (cherry picked from commit 289ddbb)
Tested: Calculation wise, the correct result is put out, but either the debugger is inaccurate or something else is happening... (cherry picked from commit 5085cbf)
(cherry picked from commit 166438c)
(cherry picked from commit d45567e)
(cherry picked from commit 436a2e5)
it does not need to know the craft. Also just moving around the cached coordinates is more reliable as it does not rely on a changing center of the craft (cherry picked from commit 8fe3979)
(cherry picked from commit ab9322b)
(cherry picked from commit 2792c48)
TylerS1066
requested changes
Jan 26, 2025
Movecraft/src/main/java/net/countercraft/movecraft/async/rotation/RotationTask.java
Outdated
Show resolved
Hide resolved
Movecraft/src/main/java/net/countercraft/movecraft/async/translation/TranslationTask.java
Outdated
Show resolved
Hide resolved
api/src/main/java/net/countercraft/movecraft/TrackedLocation.java
Outdated
Show resolved
Hide resolved
TylerS1066
reviewed
Jan 27, 2025
|
|
||
| // Java disallows private or protected fields in interfaces, this is a workaround | ||
| class Hidden { | ||
| // Concurrent so we don't have problems when accessing async (useful for addon plugins that want to do stuff async, for example NPC crafts with complex off-thread pathfinding) | ||
| protected static final Map<UUID, Craft> uuidToCraft = Collections.synchronizedMap(new WeakHashMap<>()); | ||
| } | ||
|
|
||
| public class CraftOrigin { |
There was a problem hiding this comment.
Why not use a MovecraftLocation?
There was a problem hiding this comment.
Cause thats is immutable
Comment on lines
11
to
13
| private int dx; | ||
| private int dy; | ||
| private int dz; |
There was a problem hiding this comment.
Why not use a MovecraftLocation?
There was a problem hiding this comment.
Cause thats is immutable
There was a problem hiding this comment.
And it adds additional overhead i dont need
Movecraft/src/main/java/net/countercraft/movecraft/listener/CraftPilotListener.java
Outdated
Show resolved
Hide resolved
Not necessary to use ints here as it is just a vector
Untested, but should work
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe in detail what your pull request accomplishes
Adds proper support for subcrafts to tracked locations. Transfers them to the subcraft and back to the parent on release. The object stays the same so things that use the TrackedLocation can continue to use the same old object and that that updates accordingly.
Also fixes trackedlocations being wrongly rotated on subcrafts in general (the rotation code present does not rotate it correctly, it is necessary to recalculate the new absolute and to adjust the offset via that)
Also re-works trackedlocations so that they basically "move around" a cached value. This is necessary since the old approach used the craft's midpoint as reference location. But since that midpoint can and will change (for example via subcrafts or gradual destruction or launching a torpedo for example) it cant be used as a reference location as that will also move the effective location since the internally stored vector always stays the same
Checklist