From d48a26107c1c3152842da60a4d5193080ecfd1a7 Mon Sep 17 00:00:00 2001 From: Patrick Miller Date: Fri, 12 Jul 2024 11:52:40 -0400 Subject: [PATCH] ItemType Random Safety Improvements (#6886) --- .../java/ch/njol/skript/aliases/ItemType.java | 23 +++++++++++++++---- .../skript/classes/data/BukkitClasses.java | 5 +++- .../conditions/CondIsPreferredTool.java | 13 ++++++----- .../ch/njol/skript/effects/EffOpenBook.java | 2 +- .../ch/njol/skript/effects/EffReplace.java | 7 +++--- .../ch/njol/skript/entity/BoatChestData.java | 3 +-- .../java/ch/njol/skript/entity/BoatData.java | 5 ++-- .../njol/skript/entity/DroppedItemData.java | 11 ++++++++- .../njol/skript/expressions/ExprMaxStack.java | 9 ++++++-- .../ch/njol/skript/expressions/ExprPlain.java | 3 ++- .../skript/util/visual/VisualEffects.java | 7 ++++-- .../tests/regression/ExprPlainAliasTest.java | 2 +- 12 files changed, 63 insertions(+), 27 deletions(-) diff --git a/src/main/java/ch/njol/skript/aliases/ItemType.java b/src/main/java/ch/njol/skript/aliases/ItemType.java index efaa59249f2..d2a6fc85421 100644 --- a/src/main/java/ch/njol/skript/aliases/ItemType.java +++ b/src/main/java/ch/njol/skript/aliases/ItemType.java @@ -612,15 +612,27 @@ public ItemType clone() { .collect(Collectors.toList()); if (datas.isEmpty()) return null; - int numItems = datas.size(); - int index = random.nextInt(numItems); - ItemStack is = datas.get(index).getStack(); + ItemStack is = datas.get(random.nextInt(datas.size())).getStack(); assert is != null; // verified above is = is.clone(); is.setAmount(getAmount()); return is; } + /** + * @return One random ItemStack or Material that this ItemType represents. + * A Material may only be returned for ItemStacks containing a Material where {@link Material#isItem()} is false. + */ + public Object getRandomStackOrMaterial() { + ItemData randomData = types.get(random.nextInt(types.size())); + ItemStack stack = randomData.getStack(); + if (stack == null) + return randomData.getType(); + stack = stack.clone(); + stack.setAmount(getAmount()); + return stack; + } + /** * Test whether this ItemType can be put into the given inventory completely. *

@@ -1396,8 +1408,11 @@ public void clearItemMeta() { globalMeta = null; } + /** + * @return A random Material this ItemType represents. + */ public Material getMaterial() { - ItemData data = types.get(0); + ItemData data = types.get(random.nextInt(types.size())); if (data == null) throw new IllegalStateException("material not found"); return data.getType(); diff --git a/src/main/java/ch/njol/skript/classes/data/BukkitClasses.java b/src/main/java/ch/njol/skript/classes/data/BukkitClasses.java index 231d5ef9986..6da8522d035 100644 --- a/src/main/java/ch/njol/skript/classes/data/BukkitClasses.java +++ b/src/main/java/ch/njol/skript/classes/data/BukkitClasses.java @@ -949,7 +949,10 @@ public ItemStack parse(final String s, final ParseContext context) { } final ItemStack i = t.getRandom(); - assert i != null; + if (i == null) { + Skript.error("'" + s + "' cannot represent an item"); + return null; + } return i; } diff --git a/src/main/java/ch/njol/skript/conditions/CondIsPreferredTool.java b/src/main/java/ch/njol/skript/conditions/CondIsPreferredTool.java index 195e8240d93..f69a183adfd 100644 --- a/src/main/java/ch/njol/skript/conditions/CondIsPreferredTool.java +++ b/src/main/java/ch/njol/skript/conditions/CondIsPreferredTool.java @@ -80,13 +80,14 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye public boolean check(Event event) { return blocks.check(event, block -> items.check(event, item -> { - if (block instanceof Block) { - return ((Block) block).isPreferredTool(item.getRandom()); - } else if (block instanceof BlockData) { - return ((BlockData) block).isPreferredTool(item.getRandom()); - } else { - return false; + ItemStack stack = item.getRandom(); + if (stack != null) { + if (block instanceof Block) + return ((Block) block).isPreferredTool(stack); + if (block instanceof BlockData) + return ((BlockData) block).isPreferredTool(stack); } + return false; }), isNegated()); } diff --git a/src/main/java/ch/njol/skript/effects/EffOpenBook.java b/src/main/java/ch/njol/skript/effects/EffOpenBook.java index f49df089abe..95d82caf27e 100644 --- a/src/main/java/ch/njol/skript/effects/EffOpenBook.java +++ b/src/main/java/ch/njol/skript/effects/EffOpenBook.java @@ -68,7 +68,7 @@ protected void execute(final Event e) { ItemType itemType = book.getSingle(e); if (itemType != null) { ItemStack itemStack = itemType.getRandom(); - if (itemStack.getType() == Material.WRITTEN_BOOK) { + if (itemStack != null && itemStack.getType() == Material.WRITTEN_BOOK) { for (Player player : players.getArray(e)) { player.openBook(itemStack); } diff --git a/src/main/java/ch/njol/skript/effects/EffReplace.java b/src/main/java/ch/njol/skript/effects/EffReplace.java index f6a1fd5bad8..bf163106851 100644 --- a/src/main/java/ch/njol/skript/effects/EffReplace.java +++ b/src/main/java/ch/njol/skript/effects/EffReplace.java @@ -129,9 +129,10 @@ private void replace(Event event, Object[] needles, Expression haystackExpr) if (new ItemType(itemStack).isSimilar(needle)) { ItemStack newItemStack = ((ItemType) replacement).getRandom(); - newItemStack.setAmount(itemStack.getAmount()); - - inv.setItem(slot, newItemStack); + if (newItemStack != null) { + newItemStack.setAmount(itemStack.getAmount()); + inv.setItem(slot, newItemStack); + } } } } diff --git a/src/main/java/ch/njol/skript/entity/BoatChestData.java b/src/main/java/ch/njol/skript/entity/BoatChestData.java index bb29004028d..499a7ed3e44 100644 --- a/src/main/java/ch/njol/skript/entity/BoatChestData.java +++ b/src/main/java/ch/njol/skript/entity/BoatChestData.java @@ -110,8 +110,7 @@ public boolean isSupertypeOf(EntityData e) { public boolean isOfItemType(ItemType itemType) { int ordinal = -1; - ItemStack stack = itemType.getRandom(); - Material type = stack.getType(); + Material type = itemType.getMaterial(); if (type == Material.OAK_CHEST_BOAT) ordinal = 0; else if (type == Material.SPRUCE_CHEST_BOAT) diff --git a/src/main/java/ch/njol/skript/entity/BoatData.java b/src/main/java/ch/njol/skript/entity/BoatData.java index b273f2ca370..36bc23691d7 100644 --- a/src/main/java/ch/njol/skript/entity/BoatData.java +++ b/src/main/java/ch/njol/skript/entity/BoatData.java @@ -112,9 +112,8 @@ public boolean isSupertypeOf(EntityData e) { public boolean isOfItemType(ItemType i){ int ordinal = -1; - - ItemStack stack = i.getRandom(); - Material type = stack.getType(); + + Material type = i.getMaterial(); if (type == Material.OAK_BOAT) ordinal = 0; else if (type == Material.SPRUCE_BOAT) diff --git a/src/main/java/ch/njol/skript/entity/DroppedItemData.java b/src/main/java/ch/njol/skript/entity/DroppedItemData.java index e9b136407c1..aa7b4a6b8a4 100644 --- a/src/main/java/ch/njol/skript/entity/DroppedItemData.java +++ b/src/main/java/ch/njol/skript/entity/DroppedItemData.java @@ -64,8 +64,15 @@ public DroppedItemData(ItemType @Nullable [] types) { @Override protected boolean init(Literal[] expressions, int matchedPattern, ParseResult parseResult) { - if (expressions.length > 0 && expressions[0] != null) + if (expressions.length > 0 && expressions[0] != null) { types = (ItemType[]) expressions[0].getAll(); + for (ItemType type : types) { + if (!type.getMaterial().isItem()) { + Skript.error("'" + type + "' cannot represent a dropped item"); + return false; + } + } + } return true; } @@ -97,6 +104,7 @@ public void set(final Item entity) { final ItemType t = CollectionUtils.getRandom(types); assert t != null; ItemStack stack = t.getItem().getRandom(); + assert stack != null; // should be true by init checks entity.setItemStack(stack); } @@ -159,6 +167,7 @@ public boolean canSpawn(@Nullable World world) { final ItemType itemType = CollectionUtils.getRandom(types); assert itemType != null; ItemStack stack = itemType.getItem().getRandom(); + assert stack != null; // should be true by init checks Item item; if (consumer == null) { diff --git a/src/main/java/ch/njol/skript/expressions/ExprMaxStack.java b/src/main/java/ch/njol/skript/expressions/ExprMaxStack.java index 71abeb61b7d..77b65fa5191 100644 --- a/src/main/java/ch/njol/skript/expressions/ExprMaxStack.java +++ b/src/main/java/ch/njol/skript/expressions/ExprMaxStack.java @@ -24,6 +24,8 @@ import ch.njol.skript.doc.Name; import ch.njol.skript.doc.Since; import ch.njol.skript.expressions.base.SimplePropertyExpression; +import org.bukkit.Material; +import org.bukkit.inventory.ItemStack; /** * @author joeuguce99 @@ -40,8 +42,11 @@ public class ExprMaxStack extends SimplePropertyExpression { @SuppressWarnings("null") @Override - public Long convert(final ItemType i) { - return (long) i.getRandom().getMaxStackSize(); + public Long convert(ItemType itemType) { + Object random = itemType.getRandomStackOrMaterial(); + if (random instanceof Material) + return (long) ((Material) random).getMaxStackSize(); + return (long) ((ItemStack) random).getMaxStackSize(); } @Override diff --git a/src/main/java/ch/njol/skript/expressions/ExprPlain.java b/src/main/java/ch/njol/skript/expressions/ExprPlain.java index ca73ba22820..67fb7ca3086 100644 --- a/src/main/java/ch/njol/skript/expressions/ExprPlain.java +++ b/src/main/java/ch/njol/skript/expressions/ExprPlain.java @@ -32,6 +32,7 @@ import ch.njol.util.Kleenean; import org.bukkit.event.Event; +import org.bukkit.inventory.ItemStack; import org.eclipse.jdt.annotation.Nullable; @Name("Plain Item") @@ -61,7 +62,7 @@ protected ItemType[] get(Event e) { ItemType itemType = item.getSingle(e); if (itemType == null) return new ItemType[0]; - ItemData data = new ItemData(itemType.getRandom().getType()); + ItemData data = new ItemData(itemType.getMaterial()); data.setPlain(true); ItemType plain = new ItemType(data); return new ItemType[]{plain}; diff --git a/src/main/java/ch/njol/skript/util/visual/VisualEffects.java b/src/main/java/ch/njol/skript/util/visual/VisualEffects.java index dd9d31456c9..e8f4beb7333 100644 --- a/src/main/java/ch/njol/skript/util/visual/VisualEffects.java +++ b/src/main/java/ch/njol/skript/util/visual/VisualEffects.java @@ -151,8 +151,11 @@ private static void registerDataSupplier(String id, BiFunction