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

Redesign ComponentAccessExpr, improve error diagnostics #35

Merged
merged 10 commits into from
Apr 5, 2024
Merged

Conversation

rj00a
Copy link
Owner

@rj00a rj00a commented Apr 5, 2024

This PR redesigns the ComponentAccessExpr type to track access conflicts more precisely. Queries such as Or<Or<(&mut A, With<&B>), (&A, Not<&B>)>, (&A, Not<&B>)> and (&mut A, &mut A, Not<&A>) will now pass the component access checker.

Additionally, we now defer access errors until after handler init has finished. This improves diagnostics by showing a list of all access conflicts together.

TODO

  • Clean up warnings

@rj00a rj00a marked this pull request as ready for review April 5, 2024 15:45
@rj00a rj00a merged commit 8ab990d into main Apr 5, 2024
6 checks passed
@rj00a rj00a deleted the access_redesign branch April 5, 2024 16:32
@andrewgazelka
Copy link
Contributor

yay :)

@andrewgazelka
Copy link
Contributor

andrewgazelka commented Apr 5, 2024

I think there is a regression.

My code suddenly stops working (I can't see entities or anything on my server) when using 8ab990d. It works perfectly fine on https://github.com/andrewgazelka/evenio/tree/fix-collisions-gt-2?branch=fix-collisions-gt-2

https://github.com/andrewgazelka/hyperion

rj00a added a commit that referenced this pull request Apr 6, 2024
`Not<Q>` queries weren't being handled correctly in `matches_archetype`.
There are now unit tests to cover this.

Should fix the issue mentioned in
#35 (comment)
@rj00a
Copy link
Owner Author

rj00a commented Apr 6, 2024

@andrewgazelka Should be fixed in #37. Let me know if you run into anything else.

@andrewgazelka
Copy link
Contributor

@andrewgazelka Should be fixed in #37. Let me know if you run into anything else.

Fixed for me :)

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