Components deriving Reflect doesn't necessarily mean they should be saved

[ostwilkens]

Thank you for creating this plugin, and updating it so quickly for 0.11! I'm using it to set up "hot reload"-like development.

I add a MaterialMesh2dBundle to my player entity to make it visible in the game world. It contains the bevy_render::view::visibility::ComputedVisibility component, which I do not want to save, but does derive Reflect. This particular component causes an error when trying to save it: moonshine_save::save: save failed: Ron(Message("Type 'bevy_render::view::visibility::ComputedVisibilityFlags' did not register ReflectSerialize"))

I think it would make sense to require something like #[derive(Save)] for each component you want to save.

[Zeenobit]

Typically, you'd want to split your saved game data from game aesthetics for specifically these reasons. Think Model/View pattern, where your save data is the model, and the game data is the view.

So in your case, I would spawn 2 entities to represent the player, and link them via a non-reflect, non-saved component:

#[derive(Component)] // <--- Not saved!
struct PlayerVisuals(Entity);

#[derive(Component, Reflect)] // <--- Saved!
#[reflect(Component)]
struct Player;

struct PlayerBundle {
    ...
    player: Player,
    save: Save,
}

struct PlayerVisualsBundle {
    mesh: ...
    visibility: VisibilityBundle,
    ...
}

And then I would have a system which spawns a PlayerVisualsBundle any time a PlayerBundle is spawned, and associate it with a PlayerVisuals component:

fn spawn_player_visuals(query: Query<Entity, Added<Player>>, mut commands: Commands) {
    for entity in query.iter() {
        let player_visuals_entity = commands.spawn(PlayerVisualsBundle { ... });
        let player_visuals = PlayerVisuals(player_visuals_entity);
        commands.entity(entity).insert(player_visuals);
    }
}

Alternatively, if you absolutely must save include your aesthetic components with your saved data, you may use a custom save pipeline:

pub fn save_into_file_without_visibility(path: impl Into<PathBuf>) -> SavePipeline {
    save::<With<Save>>
        .pipe(remove_component::<ComputedVisbility>())
        .pipe(into_file(path.into()))
        .pipe(finish)
        .in_set(SaveSet::Save)
}

You can check the existing save pipelines to see how you can customize this further.

Personally, I'd recommend the first approach for scalability and simplicity.

[ostwilkens]

Thank you for the quick reply! When structuring things like that, the current requirements do make sense. I have spent quite some time working with bevy, and I haven't considered setting things up like this. It's a testament to how flexible bevy & ecs really is. Retaining this flexibility is key to making the plugin useful for lots of people.

In my mind, it seems more complex to have multiple entities representing the same thing. I prefer my single player entity to have all entities that pertain to the player. I think of it like a key or ID, and systems can select only the relevant components, so it is not an issue if the player has lots of components.

Most important, for adoption at least, is ease of integration into existing projects. Having the Model/View-setup as a requirement (or custom pipeline) can be a pretty high barrier-to-entry. The simplest possible integration seems to me, to point out which components you want to persist, and you're done.

I understand if you feel otherwise, and I could attempt implementing this myself if you disagree.

[ostwilkens]

Having developed my thoughts on this I think an attribute on bundle struct fields makes most sense. I'm inexperienced on attributes so this might be incorrect, but this is the gist of it.

#[derive(Bundle)]
struct PlayerBundle {
    #[save]
    player: Player, 
    #[save]
    position: Position,
    #[save]
    velocity: Velocity, 
    // do not save:
    sprite: MaterialMesh2dBundle,
}

[Zeenobit]

I initially considered a similar implementation, but later I decided against it. This is mainly because the bundle itself never has any representation on the entity. Which means to implement the API you're suggesting, you'd have to keep track of exactly which components are saved on the entities, potentially requiring "meta components" or requiring a separate mapping of entities and saved component IDs somewhere else. This design gets more and more complicated the further you go.

