-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Fix enchantment init #11624
base: master
Are you sure you want to change the base?
Fix enchantment init #11624
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is potentially going to be fun if nothing is expecting those fields to be nullable, but, the reality here is that this is somewhat unavoidable
There is one concern that removing enchantments will break even in vanilla somewhere down the line, as is the case with many other client synchronized registries making the client crash once it does try to reference individual ones somewhere in game logic |
hmm exists a form to check this? i mean the datapack allow make this in vanilla if a issue happen is more a report for the MOJIRA? if they allow make this |
Okay for enchantments specifically I don't see such use, so it might be okay to remove them as long as all loot tables etc. that reference the enchantments are removed by the datapack? but it makes for weird api here, if we merge this we just have to make sure it's very clear they can be null |
At least my two cents on this. Additionally, removing vanilla enchants is also a rather edge-case imo. More opinions obviously needed tho. |
Fixs #11623 like Tags enchantments (and pretty sure many other Registry things) can be removed by datapacks and how CrafBukkit (for legacy) expects vanilla ever exists can cause issues...
This PR just make the Vanillla Enchantments can be null in the Bukkit Enchantment class also mention in docs about this (more "easy" than add Nullable to all the static instances of Enchantment with the same issue (or maybe this can be better)