Trial Spawner and Vaults 1.21 additions #2790
Conversation
| // | ||
| // @Location true | ||
| // | ||
| // @Triggers when a vault block state changes |
There was a problem hiding this comment.
Needs to be block's or block changes state.
Also . at the end.
paper/src/main/java/com/denizenscript/denizen/paper/events/VaultChangesStateScriptEvent.java
Show resolved
Hide resolved
| public BlockDispenseLootScriptEvent() { | ||
| registerCouldMatcher("loot dispenses from <block>"); | ||
| this.<BlockDispenseLootScriptEvent, ListTag>registerDetermination(null, ListTag.class, (evt, context, input) -> { | ||
| List<ItemStack> items = new ArrayList<>(); |
There was a problem hiding this comment.
Nitpick, but can initialize to input.size().
| @Override | ||
| public ObjectTag getContext(String name) { | ||
| return switch (name) { | ||
| case "item" -> item; |
There was a problem hiding this comment.
This has the same issue with the context potentially becoming outdated due to another plugin accessing it (e.g. if you - wait a bit in a script and then read it) - need to dynamically get the item here, but can keep the field for matches optimization.
|
|
||
| public BlockDispenseLootScriptEvent() { | ||
| registerCouldMatcher("loot dispenses from <block>"); | ||
| this.<BlockDispenseLootScriptEvent, ListTag>registerDetermination(null, ListTag.class, (evt, context, input) -> { |
There was a problem hiding this comment.
Just in general, prefixed determinations (to match their context counterparts) are usually preferred as they're easier to understand in scripts & look cleaner if the event ever gets more than one determination.
| || data instanceof Tripwire | ||
| || (NMSHandler.getVersion().isAtLeast(NMSVersion.v1_19) && (data instanceof SculkCatalyst | ||
| || data instanceof SculkShrieker)); | ||
| || data instanceof SculkShrieker)) |
| else if (isSculkCatalyst()) { | ||
| return ((SculkCatalyst) material.getModernData()).isBloom() ? "BLOOM" : "NORMAL"; // TODO: 1.19 | ||
| else if (NMSHandler.getVersion().isAtLeast(NMSVersion.v1_21) && getBlockData() instanceof TrialSpawner trialSpawner) { | ||
| return new ElementTag(trialSpawner.getTrialSpawnerState().name(), true); |
There was a problem hiding this comment.
Can use the enum constructor for these
| // | ||
| // @Location true | ||
| // | ||
| // @Triggers when a vault block's state changes. A list of states can be found at <@link url https://hub.spigotmc.org/javadocs/spigot/org/bukkit/block/data/type/TrialSpawner.State.html>. |
| import org.bukkit.event.EventHandler; | ||
| import org.bukkit.event.Listener; | ||
|
|
||
| public class VaultChangeStateScriptEvent extends BukkitScriptEvent implements Listener { |
There was a problem hiding this comment.
Nitpick, but the event is called changes and this says change.
| if (!runInCheck(path, location)) { | ||
| return false; | ||
| } | ||
| if (!path.tryArgObject(2, new ItemTag(event.getDisplayItem()))) { |
There was a problem hiding this comment.
This could be put in a field for matches optimization (getContext should still access it directly though).
plugin/src/main/java/com/denizenscript/denizen/objects/properties/entity/EntityItem.java
Show resolved
Hide resolved
| this.<VaultDisplayItemScriptEvent, ItemTag>registerDetermination(null, ItemTag.class, (evt, context, input) -> { | ||
| evt.event.setDisplayItem(input.getItemStack()); | ||
| }); |
There was a problem hiding this comment.
Generally prefixed determinations are preferred - easier to read/work with in code, and easier to manage for us when more get added.
| // @Triggers when a block dispenses loot containing multiple items. | ||
| // | ||
| // @Context | ||
| // <context.loot> returns a ListTag(ItemTag) of outcome items. |
There was a problem hiding this comment.
outcome here sounds a little weird imo? I'd just say loot items maybe, or something like items being dispensed if you want to be more explicit.
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class BlockDispenseLootScriptEvent extends BukkitScriptEvent implements Listener { |
There was a problem hiding this comment.
I believe the class name usually matches the Denizen name over the bukkit name, so that it's easier to spot in code for us.
2 participants
Replaces #2771
Additions:
Updates:
Some of this was copied from @MrMaleficus's earlier PR, so credit to them for the events.