Save versioning and migrations

[hankjordan]

Even when the bevy_save save format is stabilized, users will need a way to migrate save files between different versions of their app.

Ideally, this should be handled transparently.

[dmlary]

Do you have a rough architecture in mind for versioning? I've hit the point where moving forward with bevy_save doesn't make sense without versioning. I'm happy to spend some time on a PR, but I don't want to diverge wildly from something you already have planned.

[hankjordan]

Do you have a rough architecture in mind for versioning? I've hit the point where moving forward with bevy_save doesn't make sense without versioning. I'm happy to spend some time on a PR, but I don't want to diverge wildly from something you already have planned.

Not particularly, maybe something along these lines:

// must be dyn-safe
pub trait Migration {
  fn up(&self, ...);
  fn down(&self, ...) {
    // Default implementation does nothing
  }
}

Note that 'migrations' may be responsible for

• Snapshot changes
  • Changing type names for components and resources
  • Adding or removing components from entities
  • Adding or removing entities
  • Adding or removing resources
• Saver / Loader changes
  • Translating between different save file formats (eg JSON / Bincode / your-savefile-v1 / your-savefile-v2)
  • Changing save file directory structure (or table layout for databases)
• Changing workspace name (affects save directory)
• ... other things I may have missed

[dmlary]

I'm missing a few pieces still:

• Where does version sit?
  • Is it in Snapshot?
  • Is it in a user-defined, top-level wrapper struct containing a Snapshot?
  • Is it per-entity or per-component?
• Similar to the above, where do you envision users implementing the Migration trait?

I can see the use of a per-Component Migration, but it could get very ugly when renaming/removing components. I think that would require users to keep dead components around forever, but that may already be a problem.

[hankjordan]

I'm missing a few pieces still:

* Where does `version` sit?
  
  * Is it in `Snapshot`?
  * Is it in a user-defined, top-level wrapper struct containing a `Snapshot`?
  * Is it per-entity or per-component?

* Similar to the above, where do you envision users implementing the `Migration` trait?

I can see the use of a per-Component Migration, but it could get very ugly when renaming/removing components. I think that would require users to keep dead components around forever, but that may already be a problem.

• Version does not belong in the Snapshot type itself
• No wrapper types if we can avoid it
• There would be a single global AppVersion resource that is initialized by the user
• Users will likely have a module called migrations that contains many individual implementations of Migration, similar to how migrations are handled elsewhere
• We definitely need the ability to migrate individual components, with both name changes and field changes
  • Keeping old versions of types is an inherent issue with migrations and can not be avoided. This problem can be mitigated by embedding migrated types within the migrations themselves:

struct Migration000;

impl Migration for Migration000 {
  fn up(&self, ...) {
    struct OldType {
      key: String,
      value: f32,
    }

    struct NewType {
      key: String,
      value: String
    }

    todo!()
  }
}

• You would need to specify the type names of the old types to migrate

Additionally:

• Perhaps the Saver / Loader should be in charge of versioning - databases might write the version to a PRAGMA or a __bevy_save table, file IO would write to version.txt, etc
  • This does not solve the problem of switching between bevy_save versions or Saver / Loader implementations
  • Another solution could be to save / load version to a specifically named key but in a known format, this means the Saver / Loader would not need to be updated to have versioning
• Each Migration should not be limited to a single action (or a single component), this would be an ergonomic nightmare

[dmlary]

Ok, I see now. You’re viewing the save data as a single database that is migrated when the new version of the app is run.

I’m actually using bevy_save to write individual save files that can be loaded in any later version of the app (it’s a level editor). My usage of bevy_save is limited to SnapshotBuilder and SnapshotSerializer directly, as the database approach used at the high-level of bevy_save doesn’t allow me to split levels and/or tilesets out to individual files.

I’m gonna think about this more to see if there’s some overlap here for versioning that I can leverage and implement.

[hankjordan]

Actually, now that I think about it some more you sre 100% right - we have no real way of discovering all of the individual snapshots to migrate when loading the App!

Using it separately is definitely something I want to support as it is currently very necessary

So "version" really belongs in the SnapshotSerializer and SnapshotDeserializer(?). It would then appear transiently as a field in the save file.

[dmlary]

So "version" really belongs in the SnapshotSerializer and SnapshotDeserializer(?). It would then appear transiently as a field in the save file

I'm not sure if SnapshotSerializer & SnapshotDeserializer will work with the addition of &impl Migration on the deserialize size.

struct Migration000;
impl Migration for Migration000 {}

// serialize looks just fine
SnapshotSerializer::new(&snapshot, registry)
    .with_version(1)
    .serialize(&mut serializer)?;

// if the version is only available in the file and not in `Snapshot`, then we must pass
// `impl Migration` to the deserializer 
let de = SnapshotDeserializer::new(&registry)
    .with_migrations([Migration000, ...]);
let snap = de.deserialize(deserializer)?;
snap.into_applier(world).apply();

// Compare that to having version in `Snapshot`
let de = SnapshotDeserializer::new(&registry);
let snap = de.deserialize(deserializer)?;
snap.apply_migrations([Migration000, ...]);
snap.into_applier(world).apply();

To me, it feels more logical for migration to be applied to the Snapshot not the Deserializer which makes me want to put version in Snapshot.

That said, if you're opposed to that, perhaps another option is for the Deserializer to return a tuple of (Snapshot, usize), with the second value being the version from the file.

[hankjordan]

So "version" really belongs in the SnapshotSerializer and SnapshotDeserializer(?). It would then appear transiently as a field in the save file

I'm not sure if SnapshotSerializer & SnapshotDeserializer will work with the addition of &impl Migration on the deserialize size.

struct Migration000;
impl Migration for Migration000 {}

// serialize looks just fine
SnapshotSerializer::new(&snapshot, registry)
    .with_version(1)
    .serialize(&mut serializer)?;

// if the version is only available in the file and not in `Snapshot`, then we must pass
// `impl Migration` to the deserializer 
let de = SnapshotDeserializer::new(&registry)
    .with_migrations([Migration000, ...]);
let snap = de.deserialize(deserializer)?;
snap.into_applier(world).apply();

// Compare that to having version in `Snapshot`
let de = SnapshotDeserializer::new(&registry);
let snap = de.deserialize(deserializer)?;
snap.apply_migrations([Migration000, ...]);
snap.into_applier(world).apply();

To me, it feels more logical for migration to be applied to the Snapshot not the Deserializer which makes me want to put version in Snapshot.

That said, if you're opposed to that, perhaps another option is for the Deserializer to return a tuple of (Snapshot, usize), with the second value being the version from the file.

I really think migrations belong in the Deserializer.

This allows us to have some guarantee that once you have a Snapshot it is valid for the current version of the application.

The same reasoning is why Snapshot is immutable and you must create one via Builder. It is an attempt to make invalid state unrepresentable.

It might be a little less ergonomic like this but I don't see any reason it couldn't work.