Fix item migration issues by migrating items on load#3163
Fix item migration issues by migrating items on load#3163IntegratedQuantum wants to merge 2 commits into
Conversation
|
This by the way should also retroactively fix the iron issue in existing worlds |
| pub var itemListSize: u16 = 0; | ||
|
|
||
| // Due to migrations multiple indices can map to the same item. This must be resolved during inventory loading using this map. | ||
| var realItemIndices: [65536]BaseItemIndex = undefined; |
There was a problem hiding this comment.
| var realItemIndices: [65536]BaseItemIndex = undefined; | |
| var deduplicatingItemMap: [65536]BaseItemIndex = undefined; |
Or anything less general than real? IMO current name explains nothing really.
|
|
||
| newItem.init(main.worldArena, texturePath, replacementTexturePath, id, zon); | ||
| reverseIndices.put(main.worldArena.allocator, newItem.id, @enumFromInt(itemListSize)) catch unreachable; | ||
| const result = reverseIndices.getOrPut(main.worldArena.allocator, newItem.id) catch unreachable; |
There was a problem hiding this comment.
Following edge case comes randomly to my mind and I don't exactly have time to verify it rn:
Consider migration cubyz:bar -> cubyz:foo with two opposite palette states:
items:
...
33: cubyz:foo
...
37: cubyz:bar
...
cubyz:foo 33 is initialized first, then cubyz:bar (which is now cubyz:foo) 37 is deduplicated into cubyz:foo 33
Opposite situation:
items:
...
33: cubyz:bar
...
37: cubyz:foo
...
cubyz:bar 33 is initialized first, then cubyz:foo 37 is deduplicated into cubyz:bar 33 (which is now cubyz:foo too...
I'm not sure I understand implications if there are any.
There was a problem hiding this comment.
Ad. 2: Yeah I think it doesn't matter right as long as we override cubyz:bar with cubyz:foo its fine.
There was a problem hiding this comment.
I'm not sure I understand implications if there are any.
There should be none. Both instances load the same asset, both will point at the same index after loading
2 participants
We saw everyone's iron turn into an uncraftable and unstackable version after the last migration in #2095, this was not severe since iron is mainly used in tool crafting, so no resources are lost.
For #2117 this would be more annoying since you possibly (depending on how your palette is ordered) couldn't craft your logs into anything.
Since there is no nice solution for this and I don't want to iterate through everything, I decided to go with the easiest/least effort solution: Just migrate duplicate ids on load using a direct map to the real index. This should only have a small performance impact since item loading is done rarely.
I'd be happy to hear if anyone has a better idea. (Also did we not have an issue for this problem?)