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

fix: Avoid reading eased entities mutably, until there is an intention to mutate them #31

Merged
merged 6 commits into from
Feb 18, 2024

Conversation

evenius
Copy link
Contributor

@evenius evenius commented Feb 17, 2024

Hello!

So this began with a short problem I had where any eased Component had it's Changed<T> triggered on every Update.

This PR kind of does two things, and there's also an example which helped me debug and implement these changes.

1. Only trigger Changed<T>/read mutably when necessary.

This was the main issue I had, and the change is splitting the query into two:entity_query: Query<Entity, With<T>> and mut object_query: Query<&mut T>.
Get entity early, but hold off on fetching the object with get_mut() until needed, since calling get_mut() triggers the bevy_change detection (whether you change the object or not).

2. Don't evaluate animations if state is not EasingState::Play

This became part of this PR, after noticing that if the animation state is paused, it will keep recalculating the animation state (and triggering Changed<T>) forever. I don't see any reason why we need to redo any of the logic there, since we can more or less be guaranteed that it will keep being the same... Forever? Right?

3. The controlled.rs example

This is just a file that I used to make sure my changes works as intended, and I feel it's maybe too complex to really be an example, and would be happy to remove/simplify it if needed.

Screenshot 2024-02-17 at 15 59 15

Picture of the example app in question

@mockersf
Copy link
Member

thanks!

@mockersf mockersf merged commit 29489dd into vleue:main Feb 18, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants