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

Missing new 1.16.0 enchantment #325

Closed
IdanHo opened this issue Aug 10, 2020 · 10 comments
Closed

Missing new 1.16.0 enchantment #325

IdanHo opened this issue Aug 10, 2020 · 10 comments

Comments

@IdanHo
Copy link
Member

IdanHo commented Aug 10, 2020

No new enchantments were added since 1.13.2 (and as such no version since 1.13.2 had an enchantments.json file), and as such were missing the new Soul Speed enchant added in 1.16.0

@nickelpro
Copy link
Member

No version since 1.13.2 has an enchantments field included in dataPaths.json at all. It's more correct to say no version since 1.13.2 has ported the enchantment data forward at all, largely because it's all manual work and enchantment data is little used. This was attempted to be addressed by #287 and the associated PR on the wiki extractor, but the wiki doesn't provide all needed data for the MCD schema.

The immediate issue of hand porting enchantments.json forward for 1.14+ is easy and I assume Rom will accept a PR if you do that. The more complete answer is to write a new topping for Burger that extracts this data.

@nickelpro
Copy link
Member

I've identified the class for extracting the data and intend to take a wack at building the burger extractor sometime this week when I have a minute. As long as we're doing this we can grab some other stuff MCD doesn't track like rarity and equipment slot.

@rom1504 Is it ok to add schema fields as long as we maintain old ones?

@IdanHo
Copy link
Member Author

IdanHo commented Aug 11, 2020

I'd be happy to open a pull request for the enchantments myself, but i can wait as well for the extractor if youd like.

@nickelpro
Copy link
Member

If it's not urgent, give me two days and I should have something working. It's a little harder than the two-line extractor you'd want it to be because MC changed the way these pseudo-enumerated types worked around 1.13, so getting support for all the versions Burger works on can be a little tricky.

@rom1504
Copy link
Member

rom1504 commented Aug 11, 2020 via email

@nickelpro
Copy link
Member

nickelpro commented Aug 13, 2020

So I've run into a little bit of a snag

The sequential Window IDs for enchantment are not the same as the deprecated NBT IDs for enchantments. For example in 1.13.2 the NBT ID for "punch" is 49, but the Window ID is 23. The NBT IDs are completely deprecated and any assignement in later versions will be arbitrary, and in earlier versions we need to add a "window_id" field. The Soul Speed enchantment is a good example, it has no NBT ID field listed anywhere in the jar since it came about after they were removed.

What's the best way to indicate a deprecated field in MCD? Leave it blank? Set to zero? Remove it entirely?

@rom1504
Copy link
Member

rom1504 commented Aug 13, 2020

any assignement in later versions will be arbitrary

What do you mean by arbitrary, do they still use them ?

@nickelpro
Copy link
Member

nickelpro commented Aug 13, 2020

No, the NBT IDs are not used at all in later versions. They only exist in "Fix" classes that map old enchantment data to the new format, for updating existing world files. Soul Speed did not exist in old versions and therefore doesn't even have an ID listed there.

@rom1504
Copy link
Member

rom1504 commented Aug 13, 2020

Ok then add both fields and fill them only for the versions where it makes sense

@Karang
Copy link
Contributor

Karang commented Jan 16, 2021

done

@Karang Karang closed this as completed Jan 16, 2021
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

No branches or pull requests

4 participants