-
-
Notifications
You must be signed in to change notification settings - Fork 108
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
various fixes in the prepare/init_transforms
system
#360
Conversation
!transform_to_position
prepare/init_transforms
system
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.
Thanks, looks good overall! I really like having this properly tested.
I left some small code quality suggestions, and fixes for the CI errors (mainly wrong precision for math types, ugh). The lint error seems unrelated, so you can probably ignore that.
Should we remove the comment |
Yeah, we can probably remove it. The system still isn't exactly pretty, but it's at least better and properly tested. I think we might be able to make this and other component initialization logic a bit nicer once Bevy gets "required components" (a way to automatically insert components based on dependencies to other components, Cart is working on this), assuming it allows ECS world access. But we can experiment with that when they're actually released |
All should be resolved, thx! Lmk if I can auto-squash the fixups |
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.
Thank you, looks good now! I generally tend to do squash merges like Bevy itself, so there's no need for you to squash things :)
(also I fixed the lint errors on the main branch so CI should now fully pass)
Objective
The
prepare/init_transforms
system has multiple issues:!config.transform_to_position
from loop so only the first item of the query is being processedis_rb: Has<C>
is redundant in the presence ofAdded<C>
is_rb: Has<C>
tois_rigid_body: Has<RigidBody>
Position
and/orRotation
andconfig.transform_to_position
but noTransform
, no newTransform
is addedPosition
and/orRotation
andconfig.transform_to_position
thenew_position
/new_rotation
ignore the current transform and only take the global transformSolution
Added a test to cover this system and fixed the found issues. Also refactored out some of the deep nesting in the implementation. Note that the test does not yet cover
GlobalTransform
s andparents
transforms.Changelog
Fixed
Fixed various issues in the
prepare/init_transforms
system.