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

TryRead/Write for querying optional components #25

Merged
merged 2 commits into from
Nov 11, 2019

Conversation

Ralith
Copy link
Contributor

@Ralith Ralith commented Nov 7, 2019

If this implementation strategy looks good, I'll follow up with a similar MaybeWrite.

@Ralith Ralith changed the title MaybeRead for querying optional components TryRead for querying optional components Nov 7, 2019
tests/query_api.rs Outdated Show resolved Hide resolved
@jaynus
Copy link
Contributor

jaynus commented Nov 9, 2019

@AThilenius would this meet your needs for the problems we ran into in legion_transform and reduce a lot of that query boilerplate?

@AThilenius
Copy link
Contributor

Yep, I could replace the dozen separate queries with this. It could still pick up invalid combinations like Scale + NonUniformScale, but throwing a runtime error ther is totally acceptable. Can this be done efficiently though? I'm on my phone so don't want to dig through code.

@Ralith
Copy link
Contributor Author

Ralith commented Nov 9, 2019

Can this be done efficiently though? I'm on my phone so don't want to dig through code.

A query containing TryRead should cost about the same to execute as the same query with that field removed. It's significantly more efficient than calling get_component inside a query.

@jaynus
Copy link
Contributor

jaynus commented Nov 9, 2019

It's significantly more efficient than calling get_component inside a query.

The solution currently in legion_transform is about 12 different queries for all variations, all iterating in parallel. I wonder if that is actually faster than this case still.

@TomGillen
Copy link
Collaborator

I expect performance would be similar (assuming you use par_for_each or equivalent), but the code would be vastly simplified.

@Ralith
Copy link
Contributor Author

Ralith commented Nov 10, 2019

Added TryWrite along the same lines.

@Ralith Ralith changed the title TryRead for querying optional components TryRead/Write for querying optional components Nov 10, 2019
@TomGillen TomGillen merged commit 0559919 into amethyst:master Nov 11, 2019
@Ralith Ralith deleted the maybe branch November 11, 2019 17:14
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.

4 participants