Update to Bevy 0.12

[luca-della-vedova]

New feature implementation

Implemented feature

Closes #192, #128.
Update to Bevy 0.12.

Implementation description

Apart from the usual changed APIs, Bevy 0.12 introduced a virtual rewrite of the asset system that allowed a significant refactor and cleanup of the way we handle assets.

Specifically, the most important feature is support for multiple asset sources, which allows us to split the sources into separate structures, with two main consequences:

• The asset source logic is a lot more modular, external plugins can register their sources without having to edit a monolythic piece of code.
• The AssetSource enum is a lot cleaner, we can split out the sources that don't necessarily make sense for models (i.e. Bundled, used for icons and OSMTile used for open street map tiles) and put them in their own asset source.

Specifically for Bundled assets, the new asset system introduced a builtin embedded:// asset source that does exactly the same so we can get rid of more code.

Because of #128, it was necessary to move out of surf. Luckily a new crate came up recently, ehttp which is actively maintained, does the job perfectly fine and is very lightweight (our dependency count decreased by about 100 crates by switching!)

[luca-della-vedova]

There is currently a regression in workcell editor mode where the cursor is always displayed, currently investigating

[luca-della-vedova]

This PR is now ready for review.

Since this migration added a lot more deprecation warnings with the deprecation of event.iter() I took the chance to clear out all the trivial warnings from the codebase so we can avoid introducing new ones (they would get drowned in the 100+ existing warnings).
The only remaining ones now are nameclash collisions (also introduced here, although not breaking, for now) and one unused field that sounded not trivial.

[mxgrey]

I'm just leaving two very minor nitpicks around variable naming and documentation, and one suggestion for us to think about a Model bundle refactor in a future PR.

Thanks for the massive migration effort here, and for staying on top of these valuable developments in Bevy. It's great to see a PR that has an overall negative number of new lines.

[mxgrey]

This change actually flagged to me that we have a bug here. Inside this block is a line

.insert(RenderLayers::layer(XRAY_RENDER_LAYER))

That line should've been

.insert(RenderLayers::layer(layer))

As a result we weren't rendering outlines correctly in orthographic mode.

This is a good example of why we need to clean up the warnings in this codebase, so we're not drowning out things that could help us fix bugs 😬

I've pushed a fix for this.

[luca-della-vedova]

Good find! I did notice that outline were not rendered in ortographic mode but thought it was due to the top-down perspective, nice to have addressed that

[mxgrey]

It takes a lot of cognitive load when reading through this function to understand the differences between uri and path, and why they're used differently. It would be nice to give them more distinct variable names (e.g. asset_uri and sdf_model_path) and maybe a little documentation about what's going on here and why, e.g. an example input for each branch to make it clear what kind of case it's covering.

The terseness of the Rust language can be both a blessing and a curse at the same time.

[luca-della-vedova]

Good point, I added examples in the documentation and did a minor cleanup (removing one else condition that would never happen) in 3ab3f03

[mxgrey]

Should this be geometry_name since it also gets used for collisions, not just visuals?

[luca-della-vedova]

3ab3f03

[mxgrey]

NameInSite was meant to represent names that are unique within the site. If I understand correctly this is parsing visual and collision geometries, which doesn't actually align very well with how the Model bundle was originally meant to be used.

We can leave this as-is for now since we don't yet have any particular need for NameInSites to be unique, but perhaps we can think about how to refactor the Model bundle in a future PR. We should probably introduce a NameInModel, or possibly a more general NameInTree { tree: Entity, name: String } component.

[luca-della-vedova]

Fully agree, Model served well in the early days of the codebase when each model was a single GLB file but since we started to parse SDFs with hierarchies, include primitive shapes, split of collision and visual meshes, it became very complex.
Ideally the perfect design would allow granularity to encapsulate all the components of a Model into a top level object (for example to know where to fetch a model if it comes from fuel) but at the same time would allow inspecting its parts (toggling visual / collision meshes, plus some other features needed in workcells that work with individual submeshes)

[mxgrey]

Thanks for the added documentation! It's much more clear what's going on in sdf loading now.