Add world to scene hook

[richardhozak]

Adds read only access to world when running scene hook.

My motivation for this change was, that i needed to access Meshes to create colliders, not unlike the original discussion that seems to lead to the creation of bevy-scene-hook: bevyengine/bevy#4933.

Previous version of bevy had access to world through EntityRef, documented here in migration guide: https://bevyengine.org/learn/migration-guides/0.11-0.12/#allow-disjoint-mutable-world-access-via-entitymut, the suggestion there seems to be to add system param to access World which this PR does.

---

When I made this PR, I looked into all of the source code of bevy-scene-hook to replace all occurences of SceneHook::new to add world parameter and found that there is another reloadable hook which does have World parameter and realized I can use that instead of adding World parameter to SceneHook and since I've already made the changes, the least I could do is to make a PR, so there is at least parity between the two hooks and SceneHook can be used if someone does not want the reload functionality.

Thanks for making this crate, it is an elegant solution with so little code.

[richardhozak]

When I am at it, here's an example from original bevy issue bevyengine/bevy#4933 that is made possible with this change, it is also quite similar to the code I am using:

Original code from issue:

commands.spawn_bundle(SceneBundle {
    scene: asset_server.load("models/FlightHelmet/FlightHelmet.gltf#Scene0"),
    hook: SceneHook::new(|entity_ref, entity_commands| {
        if let Some(mesh) = entity_ref.get::<Mesh>() {
            // Entity have mesh, let's assign collision:
            entity_commands.insert(RigidBody::Fixed);
            entity_commands.insert(Collider::from_bevy_mesh(mesh));
        } else if let Some(name) = entity_ref.get::<Name>() {
            // Can insert some components based on name
        }
    }),
    ..Default::default()
});

Current possible code with bevy-scene-hook and this PR (cleaned up and also updated to bevy v0.12):

commands.spawn(HookedSceneBundle {
    scene: SceneBundle {
        scene: asset_server.load("models/FlightHelmet/FlightHelmet.gltf#Scene0"),
        ..Default::default()
    },
    hook: SceneHook::new(|entity, cmds, world| {
        if let Some(mesh_handle) = entity.get::<Handle<Mesh>>() {
            let meshes = world.resource::<Assets<Mesh>>();
            let mesh = meshes.get(mesh_handle);
            // Entity have mesh, let's assign collision:
            cmds.insert(RigidBody::Fixed);
            cmds.insert(Collider::from_bevy_mesh(mesh));
        } else if let Some(name) = entity.get::<Name>() {
            // Can insert some components based on name
        }
    })
});

[nicopap]

Yep, the reloadable hook is indeed the secret super-powered version of the basic hook. It's not very discoverable, and that's my fault, you are not the first one to discover it after solving something the reloadable hook already solves.

I'm not interested in merging this in the immediate future because it's a breaking change.

A few questions:

• The next breaking release will be for bevy 0.13, so maybe it makes sense to add the world argument then?
• Does the change makes sense if the reload module exists? I'm against it (though I can be swayed). There is a quality in simplicity, and we lose it. And any benefit is rendered moot by the existence of the reload::Hook which already does all of this (and more).
• I do need to make the reload module more discoverable. Can you tell me how you could have discovered it earlier? I suppose dedicating a paragraph to it in the README and the crate-level doc could help.

[richardhozak]

• The next breaking release will be for bevy 0.13, so maybe it makes sense to add the world argument then?

Sounds reasonable.

• Does the change makes sense if the reload module exists? I'm against it (though I can be swayed). There is a quality in simplicity, and we lose it. And any benefit is rendered moot by the existence of the reload::Hook which already does all of this (and more).

I do like the simplicity of SceneHook as it fits very nicely into the rest of bevy, with HookedSceneBundle where you use standard asset_server.load() along with components from bevy-scene-hook. I would love to have this simple api combined with reloadability in one hook, maybe it can be done - doesn't bevy support hot reloading of assets under the hood that could be combined by calling SceneHook again? What I mean, try if there is a way to keep SceneHook but make it reloadable.

• I do need to make the reload module more discoverable. Can you tell me how you could have discovered it earlier? I suppose dedicating a paragraph to it in the README and the crate-level doc could help.

I think mentioning it in crate-level doc would help me. The crate level doc now states:

Please see the SceneHook documentation for detailed examples.

Just mentioning the reload::Hook would suffice:

Please see the SceneHook documentation for detailed examples.
Please see the reload::Hook documentation for more advanced usage.

[richardhozak]

Few ideas for making the hook reloadable but simple:

• Remove reload::Hook::file_path and use AssetServer::get_handle_path() then you can use SceneBundle, bringing the reload::Hook closer to SceneHook
• Make SceneHook into enum and make SceneHook::new which would work as it works now and SceneHook::new_reloadable which would create reloadable SceneHook variant

[nicopap]

Hello,

I integrated your suggestion wrt to file_path for reload::Hook, and added several references to the reload module across documentation in 10.0.0. Thank again for your help!