ExprTool - Cleanup & Additional Test

src/main/java/ch/njol/skript/bukkitutil/BukkitUtils.java

@@ -1,7 +1,10 @@
 package ch.njol.skript.bukkitutil;
 
 import ch.njol.skript.Skript;
+import com.google.common.collect.BiMap;
+import com.google.common.collect.HashBiMap;
 import org.bukkit.Registry;
+import org.bukkit.inventory.EquipmentSlot;
 import org.bukkit.potion.PotionEffectType;
 import org.jetbrains.annotations.Nullable;
 
@@ -10,6 +13,16 @@
  */
 public class BukkitUtils {
 
+	private static final BiMap<EquipmentSlot, Integer> BUKKIT_EQUIPMENT_SLOT_INDICES = HashBiMap.create();
+
+	static {
+		BUKKIT_EQUIPMENT_SLOT_INDICES.put(EquipmentSlot.FEET, 36);
+		BUKKIT_EQUIPMENT_SLOT_INDICES.put(EquipmentSlot.LEGS, 37);
+		BUKKIT_EQUIPMENT_SLOT_INDICES.put(EquipmentSlot.CHEST, 38);
+		BUKKIT_EQUIPMENT_SLOT_INDICES.put(EquipmentSlot.HEAD, 39);
+		BUKKIT_EQUIPMENT_SLOT_INDICES.put(EquipmentSlot.OFF_HAND, 40);
+	}
+
 	/**
 	 * Check if a registry exists
 	 *
@@ -36,4 +49,22 @@ public static boolean registryExists(String registry) {
 		return null;
 	}
 
+	/**
+	 * Get the inventory slot index of the {@link EquipmentSlot}
+	 * @param equipmentSlot The equipment slot to get the index of
+	 * @return The equipment slot index of the provided slot, otherwise null if invalid
+	 */
+	public static Integer getEquipmentSlotIndex(EquipmentSlot equipmentSlot) {
+		return  BUKKIT_EQUIPMENT_SLOT_INDICES.get(equipmentSlot);
+	}
+
+	/**
+	 * Get the {@link EquipmentSlot} represented by the inventory slot index
+	 * @param slotIndex The index of the equipment slot
+	 * @return The equipment slot the provided slot index, otherwise null if invalid
+	 */
+	public static EquipmentSlot getEquipmentSlotFromIndex(int slotIndex) {
+		return BUKKIT_EQUIPMENT_SLOT_INDICES.inverse().get(slotIndex);
+	}
+
 }

src/main/java/ch/njol/skript/expressions/ExprTool.java

@@ -1,73 +1,76 @@
 package ch.njol.skript.expressions;
 
 import ch.njol.skript.Skript;
-import ch.njol.skript.doc.Description;
-import ch.njol.skript.doc.Examples;
-import ch.njol.skript.doc.Name;
-import ch.njol.skript.doc.Since;
+import ch.njol.skript.bukkitutil.BukkitUtils;
+import ch.njol.skript.doc.*;
 import ch.njol.skript.effects.Delay;
 import ch.njol.skript.expressions.base.PropertyExpression;
 import ch.njol.skript.lang.Expression;
 import ch.njol.skript.lang.ExpressionType;
 import ch.njol.skript.lang.SkriptParser.ParseResult;
+import ch.njol.skript.lang.SyntaxStringBuilder;
 import ch.njol.skript.registrations.Classes;
+import ch.njol.skript.registrations.EventValues;
 import ch.njol.skript.util.slot.EquipmentSlot;
 import ch.njol.skript.util.slot.InventorySlot;
 import ch.njol.skript.util.slot.Slot;
 import ch.njol.util.Kleenean;
 import org.bukkit.entity.LivingEntity;
 import org.bukkit.event.Event;
-import org.bukkit.event.player.PlayerBucketEmptyEvent;
 import org.bukkit.event.player.PlayerBucketEvent;
-import org.bukkit.event.player.PlayerBucketFillEvent;
 import org.bukkit.event.player.PlayerItemHeldEvent;
 import org.bukkit.inventory.EntityEquipment;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.PlayerInventory;
 import org.jetbrains.annotations.Nullable;
 
-/**
- * @author Peter Güttinger
- */
 @Name("Tool")
 @Description("The item an entity is holding in their main or off hand.")
