Add mutated state when inserting an already existing component#404
Add mutated state when inserting an already existing component#404cart merged 4 commits intobevyengine:masterfrom dallenng:put_dynamic_mutated
Conversation
|
Isn't this behavior already captured by the |
|
Here's a little diagram of what's happening (assuming the commands are run sequentially on the same entity and the trackers are cleared between each command) :
|
|
As far as I can see, I fixed change tracking on |
| let state = self.state.get_mut(&ty).unwrap(); | ||
| if added { | ||
| state.added_entities[index] = true; | ||
| } else { |
There was a problem hiding this comment.
Hey sorry for the delay here. These changes make sense to me. I think this works as-is, but I'm a little worried that the implicit !added == mutated here could cause problems / create corner case issues. I'd rather just pass in a mutated flag and set it explicitly each time.
There was a problem hiding this comment.
Im specifically worried about cases where we change archetypes and some of the components aren't mutated.
There was a problem hiding this comment.
Ok, I did the changes you suggested and I think it's better like this because further call of the method will need to be explicit on the state.
Rename some variables and methods names to avoid confusion between Changed and Mutated query filter.
|
Sorry for dragging my feat on this one. This looks good to go. Thanks for sticking with this / improving our correctness! |
Fixes #333
Proposed Change
When calling
Archetype::put_dynamicwithadded = false, the mutated state will be set to true. Because if a component is not added, that means it is updated so it should be marked as mutated.I added tests to check if the behaviour is correct based on what I think the code should do, feel free to correct me if I'm wrong.
One case is not handled correctly in this PR, but I'll need a bit of guidance if this behaviour is wanted. When the entity contains A and you insert A and B, A is updated and B is added. So, A state should be mutated and B state should be added, but both state are added.