gschup/bevy_ggrs

Upgrade for Bevy 0.9

johanhelsing opened this issue · 7 comments

johanhelsing commented

Bump Bevy to 0.9 Cargo.toml and fix porting issues

gschup commented

This seems to be non-trivial on first glance:

  • GGRS sessions are added as resources and thus now need to derive bevy's Resource trait through a feature flag in GGRS. Otherwise a wrapper struct is necessary.
  • taking a snapshot of registered resources used a now no longer available API (link). In this PR resource was moved to world storage, but iterating over all resource indices is no longer possible. world.archetypes().resource().unique_components() gave direct access to the underlying SparseSet<ComponentId, ResourceData>; but in the new world.storage.resources, this is now private (link). This change was made since full access enabled unsafe or unsound code. Since we only used the indices, this was not an issue for us. But for now we will have to find another way.
johanhelsing commented

Is it something we could try to bring back in Bevy 0.9.1 with a different name and/or unsafe warnings?

gschup commented

I think they conciously removed this and it will not come back:

All APIs accessing the raw data of individual resources (mutable and read-only) have been removed as these APIs allowed for unsound unsafe code. All usages of these APIs should be changed to use World::{get, insert, remove}_resource.

We only used the keys of the sparse set, so maybe there is hope. I do however think that there might be a nicer way to do this.

Vrixyz commented

It should be possible to list all resources again with iter() function as of bevy 0.9.1: https://github.com/bevyengine/bevy/blob/v0.9.1/crates/bevy_ecs/src/storage/resource.rs#L141.
link to bevy PR for context: bevyengine/bevy#6592.

  • 👍1
gschup commented

Done! Thank you for the great work @Vrixyz @johanhelsing 🎉

  • 🎉3
johanhelsing commented

Yay!

I guess it's time for a new release then?

bevy_ggrs is currently depending on the git version of ggrs, so I guess that means a release of both is needed, and by extension also matchbox_socket, since the matchbox ggrs feature depends on ggrs?

  • 👍2
gschup commented

Yes! I will close this issue once a new version is published.

  • 👍1