Skip to content
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

Simplify transform_to_position and fix rotation denormalization #620

Merged
merged 3 commits into from
Jan 13, 2025

Conversation

Jondolf
Copy link
Owner

@Jondolf Jondolf commented Jan 10, 2025

Objective

Fixes #573 (hopefully)
Fixes #618

The transform_to_position system 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 PreviousGlobalTransform and GlobalTransform, with several seemingly unnecessary extra steps, instead of simply setting Position and Rotation to match GlobalTransform.

Solution

Significantly simplify transform_to_position, and simply set Position and Rotation to match GlobalTransform.

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!

@Jondolf Jondolf added C-Bug Something isn't working A-Transform Relates to transforms or physics positions labels Jan 10, 2025
@matjlars
Copy link

I tested this branch with my game and it works! Thanks again!!

@68317fa2
Copy link

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!

@Jondolf Jondolf merged commit 2540add into main Jan 13, 2025
5 checks passed
@Jondolf Jondolf deleted the fix-transform-to-position branch January 19, 2025 15:19
@coreh
Copy link

coreh commented Feb 4, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Transform Relates to transforms or physics positions C-Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

non-normalized rotations Error: The vector given to Dir3::new_unchecked is not normalized
4 participants