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

Update for nullable spawner types, editMeta util #2514

Merged
merged 11 commits into from Aug 5, 2023

Conversation

tal5
Copy link
Member

@tal5 tal5 commented Aug 4, 2023

Changes

  • Updated ItemSpawnerType to modern property format, and changed it to account for the spawner type being nullable internally on 1.20+ (previously just threw an NPE), including support for setting it to null in the mechanism.
  • Updated LocationTag.spawner_type for the type being nullable, including updating the mech to modern registration.
    This also includes a workaround (makeBlockStateRaw) for setting it to null in the mechanism due to a Spigot bug - I have already opened an issue @ Spigot for this this has since been fixed.
  • (Unrelated cleanup) Cleaned up ItemInstrument:
    • Removed the allows custom instruments from the meta, as Spigot uses hard-coded ones :/
    • The meta now makes sense for both the tag & mech.
    • Documented it being nullable, and how Minecraft defaults to ponder when unset (which is effectively random because it's based on the internal order of stuff - it grabs the first entry from the horn instruments tag/the instrument registry).
    • Added support for setting it to null.
    • Removed getMusicInstrument in favor of just getting it directly.
    • Removed setMusicInstrument in favor of the new ItemProperty#editMeta.
    • Changed setPropertyValue's param name to match the usual one (param -> value).
    • Removed a redundant ElementTag#asString call in setPropertyValue's error message, and slightly cleaned it up.
    • Changed describes into a instanceof check instead of a material check.

Additions

  • ItemProperty#editMeta(Class metaType, Consumer editor) - a util method to avoid boilerplate get/setItemMeta calls & casts.

mechanism.echoError("Mechanism 'LocationTag.spawner_type' is only valid for spawners.");
return;
}
if (mechanism.value == null && NMSHandler.getVersion().isAtLeast(NMSVersion.v1_20)) {
Copy link
Member

Choose a reason for hiding this comment

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

!mechanism.hasValue()

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@@ -33,4 +35,10 @@ public void setItemStack(ItemStack item) {
public void setItemMeta(ItemMeta meta) {
object.setItemMeta(meta);
}

public <T extends ItemMeta> void editMeta(Class<T> metaType, Consumer<T> editor) {
T meta = metaType.cast(getItemMeta());
Copy link
Member

Choose a reason for hiding this comment

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

Is there a specific reason to use this over just (T)? I ask specifically because I generally worry about reflection calls having unexpected performance issues, so it's preferable to avoid if not needed

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed to a direct cast

@mcmonkey4eva mcmonkey4eva merged commit 890a0b6 into DenizenScript:dev Aug 5, 2023
1 check passed
@tal5 tal5 deleted the Fix_ItemTag_Spawner_Type branch August 5, 2023 20:18
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

2 participants