Add block migrations to mitigate damage after block renames

[Argmaster]

This pull request adds tracking mechanism for asset renaming / relocation. The name "migrations" was chosen because of similarity to database migrations produced by ORMs for chaning of table layout of exisiting databases.

TODO/CHANGELOG:

• Add block migrations
• Fix all the review comments in the world

---

Usage

File assets/cubyz/blocks/_migrations.zig.zon allows to specify mapping from old ID to new ID. IDs do not include addon name, it is implied by location of _migrations.zig.zon:

.{
    .{
        .old = "oak_log",
        .new = "oak_log_new_id",
    }
}

progress towards #897

[IntegratedQuantum]

I think this is too complicated. A migration shouldn't have a type, or a name. And there should be only one file for it at the top level in the blocks/items/... directory, with a plain list of .@"old" = "new" mappings.

Secondly I would like you to test renaming two files to the same id, this has happened in the past and will likely happen again (e.g. after #357).

And finally I wonder: Does this approach scale well? Do we now need to iterate through all player's inventories, through all chunks with chests in them? And from what I can tell you'd do that every time the world is loaded, not just once after an update.

[Argmaster]

I think this is too complicated. A migration shouldn't have a type, or a name. And there should be only one file for it at the top level in the blocks/items/... directory, with a plain list of .@"old" = "new" mappings.

Ok, I was thinking about it at it seems fine in most scenarios. Although it makes it so it is not possible to control order of migrations and this is strict many-to-one relation, so we can not reuse an ID we had before and handle this with migration.
For example, if we add vines, rename them to jungle_vines then we can't add new block called vines for willow vines, they must have unique name, as vines is already present in migration map and will be renamed to jungle_vines upon loading. If we want to reuse ID, we must remove it from migration map and then on old worlds that were not migrated vines will change to different type.

If I understand correctly, handling multiple renames of the block across multiple versions is possible, eg.:

version 0:

• cubyz:limestone_wall -> cubyz:limestone/limestone_wall

version 1:

• cubyz:limestone_wall -> cubyz:limestone/wall
• cubyz:limestone/limestone_wall -> cubyz:limestone/wall

We just skip intermediate transitions. That is good for time complexity too.

Secondly I would like you to test renaming two files to the same id, this has happened in the past and will likely happen again (e.g. after #357).

You mean like when you merge two blocks into one? In theory it could work, but the result would be two numerical IDs mapping to same block name, I am not sure about the implications from game engine perspective, but you can't just shift palette to remove that gap.

And finally I wonder: Does this approach scale well? Do we now need to iterate through all player's inventories, through all chunks with chests in them? And from what I can tell you'd do that every time the world is loaded, not just once after an update.

It scales well as long as we use palette.zig.zon, items.zig.zon and biomes.zig.zon to map item names to numerical IDs and player inventories (or chests) are stored as numerical IDs, not as full block/item names (cubyz/stone). If you do that, you never have to check anything other than those 3 files (or rather palettes you load from them) because you are never updating numerical IDs, you are only updating what block/item names they map to.

[Argmaster]

I have redesigned migrations to be an hash map loaded from blocks/_migrations.zig.zon, items/_migrations.zig.zon and biomes/_migrations.zig.zon.

@IntegratedQuantum please have look at this before I go and implement palette updates for items and biomes.

[IntegratedQuantum]

I hate to do this, but this PR is too big which makes it difficult to review, I've had similarly sized PRs in the past and it always takes an eternity to review.

So please do the following:

• Move all the unrelated changes, such as reordering, reformatting and others into a separate PR
• Try to simplify your code if possible
• Try to split the functionality into smaller parts, e.g. here we could easily separate migration loading from applying the migrations.

Also somewhat related to that I'd suggest to read the contributing guidelines if you haven't already.

[IntegratedQuantum]

You mean like when you merge two blocks into one? In theory it could work, but the result would be two numerical IDs mapping to same block name, I am not sure about the implications from game engine perspective, but you can't just shift palette to remove that gap.

Well, I'm not sure about the implications either, that's why I want you to test it.

[Argmaster]

There, I yeeted ~170 lines of unnecessary changes.

I did read contribution guidelines.

I am doing one feature, the feature is ID migrations. To implement and verify that I need to implement full flow at once.
Since the diff is getting scarry, I think we could include items and biomes migrations as separate PR, so unexpected problems don't boost this diff >500 lines again.

There was quite some unnecessary whitespace changes, I got rid of them. The rest I consider a necessary byproduct of my work, dictated by boy scout rule Leave the code better than you found it.

While reviewing you can mark files as Viewed (top right corner), so unless there are new changes / force pushes in those files, they will stay collapsed for consecutive reviews. There was a lot of changes since last review, but there was also significant shift in paradigm here. Sorry for doing the stupid thing first, I'm getting used to zig as I go.

I don't agree with lack of style guidelines enforced by formatter, but I guess not my sandbox, not my rules. If you won't automate what is there to be automated you will always waste time disputing if and in boolean expression looks like function call.

[IntegratedQuantum]

I just tried this and I think there are 2 things that make it a bit awkward to use:

• The block always needs to be prefixed with cubyz:. This not only adds more noise, but also allows you to remap ids from another addon, which should be illegal in my opinion.
• Having source = destination feels weird. I think destination = source is more natural, at least to me as a programmer. @ikabod-kee I'd like to hear your opinion on this.

[Argmaster]

I think that we should anticipate that migrations might be also useful for addons to alleviate their block changes. On top of that remapping vanilla blocks to custom blocks is also a feature, as this leaves an option to substitute stuff without rewriting all of the biome code. It's just that with great power comes great responsibility.

[IntegratedQuantum]

On top of that remapping vanilla blocks to custom blocks is also a feature

Yeah, but the game re-adds all blocks that weren't found in the palette, so if an addon does this, then the palette grows with each launch.

Now that I think about it, maybe we should only allow migrations if the source id isn't there anymore. That would prevent you from doing this accidentally.

[Argmaster]

Oh but there is a problem with making it destination = source - It would make it impossible to merge two blocks into one, as hash map keys have to be unique, destination would not be allowed to appear twice to replace two source IDs. To keep merging ability we would have to change storage medium to list, so no nice assignment syntax anyway. On top of that I think it would increate time complexity from O(n) to O(n*m), as both palette and migrations would be list, so search complexity increases from O(1) to O(n) regardless of which list we chose to iterate.

[Argmaster]

Also I don't agree it feels weird, as you are assigning new value to old ID, hence old = new notation, same way as if you were assigning new value to existing variable.

Damn thinking about which side is source and which is destination makes me sick, you sure you don't want list of objects notation? 😝

.{
    .{.old = "foo", .new = "bar"}
}

[IntegratedQuantum]

Yeah, maybe the list of objects is the best approach to avoid any ambiguities.

[Argmaster]

Back to (almost) initial implementation it is! (or rather will be :P)

[IntegratedQuantum]

Yeah, on the user facing front it's pretty similar.

[IntegratedQuantum]

Everything seems good now, and it works, but there is still some work ahead:

• block migrations alone are not good enough without item migrations, these should be implemented before using block migrations for anything.
• allowing multiple block ids would be nice, this is also relevant if you accidentally opened the world after renaming, but before adding an entry to the migration table (in which case it has already created the new block with a separate id)

[IntegratedQuantum]

In the future, how about adding a struct that stores all migrations and zon data, the struct could handle the init, deinit and clone operations, which would greatly simplify some of the code here.

[Argmaster]

Yes, this is probably the way to go.