Skip to content
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 map codecs more resistant to erroring entries. Fixes MC-197860 #55

Closed

Conversation

alcatrazEscapee
Copy link

This is a really simple change that I believe fixes MC-197860. More details are in the issue there, but the root of the problem is the UnboundedMapCodec's behavior in the case of erroring entries. Previously, this (along with any other BaseMapCodec implementor) would discard all valid entries after an error, while only actually reporting actual erroring entries.

This PR moves the collection of read results outside the check for the data result in the accumulator, such that all valid entries will get added to the immutable map builder, while keeping intact the behavior of the apply2stable on the actual success or failure of the result in question.

Copy link

@50ap5ud5 50ap5ud5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I can confirm this change does indeed fix MC-19786.

I will link some of my findings made for the Forge version fix:

  • Minecraft already forces player to overworld if dimension is missing
  • PR preserves vanilla dimensions
  • PR also preserves working custom dimensions

More information about the testing process and related files:
https://drive.google.com/file/d/1TNqRLxnF9YVujFEvU3STZPzDwUuTUo0-/view?usp=drivesdk

Copy link
Contributor

@liach liach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should only override this method in UnboundedMapCodec. This one imo would break the behavior of simple map codec and make it return incomplete maps for pojo en/decoding.

@peterix
Copy link
Contributor

peterix commented Oct 26, 2022

Closing and reopening to rerun checks.

@FoggierTiger535
Copy link

I have this issue on my singleplayer world, would love to get my nether and end dimensions avaliable again, how do I use this fix? I'm not good with code so this is probably really simple and im just dumb :D

@Gegy
Copy link
Member

Gegy commented Mar 19, 2024

Thank you for this PR! I believe the issue addressed here is now resolved by this commit. 😊

@alcatrazEscapee
Copy link
Author

Thank you!

@alcatrazEscapee alcatrazEscapee deleted the map-error-handling branch March 20, 2024 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.