Fix DimPos being persisted under a hardcoded NBT key - #235
Merged
Conversation
The ValueOutput migration made the DimPos NBT type write its data to a hardcoded "dim" child, while it is read back from a child named after the field. As a result, every persisted DimPos failed to deserialize with a "NoSuchElementException: No value present", and multiple DimPos fields in the same tag overwrote each other. Reading falls back to the old hardcoded key, so data written by affected versions remains readable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QoinY4GW1gaXggj3WxDUcF
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Problem
The
ValueInput/ValueOutputmigration left theDimPosNBT type inNBTClassTypesNeoForgeasymmetric — it writes to a hardcoded"dim"child but reads a child named after the field:So no persisted
DimPosever round-trips; reading always throws.master-1.21-ltsand earlier are correct (tag.put(name, dimPos)/tag.getCompound(name)), so this is a regression limited to theValueOutput-based branches. A second consequence: twoDimPosfields written into the same tag under different names clobber each other.This is what crashes IntegratedDynamics clients in CyclopsMC/IntegratedDynamics#1703, via
ValueTypeListProxyPositioned:PositionedOperatorand anything else persisting aDimPoshit the same thing.Changes
tag.child(name)instead oftag.child("dim")."dim"key when the named child is absent, so data already written by affected versions stays readable instead of throwing.Testing
loader-neoforge's own build does not resolve here (thecommoncapabilitiesAPI dependency is unreachable from this environment, and it fails identically on a clean checkout), so I type-checked the changed file against the locally builtloader-commonclasses plus the patched Minecraft jar, and drove a round-trip throughTagValueOutput/TagValueInput:Before:
(the reporter's exact frames)
After:
where
legacy readfeeds in a tag whoseDimPossits under the old"dim"key.Branch
Affected branches are
master-1.21.8,master-1.21.10,master-26-ltsandmaster-26. The two1.21.xbranches look dormant (last commits Nov/Dec 2025), so this targetsmaster-26-ltsfor upmerging — happy to retarget if you want it lower.Generated by Claude Code