Generalize applier hook

[bbarker]

Hello,

I've been getting familiar with bevy_save while trying to use some other game plugins that take care of a lot of component handling and initialization. My current conclusion is that in this setting, it is generally better to let the systems do thir thing, and just save the state that I need and reapply it to existing entities.

I was able to do that with a minor change to Hook, and was wondering if it would be ok to submit an MR with this:

pub trait Hook: for<'a> Fn(&'a EntityRef, &'a EntityRef, &'a mut Commands) + Send + Sync {}

impl<T> Hook for T where T: for<'a> Fn(&'a EntityRef, &'a EntityRef, &'a mut Commands) + Send + Sync {}

// .... snip ....

if let Some(hook) = &self.hook {
    let mut queue = CommandQueue::default();
    let mut commands = Commands::new(&mut queue, self.world);

    for (old_entity, entity) in entity_map {
        let entity_ref = self.world.entity(*entity);
        let entity_old_ref = self.world.entity(*old_entity);

        hook(&entity_ref, &entity_old_ref, &mut commands);
    }

    queue.apply(self.world);
}

Here the changes are: 1) pass the more general Commands instead of EntityCommands, so we can use it on both entities if needed (I tried to construct two EntityCommands but had borrowing issues) and 2) also give access to the old entity.

Usage

Then I can use it like this:

  let res = snapshot
      .applier(world)
      .prefab::<PlayerPrefab>()
      .hook(move |entity, old_entity, cmds| {
          if let Some((grid_coords, new_transform)) = m! {
            _player <- entity.get::<Player>();
            grid_coords <- entity.get::<GridCoords>();
            let default_transform = Transform::default();
            transform <- entity.get::<Transform>().or(Some(&default_transform));
            let centered_position = centered_position(grid_coords);
            let new_trans = centered_position.extend(transform.translation.z);
            return (grid_coords, transform.with_translation(new_trans));
          } {
              let mut cmds_old_entity = cmds.entity(old_entity.id());
              cmds_old_entity.insert((*grid_coords, new_transform));
              let _ = cmds_old_entity.log_components();
              let cmds_new_entity = cmds.entity(entity.id());
              cmds_new_entity.despawn_recursive();
          };
      })
      .apply();

[bbarker]

A cleaner solution might be to modify Prefab's spawn from:

    fn spawn(self, target: Entity, world: &mut World);

to

    fn spawn(self, target: Entity, world: &mut World, orig_entity: Entity);

This would be closer to the original design and feel a bit less hacky in that we don't need a lot of manual hooks; the spawn will just need to call despawn on the new entity since it isn't actually needed.

But this change would requite more modifications to bevy_save, and i've not tested it yet. I may create a branch rather than continuing to hack in ~/.cargo/registry for this, but any comments welcome, as I'll take some time now to update my project to bevy 0.16

[bbarker]

@hankjordan Just wanted to see if you had any opinions on this approach - I'm happy to put together an MR if you're unsure, in any case.

[bbarker]

I added https://github.com/hankjordan/bevy_save/pull/47 but have not yet had a chance to test it at runtime, as my game may be suffering from some update issues after the 0.16 update.

But his is an example of how I'm using it:

    fn spawn(self, target: Entity, world: &mut World, target_original: Option<Entity>) {
        let target_to_use = target_original.unwrap_or(target);
        debug!("Calling PlayerPrefab::spawn");
        world
            .entity_mut(target_to_use)
            .insert((Player, self.grid_coords, self.animation));
        if target_to_use != target {
            world.entity_mut(target).despawn();
        }
    }

[bbarker]

As noted in #47, this is working - I hope #47 is an acceptable approach but welcome to feedback! thanks