An alternative approach would be the one you proposed first (i.e. #[derive(Component, Save)]). This approach would require registration of components with the app so that the save pipeline can filter for these components. Again, this would require keeping a separate mapping of such components, which means these mapping would have to be populated at runtime, most likely via some sort of a .register_saved_type<T>() app function, similar to #[derive(Reflect)] and .register_type<T>().

For this library, I specifically wanted to avoid these problems, which is why it's designed as such. In my opinion, it makes for a much more simplified architecture as there is no additional mapping stored anywhere, and you can tag entities for save without any further registration at runtime. I think the separation between saved and game data is also a perk because then the save data can act as your single source of truth on which you build your game visuals.

Keep in mind, there are alternative save libraries which do implement the other method: https://crates.io/crates/bevy_save This one specifically does exactly what I mentioned earlier using App.register_saveable::<T>().

Additional libraries: https://github.com/bevyengine/bevy-assets/issues/335

[csandven]

Have the same issue with this when updating to the latest version. I have multiple sprites that are children of an entity that gets saved, they are inserted like the documentation here is recommending. Before updating the version, I used the hierarcy feature and it worked beautifully.

Example:

#[derive(Bundle, Default)]
pub struct CitizenBundle {
  spatial: SpatialBundle,
  name: CitizenName,
  save: Save,
  load: Unload,
}

impl CitizenBundle {

  // The new entities area spawned here and added as children where this fn is called.
  pub fn spawn_sprites(
    commands: &mut Commands,
    body_parts: &BodyParts,
    image_assets: &ImageAssets,
  ) -> (Entity, Entity, Entity) {
    let body = commands
      .spawn((
        SpriteSheetBundle {
          sprite: TextureAtlasSprite {
            index: body_parts.body.to_owned(),
            ..Default::default()
          },
          texture_atlas: image_assets.citizen_bodies.clone(),
          ..Default::default()
        },
        BodyPartStartIndex(body_parts.body.to_owned()),
      ))
      .id();
    let hair = commands
      .spawn((
        SpriteSheetBundle {
          sprite: TextureAtlasSprite {
            index: body_parts.hair.to_owned(),
            ..Default::default()
          },
          transform: Transform::from_xyz(0., 18., 51.),
          texture_atlas: image_assets.citizen_hair.clone(),
          ..Default::default()
        },
        BodyPartStartIndex(body_parts.body.to_owned()),
      ))
      .id();
    let head = commands
      .spawn((
        SpriteSheetBundle {
          sprite: TextureAtlasSprite {
            index: body_parts.head.to_owned(),
            ..Default::default()
          },
          transform: Transform::from_xyz(0., 18., 50.),
          texture_atlas: image_assets.citizen_heads.clone(),
          ..Default::default()
        },
        BodyPartStartIndex(body_parts.body.to_owned()),
      ))
      .id();
    (body, head, hair)
  }
}
}

[Zeenobit]

@csandven If you don't want hierarchy (or any other component), you can use remove_component and a modified save pipeline.

For example, if you're using save_into_file_on_request, you can make a modified version of it:

pub fn save_into_file_on_request_without_hierarchy<R>() -> SavePipeline
where
    R: SaveIntoFileRequest + Resource,
{
    (
        save::<With<Save>>
            .pipe(remove_component::<bevy_hierarchy::Children>)
            .pipe(remove_component::<bevy_hierarchy::Parent>)
            .pipe(file_from_request::<R>)
            .pipe(into_file_dyn)
            .pipe(finish),
        remove_resource::<R>,
    )
        .chain()
        .distributive_run_if(has_resource::<R>)
        .in_set(SaveSet::Save)
}

I want to make pipeline customization a bit more ergonomic with something like a SavePipelineBuilder. But for now, this should give you the same behavior as before without the hierarchy feature enabled.

Also, you don't need to mark Saved entities with Unload as its redundant. Save implies Unload automatically.

[Zeenobit]

As of v0.3.1, you can use a SaveFilter with custom save pipelines. The default save pipelines generate SaveFilter for all entities with a Save component:

pub fn save_into_file(path: impl Into<PathBuf>) -> SavePipeline {
    filter::<With<Save>>
        .pipe(save)
        .pipe(into_file(path.into()))
        .pipe(finish)
        .in_set(SaveSet::Save)
}

filter is a simple system which just collects all entities with a Save component:

#[derive(Default)]
pub struct SaveFilter {
    pub entities: EntityFilter,
    pub scene: SceneFilter,
}

pub fn filter<Filter: ReadOnlyWorldQuery>(entities: Query<Entity, Filter>) -> SaveFilter {
    SaveFilter {
        entities: EntityFilter::allow(&entities),
        ..Default::default()
    }
}

A SceneFilter may be used to allow/deny components as required. This is passed directly into the DynamicSceneBuilder during serialization. An EntityFilter may be used to filter entities.

This change simplifies the implementation of custom save pipelines with mixed data. For example, to avoid saving hierarchy like the previous comment:

pub fn filter_without_hierarchy<Filter: ReadOnlyWorldQuery>(entities: Query<Entity, Filter>) -> SaveFilter {
    let entities = EntityFilter::allow(&entities);
    let mut scene = SceneFilter::default();
    scene.deny::<Parent>();
    scene.deny::<Children>();
    SaveFilter { entities, scene }
}

pub fn save_into_file_on_request_without_hierarchy<R>() -> SavePipeline
where
    R: SaveIntoFileRequest + Resource,
{
    filter_without_hierarchy::<With<Save>>
        .pipe(save)
        .pipe(file_from_request::<R>)
        .pipe(into_file_dyn)
        .pipe(finish)
        .pipe(remove_resource::<R>)
        .run_if(has_resource::<R>)
        .in_set(SaveSet::Save)
}

[Zeenobit]

As of v0.3.2, components can now be excluded from a save pipeline using the SavePipelineBuilder::exclude_component() function.