Skip to content
This repository has been archived by the owner on Jul 16, 2024. It is now read-only.

Snapshot should return a "View" Handle #79

Open
Ukendio opened this issue Dec 20, 2023 · 1 comment · May be fixed by #80
Open

Snapshot should return a "View" Handle #79

Ukendio opened this issue Dec 20, 2023 · 1 comment · May be fixed by #80
Labels
enhancement New feature or request

Comments

@Ukendio
Copy link
Contributor

Ukendio commented Dec 20, 2023

queryResult:snapshot() performs all of the iterator task at once and provides you the results of the query at the moment it is called. Hecs has a similar feature with query().views(), which is meant for repeated random access. It provides you a handle to discern whether it contains a specific entity and to access its data easily at an amortized cost via Views.get() and Views.contains() respectively.

I think providing similar methods will make :snapshot a more meaningful arm to QueryResult. We could even naively implement this by changing the list into a dictionary that maps the entity id to its packed data, and returning it, allowing people to just index it. But this wouldn't preserve the same order of traversal of entities as the query.

Considerations

This is a breaking change as it would most likely change the return type. Alternatively we can look to make a separate method called view.

Side note

Currently, you can't follow the :without() arm with :snapshot(). We may want to make a separate change to address this. #85

For tracking
Partial implementation: #80

@Ukendio
Copy link
Contributor Author

Ukendio commented Dec 20, 2023

@LastTalon Extending the API instead

But I think we should extend the API rather than change it. I think adding a view method would be better. Bevy has a lot of thought into these aspects of usage. Since future changes we may want to make down the road and/or as roblox makes things possible will be affected by the API we make now. What I definitely don't want to do at this moment is break people's current use of snapshot. We're allowed to break things like that in v0, but I don't see a good reason to with this. There's no reason the current snapshot is bad. It just doesn't do these other things we also want.

@Ukendio Ukendio linked a pull request Dec 20, 2023 that will close this issue
4 tasks
@jackTabsCode jackTabsCode added the enhancement New feature or request label Dec 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants