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

Enchantement ids were not retrieved correctly during deserialization #3906

Merged

Conversation

AlexandreArcil
Copy link
Contributor

Version

Minecraft: 1.19.4
SpongeAPI: 10.1.0-SNAPSHOT
Sponge: 1.19.4-10.1.0-SNAPSHOT
SpongeVanilla: 1.19.4-10.0.0-RC0

The bug

The deserialization of an enchanted item in a config file produces an item without its enchantments.
Reported by #3904.

Description

During the deserilization of an enchanted item, it will first create a CompoundTag from UnsafeData and then fix the enchantment data. The problem is that the enchantment ids in the CompoundTag are String which represent the resource location of an enchantment but they were retrieved as short. As a result, the modified CompoundTag contained enchantment ids with "0s" as value.
After that, if the enchantments were retrieved from the itemstack, the Optional was empty because it couldn't retrieved the enchantment type from the incorrect value "0s".

The fix

The fix is to retrieve and set as a string the enchantment id in the nbt and not as a short.

@aromaa aromaa added type: bug Something isn't working system: data version: 1.19 API: 10 labels Nov 2, 2023
Copy link
Member

@aromaa aromaa left a comment

Choose a reason for hiding this comment

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

Should be fine by just removing the enchantment id altogether as its a string and does not need fixup for its type.

final CompoundTag nbttagcompound = nbttaglist.getCompound(i);
final short lvl = nbttagcompound.getShort(Constants.Item.ITEM_ENCHANTMENT_LEVEL);

nbttagcompound.putShort(Constants.Item.ITEM_ENCHANTMENT_LEVEL, lvl);

@AlexandreArcil
Copy link
Contributor Author

Done !

Copy link
Member

@aromaa aromaa left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@aromaa aromaa merged commit f16405e into SpongePowered:api-10 Nov 5, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
system: data type: bug Something isn't working version: 1.19 API: 10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants