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

Add DamageType RegistryEvent #11783

Merged
merged 13 commits into from
Dec 27, 2024

Conversation

Chaosdave34
Copy link
Contributor

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.

@Chaosdave34 Chaosdave34 requested a review from a team as a code owner December 23, 2024 12:29
@kennytv kennytv added the type: feature Request for a new Feature. label Dec 23, 2024
Copy link
Member

@Machine-Maker Machine-Maker left a comment

Choose a reason for hiding this comment

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

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.

@lynxplay lynxplay force-pushed the feat/damage-type-registry-event branch from 76c8611 to 5917a72 Compare December 27, 2024 22:48
@lynxplay lynxplay merged commit 5c7537c into PaperMC:main Dec 27, 2024
@lynxplay
Copy link
Contributor

Totally missed it on my first review, but welcome to paper 🥳
Great first PR and thank you for your work!

@Chaosdave34 Chaosdave34 deleted the feat/damage-type-registry-event branch December 27, 2024 23:17
@Chaosdave34
Copy link
Contributor Author

Chaosdave34 commented Dec 27, 2024

Totally missed it on my first review, but welcome to paper 🥳 Great first PR and thank you for your work!

Tyvm!

Although you might missed this one.

@lynxplay
Copy link
Contributor

Well get congratulated twice then xD

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature Request for a new Feature.
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

5 participants