-@Examples({"player's tool is a pickaxe",
-	"player's off hand tool is a shield",
-	"set tool of all players to a diamond sword",
-	"set offhand tool of target entity to a bow"})
+@Example("player's tool is a pickaxe")
+@Example("player's off hand tool is a shield")
+@Example("set tool of all players to a diamond sword")
+@Example("set offhand tool of target entity to a bow")
 @Since("1.0")
 public class ExprTool extends PropertyExpression<LivingEntity, Slot> {
+
 	static {
 		Skript.registerExpression(ExprTool.class, Slot.class, ExpressionType.PROPERTY,
-			"[the] ((tool|held item|weapon)|1¦(off[ ]hand (tool|item))) [of %livingentities%]",
-			"%livingentities%'[s] ((tool|held item|weapon)|1¦(off[ ]hand (tool|item)))");
+			"[the] (tool|held item|weapon) [of %livingentities%]",
+			"%livingentities%'[s] (tool|held item|weapon)",
+			"[the] off[ ]hand (tool|item) [of %livingentities%]",
+			"%livingentities%'[s] off[ ]hand (tool|item)"
+		);
 	}
 
 	private boolean offHand;
 
-	@SuppressWarnings({"unchecked", "null"})
 	@Override
 	public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parser) {
+		//noinspection unchecked
 		setExpr((Expression<LivingEntity>) exprs[0]);
-		offHand = parser.mark == 1;
+		offHand = matchedPattern >= 2;
 		return true;
 	}
 
 	@Override
 	protected Slot[] get(Event event, LivingEntity[] source) {
-		final boolean delayed = Delay.isDelayed(event);
+		boolean delayed = Delay.isDelayed(event);
 		return get(source, entity -> {
 			if (!delayed) {
 				if (!offHand && event instanceof PlayerItemHeldEvent playerItemHeldEvent
 					&& playerItemHeldEvent.getPlayer() == entity) {
-					final PlayerInventory i = playerItemHeldEvent.getPlayer().getInventory();
-					return new InventorySlot(i, getTime() >= 0 ? playerItemHeldEvent.getNewSlot() : playerItemHeldEvent.getPreviousSlot());
+
+					PlayerInventory inventory = playerItemHeldEvent.getPlayer().getInventory();
+					return new InventorySlot(inventory, getTime() >= 0 ? playerItemHeldEvent.getNewSlot() : playerItemHeldEvent.getPreviousSlot());
+
 				} else if (event instanceof PlayerBucketEvent playerBucketEvent
 					&& playerBucketEvent.getPlayer() == entity) {
-					final PlayerInventory i = playerBucketEvent.getPlayer().getInventory();
-					boolean isOffHand = playerBucketEvent.getHand() == org.bukkit.inventory.EquipmentSlot.OFF_HAND || offHand;
-					return new InventorySlot(i, isOffHand ? EquipmentSlot.EquipSlot.OFF_HAND.slotNumber
-						: playerBucketEvent.getPlayer().getInventory().getHeldItemSlot()) {
+
+					PlayerInventory inventory = playerBucketEvent.getPlayer().getInventory();
+					boolean isOffHand = offHand || playerBucketEvent.getHand() == org.bukkit.inventory.EquipmentSlot.OFF_HAND;
+					int inventorySlot = isOffHand ? BukkitUtils.getEquipmentSlotIndex(org.bukkit.inventory.EquipmentSlot.OFF_HAND)
+						: inventory.getHeldItemSlot();
+					return new InventorySlot(inventory, inventorySlot) {
 						@Override
 						public ItemStack getItem() {
 							return getTime() <= 0 ? super.getItem() : playerBucketEvent.getItemStack();
@@ -91,10 +94,15 @@ public void setItem(final @Nullable ItemStack item) {
 			return new EquipmentSlot(equipment, offHand ? org.bukkit.inventory.EquipmentSlot.OFF_HAND : org.bukkit.inventory.EquipmentSlot.HAND) {
 				@Override
 				public String toString(@Nullable Event event, boolean debug) {
-					String time = getTime() == 1 ? "future " : getTime() == -1 ? "former " : "";
-					String hand = offHand ? "off hand" : "";
-					String item = Classes.toString(getItem());
-					return String.format("%s %s tool of %s", time, hand, item);
+					SyntaxStringBuilder syntaxBuilder = new SyntaxStringBuilder(event, debug);
+					switch (getTime()) {
+						case EventValues.TIME_FUTURE -> syntaxBuilder.append("future");
+						case EventValues.TIME_PAST -> syntaxBuilder.append("former");
+					}
+					if (offHand)
+						syntaxBuilder.append("off hand");
+					syntaxBuilder.append("tool of", Classes.toString(getItem()));
+					return syntaxBuilder.toString();
 				}
 			};
 		});
@@ -106,15 +114,17 @@ public Class<Slot> getReturnType() {
 	}
 
 	@Override
-	public String toString(final @Nullable Event e, final boolean debug) {
-		String hand = offHand ? "off hand" : "";
-		return String.format("%s tool of %s", hand, getExpr().toString(e, debug));
+	public String toString(@Nullable Event event, boolean debug) {
+		SyntaxStringBuilder syntaxBuilder = new SyntaxStringBuilder(event, debug);
+		if (offHand)
+			syntaxBuilder.append("off hand");
+		syntaxBuilder.append("tool of", getExpr());
+		return syntaxBuilder.toString();
 	}
 
-	@SuppressWarnings("unchecked")
 	@Override
-	public boolean setTime(final int time) {
-		return super.setTime(time, getExpr(), PlayerItemHeldEvent.class, PlayerBucketFillEvent.class, PlayerBucketEmptyEvent.class);
+	public boolean setTime(int time) {
+		return super.setTime(time, getExpr(), PlayerItemHeldEvent.class, PlayerBucketEvent.class);
 	}
 
 }

src/main/java/ch/njol/skript/util/slot/EquipmentSlot.java

@@ -1,5 +1,6 @@
 package ch.njol.skript.util.slot;
 
+import ch.njol.skript.bukkitutil.BukkitUtils;
 import ch.njol.skript.bukkitutil.PlayerUtils;
 import ch.njol.skript.lang.SyntaxStringBuilder;
 import ch.njol.skript.registrations.Classes;
@@ -13,9 +14,7 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.util.HashMap;
 import java.util.Locale;
-import java.util.Map;
 
 /**
  * Represents equipment slot of an entity.
@@ -123,22 +122,6 @@ public void set(EntityEquipment entityEquipment, @Nullable ItemStack item) {
 
 	}
 
-	private static final Map<org.bukkit.inventory.EquipmentSlot, Integer> BUKKIT_SLOT_INDICES = new HashMap<>();
-	private static final Map<Integer, org.bukkit.inventory.EquipmentSlot> BUKKIT_SLOT_INDICES_REVERSED = new HashMap<>();
-
-	static {
-		BUKKIT_SLOT_INDICES.put(org.bukkit.inventory.EquipmentSlot.FEET, 36);
-		BUKKIT_SLOT_INDICES.put(org.bukkit.inventory.EquipmentSlot.LEGS, 37);
-		BUKKIT_SLOT_INDICES.put(org.bukkit.inventory.EquipmentSlot.CHEST, 38);
-		BUKKIT_SLOT_INDICES.put(org.bukkit.inventory.EquipmentSlot.HEAD, 39);
-		BUKKIT_SLOT_INDICES.put(org.bukkit.inventory.EquipmentSlot.OFF_HAND, 40);
-		BUKKIT_SLOT_INDICES_REVERSED.put(36, org.bukkit.inventory.EquipmentSlot.FEET);
-		BUKKIT_SLOT_INDICES_REVERSED.put(37, org.bukkit.inventory.EquipmentSlot.LEGS);
-		BUKKIT_SLOT_INDICES_REVERSED.put(38, org.bukkit.inventory.EquipmentSlot.CHEST);
-		BUKKIT_SLOT_INDICES_REVERSED.put(39, org.bukkit.inventory.EquipmentSlot.HEAD);
-		BUKKIT_SLOT_INDICES_REVERSED.put(40, org.bukkit.inventory.EquipmentSlot.OFF_HAND);
-	}
-
 	private final EntityEquipment entityEquipment;
 	private EquipSlot skriptSlot;
 	private final int slotIndex;
@@ -192,12 +175,7 @@ public EquipmentSlot(@NotNull EntityEquipment equipment, @NotNull org.bukkit.inv
 	}
 
 	public EquipmentSlot(@NotNull HumanEntity holder, int index) {
-		/*
-		 * slot: 6 entries in EquipSlot, indices descending
-		 *  So this math trick gets us the EquipSlot from inventory slot index
-		 * slotToString: Referring to numeric slot id, right?
-		 */
-		this(holder.getEquipment(), BUKKIT_SLOT_INDICES_REVERSED.get(index), true);
+		this(holder.getEquipment(), BukkitUtils.getEquipmentSlotFromIndex(index), true);
 	}
 
 	@Override
@@ -206,7 +184,7 @@ public EquipmentSlot(@NotNull HumanEntity holder, int index) {
 			return skriptSlot.get(entityEquipment);
 		return entityEquipment.getItem(bukkitSlot);
 	}
-	
+
 	@Override
 	public void setItem(@Nullable ItemStack item) {
 		if (skriptSlot != null) {
@@ -217,21 +195,21 @@ public void setItem(@Nullable ItemStack item) {
 		if (entityEquipment.getHolder() instanceof Player player)
 			PlayerUtils.updateInventory(player);
 	}
-	
+
 	@Override
 	public int getAmount() {
 		ItemStack item = getItem();
 		return item != null ? item.getAmount() : 0;
 	}
-	
+
 	@Override
 	public void setAmount(int amount) {
 		ItemStack item = getItem();
 		if (item != null)
 			item.setAmount(amount);
 		setItem(item);
 	}
-	
+
 	/**
 	 * @deprecated Use {@link EquipmentSlot#EquipmentSlot(EntityEquipment, org.bukkit.inventory.EquipmentSlot)} and {@link #getEquipmentSlot()}
 	 */
@@ -255,8 +233,8 @@ public int getIndex() {
 			return slotIndex;
 		} else if (skriptSlot != null) {
 			return skriptSlot.slotNumber;
-		} else if (BUKKIT_SLOT_INDICES.containsKey(bukkitSlot)) {
-			return BUKKIT_SLOT_INDICES.get(bukkitSlot);
+		} else if (BukkitUtils.getEquipmentSlotIndex(bukkitSlot) != null) {
+			return BukkitUtils.getEquipmentSlotIndex(bukkitSlot);
 		}
 		return -1;
 	}
@@ -276,5 +254,5 @@ public String toString(@Nullable Event event, boolean debug) {
 		}
 		return Classes.toString(getItem());
 	}
-	
+
 }

src/test/skript/tests/syntaxes/expressions/ExprTool.sk

@@ -0,0 +1,41 @@
+test "ExprTool":
+	spawn a skeleton at test location:
+		set helmet of entity to diamond helmet # preventing death from sun
+		set {_skeleton} to entity
+
+	# Main Hand
+
+	set tool of {_skeleton} to a diamond
+	assert tool of {_skeleton} is a diamond with "The held item of the skeleton wasn't set to a diamond"
+
+	add 3 diamonds to the tool of {_skeleton}
+	assert weapon of {_skeleton} is 4 diamonds with "The held item of the skeleton didn't become 4 diamonds"
+
+	remove stone from the held item of {_skeleton}
+	assert held item of {_skeleton} is 4 diamonds with "The held item of the skeleton was no longer 4 diamonds after removing a different item"
+
+	remove 2 diamonds from the tool of {_skeleton}
+	assert tool of {_skeleton} is 2 diamonds with "The held item of the skeleton did not become 2 diamonds after removing 2 diamonds from 4"
+
+	clear tool of {_skeleton}
+	assert tool of {_skeleton} is air with "The held item of the skeleton did not become air after being cleared"
+
+	# Off Hand
+
+	set offhand tool of {_skeleton} to a diamond
+	assert offhand tool of {_skeleton} is a diamond with "The offhand item of the skeleton wasn't set to a diamond"
+
+	add 3 diamonds to the offhand tool of {_skeleton}
+	assert offhand tool of {_skeleton} is 4 diamonds with "The offhand item of the skeleton didn't become 4 diamonds"
+
+	remove stone from the offhand item of {_skeleton}
+	assert offhand tool of {_skeleton} is 4 diamonds with "The offhand item of the skeleton was no longer 4 diamonds after removing an incorrect item"
+
+	remove 2 diamonds from the offhand tool of {_skeleton}
+	assert offhand tool of {_skeleton} is 2 diamonds with "The offhand item of the skeleton did not become 2 diamonds after removing 2 diamonds from 4"
+
+	clear offhand item of {_skeleton}
+	assert offhand tool of {_skeleton} is air with "The offhand item of the skeleton did not become air after being cleared"
+
+	# Cleanup
+	delete entity within {_skeleton}