Added/Changed detection #30
Conversation
bzm3r
force-pushed
the
added-changed
branch
2 times, most recently
from
87030ea to
b7bad05
Compare
| let (column, meta) = unsafe { zip_exact(&mut self.components, &mut self.meta) } | ||
| .find_map(|(&component, meta)| self.table.get_column(component).zip(Some(meta)))?; | ||
|
|
||
| // SAFETY: we know that the table row is a valid index??? |
There was a problem hiding this comment.
To be more explicit about this invariant, it would be good to add a safety comment to the self.table_row field. Something like // SAFETY: must correspond to a valid row in `self.table` .
| .byte_add(self.table_row * meta.size_bytes); | ||
| meta.dyn_ctor.cast(ptr) | ||
| }) | ||
| .or_else(|| self.next()) |
There was a problem hiding this comment.
We should use loops instead of recursion here, though this PR is already really big so we can leave this change for a followup if you prefer.
| // SAFETY ISSUE! SAFETY ISSUE! SAFETY ISSUE! Unlike in the write case, we do not have | ||
| // exclusive access! We have shared access to the entire column and ticks? This might be | ||
| // okay though because there cannot be any other accesses with write access at the same | ||
| // time? |
There was a problem hiding this comment.
This is correct. AddedReadTraits registers read-access, so the scheduler prevents write-access from occurring. Same deal in the other places you have this comment.
| /// [`WorldQuery`] adapter that fetches entities with exactly one component implementing a trait, | ||
| /// with the condition that the component was newly added since the last tick | ||
| pub struct AddedOne<T>(pub T); |
There was a problem hiding this comment.
Just making sure, this essentially a shorthand for Query<One<&dyn Trait>, AddedOneFilter<dyn Trait>>, right?
|
I am okay with that :) |
|
@alice-i-cecile are you still planning to tackle this? |
|
No: I'm not currently using bevy-trait-query in any of my projects. |
|
Thanks for letting me know. Then I will try to adopt this. |
|
A lot of things changed in Bevy related to change detection: |
|
This was rebased later on, and the comments here will be addressed through review there: #42 |
Implements
Added/Changedfunctionality for bevy-trait-query!Closes: #23
What is needed to for this to go out of draft status:
add more change detection tests
update documentation to show how change detection should be used