Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Reflexive world queries #68
base: main
Are you sure you want to change the base?
Reflexive world queries #68
Changes from 22 commits
bd353a5
f23f025
0def33b
7243490
dcbf1a8
c402362
496c810
6083c1a
7e9a60a
2b0d37b
4449c68
e7780b4
5cb49fb
b34d4c9
1a97203
534dba0
d7370df
0f51714
f570b1d
82a5ee5
7d29f57
98bddef
425dc78
5fcf590
ed2b8db
6ca873d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This is my biggest gripe with this approach. This sacrifices the simpler mental model for normal users and happy path and forces any mutative query to use
Mut<T>
over&mut T
, and that mapping from&T
to&mut T
is now lost.We can address this by implementing
&mut T: World Query
, and just forcibly mark everything matched as mutated, but that would make the distinction between&mut T
andMut<T>
extremely nuanced.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.
Yeah, I'm nervous about that, especially because of how subtly it breaks existing code.
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.
Not sure if we're on the same page, but I want to clarify that you can still write
Query<&mut T>
and have it returnMut<T>
-- the reflexive constraint is only for namedWorldQuery
structs made using the derive macro. Very little user code will be affected in practice. I think this constraint will surprise users at first, but I also think it will make sense when they think about it, or read the docs and have it explained to them (we'll have to think of a good way of explaining this that would make it click more easily). Also, this behavior is consistent with how theSystemParam
derive macro works, which might make it easier to understand for some users.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.
Another point I want to add: using
&mut T
in a derived WorldQuery already fails with a confusing type error, until you read the docs and see that you need the special attribute#[world_query(mutable)]
. Since we're already relying on the user to read the docs for this macro to be usable, I don't think there's much of a regression caused by requiring world queries to be reflexive.