Simplify transform_to_position and fix rotation denormalization#620
Merged
Conversation
This was referenced Jan 10, 2025
|
I tested this branch with my game and it works! Thanks again!! |
|
I tested it too and can confirm this with a different example, where the panic, especially the whole denormalization stops after the different branch. However, I used the dynamic character example with extended mouse movement and could imagine, that this could also be somehow related to bevyengine/bevy#16480 (comment). I also want to thank you very much @Jondolf for your time and for generally maintaining this project! |
|
Hey, sorry for taking this long to respond! Just revered my workaround, and updated to the latest version, and I am getting no more crashes, or the weird rotation behavior, so I believe this is indeed fixed. |
This file contains hidden or 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
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.
Objective
Fixes #573 (hopefully)
Fixes #618
The
transform_to_positionsystem is currently doing some rather complicated computations that appear to be prone to causing error and rotation denormalization.It is somewhat difficult to understand what exactly the system is even doing (it has been a long time since I wrote it), but it essentially seems to apply the change between
PreviousGlobalTransformandGlobalTransform, with several seemingly unnecessary extra steps, instead of simply settingPositionandRotationto matchGlobalTransform.Solution
Significantly simplify
transform_to_position, and simply setPositionandRotationto matchGlobalTransform.In my testing, this fixes the denormalization issue in #618, and I suspect that it may have also helped with #573. Confirmation from @coreh or someone else struggling with the runtime panic would be appreciated!
Additional Information
I'm not entirely sure why I originally wrote this the way I did. This commit implies that it was done to fix some problem with change detection (maybe for sleeping?) but the version in this PR seems to work perfectly fine based on my brief testing.
I haven't noticed this breaking anything else, and I don't see why it would have, but if someone does notice regressions, please let me know!