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

EnchantmentType - prepare for MC 1.21 custom enchantments #6687

Open
wants to merge 16 commits into
base: dev/feature
Choose a base branch
from

Conversation

ShaneBeee
Copy link
Contributor

@ShaneBeee ShaneBeee commented May 11, 2024

Description

This PR aims to prepare Skript for future changes with Enchantments.
Minecraft has moved enchantments to be data driven in 1.21.
This also means the ability to create custom enchantments.
This PR helps prepare Skript for that change by using the Bukkit registry rather than hard coding and using the specific fields.

Mojang is planning a 1.21 release for sometime this summer. I figured it would be best to prepare ourselves now.

NOTES:

  • parsing will first attempt to grab from lang file
  • if parsing fails from lang file, we move on to get from key from Registry (this allows for parsing custom enchantments)
  • toString will first attempt to find the a name from the lang file (ex: "Curse of Vanishing 1")
  • if that fails, will send the key for a vanilla enchantment (ex: "vanishing_curse") or the full namespace for custom enchants (ex: "my_thing:explosive")
  • Static Enchantment related methods/fields have been moved to EnchantmentUtils

Target Minecraft Versions: 1.21+
Requirements: none
Related Issues: none

@APickledWalrus
Copy link
Member

APickledWalrus commented May 13, 2024

I might prefer to keep support for custom names for Minecraft-provided enchantments. That is, one could write minecraft:vanishing_curse or curse of vanishing. This would keep the process automated but allow us to provide support for the typical representation of an enchantment's name. I'm not a super big fan of just vanishing_curse or vanishing curse (i think requiring the namespace is better when not using a Skript-provided name alias)

@ShaneBeee
Copy link
Contributor Author

I might prefer to keep support for custom names for Minecraft-provided enchantments. That is, one could write minecraft:vanishing_curse or curse of vanishing. This would keep the process automated but allow us to provide support for the typical representation of an enchantment's name. I'm not a super big fan of just vanishing_curse or vanishing curse (i think requiring the namespace is better when not using a Skript-provided name alias)

thats probably a good idea, for a little backwards compatibility too

@sovdeeth
Copy link
Member

Seconding pickle
Always have the key-based names, but also allow custom names via .lang

@ShaneBeee ShaneBeee marked this pull request as ready for review May 13, 2024 19:30
ShaneBeee and others added 2 commits May 13, 2024 13:04
Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>
Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>
- skip adding names if not in lang file
- toString should return full namespaced key if no name found
Copy link
Member

@APickledWalrus APickledWalrus 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. May be wise to wait on #6695 if that implementation can be used here instead.

final String[] names = Language.getList("enchantments." + key.getKey());

if (!names[0].startsWith("enchantments.")) {
NAMES.put(enchantment, names[0]);
Copy link
Member

Choose a reason for hiding this comment

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

We could also choose to store names for undefined enchantments in this map (e.g. the full namespaced key as done in toString)

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.

None yet

4 participants