Add DamageType RegistryEvent

[Chaosdave34]

Add RegistryEvent for DamageType

Changes:

• Add DamageTypeRegistryEntry
• Add PaperDamageTypeRegistryEntry
• Make DamageType PaperRegistry writeable
• Add DamageType Event to RegistryEvents

But I ran into the problem, that DamageEffect uses Bukkit.getUnsafe() for initialization and that seems to be not available at bootstrap. I fixed that with the following changes, but I am not sure if that's the correct way to do is as it may break plugins:

• Turn DamageType interface into an enum
• Add CraftDamageType damageEffectToBukkit() and damageEffectToNMS() to CraftDamageType
• Add getSoundForDamageEffect() to UnsafeValues (and CraftMagicNumbers)
• Deprecated CraftDamageEffect, UnsafeValues#getDamageEffect() and DamageEffect#getDamageEffect()

The only breaking change I see is when someone tries to cast DamageType to CraftDamageType which is no longer possible with my approach.

[masmc05]

Why downgrade a perfectly working interface to enum? Not only this brings a lot of issues that both spigot and paper are fighting right now, but it breaks plugins without any added rewriting in this pr.

[Chaosdave34]

Using an enum instead of an interface was the first idea that came to my mind. I know that it comes with the mentioned problems.

Maybe use the id of the damage effect as String instead of the Bukkit Interface for DamageTypeRegistryEntry?

[masmc05]

Yeah, either other form of reference, or trying to resolve it not using CraftServer.

I think a pretty valid solution could be providing another access to unsafe values other than trough craft server. There was already a discussion of making some static access method in a pr, including to get the data and protocol versions in bootstrap, but that died down. Basically a provider class, implementation of that, retrieve it with https://www.baeldung.com/java-spi and hold it in a local class within the static getter method in UnsafeValues class

[lynxplay]

Agree with that sentiment ^
Making this an enum is not something I'd like to see.
We'll end up with some version of unsafe values at bootstrap time down the line anyway so, yea.

Alternatively, we can attempt to predict that damage effects might be a registry value at some point?
Which would mean we could just move this to keys instead of the actual resolved value.
That would need a generated key class tho.
I'd personally be semi in favour of that approach as it means we won't have to break API there if that ever changes and it does get rid of the bootstrap issue. Will poke MM about it tho for his ideas.

[Chaosdave34]

Maybe this could be a solution.

Additionally instead of having an extra class (StaticUnsafeValues), Bukkit's UnsafeValues interface could have a static getter.
The point where the provider is initialized is probably not the best, but it's loaded before the plugin bootstrapper runs.

[Machine-Maker]

I want a separate type called something like “APIBridge” that uses the service loader API for stuff like this, so we can move actual API out of UnsafeValues. UnsafeValues should be for actual “unsafe” api, not just a place to store “bridge” methods.

But either way, yeah, this shouldn’t be changed back to an enum.

[Machine-Maker]

As for the StaticUnsafeValues, I have a slightly different idea on how to solve it, but it can just be left in for now, not a big deal.

There should be a standardized way to interact with the "pseudo-registries" types of which DamageEffect is part of. All the types on the server that implement StringRepresentable are a different sort of registry where there is a defined mapping between a string and an instance. There will be/already are other cases where a way to convert between them would be useful if we don't want to use an enum on the API side.

But this isn't something you have to figure out, that'll be for us to decide on.