-
Notifications
You must be signed in to change notification settings - Fork 413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make DFU error-tolerant to mod custom generator types #3213
Conversation
fabric-dimensions-v1/src/main/java/net/fabricmc/fabric/mixin/dimension/Schema2832Mixin.java
Outdated
Show resolved
Hide resolved
The core thing is in All other mixins are for only enabling fail-soft in the specified places. If making all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think minimizing behavioral impact is more important, so having more mixins is ok.
* Make DFU error-tolerant to mod custom generator types. * Fix license. Rename mixin. * Fix license. * Disable remap in Schema2832Mixin
* Make DFU error-tolerant to mod custom generator types. * Fix license. Rename mixin. * Fix license. * Disable remap in Schema2832Mixin
The PR fixes an issue of DFU that, when upgrading a world, it fails to deserialize mod custom generator type when upgrading, then the vanilla dimensions may get lost.
My previous PR about dimension deserialization is different. That PR fixes the issue after uninstalling dimension mod/dimension datapack by making the deserialization fail-soft. That deserialization is for reading data in
level.dat
. This PR fixes deserialization in DFU, triggered when upgrading world.In MC, it firstly upgrade the data then deserialize the data. It also does deserialization and serialization during upgrading. The deserialization here means the deserialization during upgrading.
The fix is to change
Schema2823
and make itsTaggedChoice
to ignore unknown chunk generator types.When testing it, I found that DFU is sensitive to dimension id's hashcode, that's why I added another dimension
fabric_dimension:void_earilier_in_hash_map
into the test mod. InNbtCompound
it usesHashMap
so the order in the map is correlated with hashcode. InCompoundListCodec.decode
, it deserializes according to the order in the map. And when one entry's deserialization fails, the thing inresult
will be a failed result. Inapply2stable
, if one fails, the failure will propagate the the remaining elements (it's how applicative works). If the result is in failed state, the lambda inapply2stable
will not execute, so the remaining things will not go to the result even if deserialziation succeededs. The dimensionfabric_dimensions:void
is after all vanilla dimensions in hash map so it will not trigger this issue. So I added the dimensionfabric_dimension:void_earilier_in_hash_map
to make this issue manifest.DFU is so "GREAT".
This only happens with custom chunk generator types (not happening with dimension datapacks). To test this, I added the dimension
fabric_dimension:void_earilier_in_hash_map
in 1.19.4 branch of fabric api and created a world that contains that dimension in 1.19.4.TestDimUpgrade.zip
Without this PR, opening that world then it will log
Unsupported key: fabric_dimension:void; Unsupported key: fabric_dimension:void
and the nether and end dimensions will vanish.This makes the DFU to not fail when encountering custom generator type. (The PR also make it not fail when encountering custom biome source type in vanilla chunk generator types). It will possibly be compatible with mod custom datafixers for mod custom chunk generator in
level.dat
(I haven't seen such mod).About the usage of
@Redirect
: MixinExtra allows WrapOperation to a call that has been redirected, so there is not much need to replace redirect to other injections (I think other injections are more error-prone than redirect in this case).