Llama+TraderLlama Fix & ExprSpawnerType

src/main/java/ch/njol/skript/entity/LlamaData.java

@@ -1,32 +1,25 @@
 package ch.njol.skript.entity;
 
+import ch.njol.skript.lang.Literal;
+import ch.njol.skript.lang.SkriptParser.ParseResult;
+import ch.njol.util.coll.CollectionUtils;
 import org.bukkit.entity.Llama;
 import org.bukkit.entity.Llama.Color;
 import org.bukkit.entity.TraderLlama;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import ch.njol.skript.Skript;
-import ch.njol.skript.lang.Literal;
-import ch.njol.skript.lang.SkriptParser.ParseResult;
-import ch.njol.util.coll.CollectionUtils;
-
 public class LlamaData extends EntityData<Llama> {
-
-	private final static boolean TRADER_SUPPORT = Skript.classExists("org.bukkit.entity.TraderLlama");
+
+	private static final Color[] LLAMA_COLORS = Color.values();
+
 	static {
-		if (TRADER_SUPPORT)
-			EntityData.register(LlamaData.class, "llama", Llama.class, 0,
-					"llama", "creamy llama", "white llama", "brown llama", "gray llama",
-				"trader llama", "creamy trader llama", "white trader llama", "brown trader llama", "gray trader llama");
-		else if (Skript.classExists("org.bukkit.entity.Llama"))
-			EntityData.register(LlamaData.class, "llama", Llama.class, 0,
-					"llama", "creamy llama",
-					"white llama", "brown llama", "gray llama");
+		EntityData.register(LlamaData.class, "llama", Llama.class, 0,
+			"llama", "creamy llama", "white llama", "brown llama", "gray llama",
+			"trader llama", "creamy trader llama", "white trader llama", "brown trader llama", "gray trader llama");
 	}
-
-	@Nullable
-	private Color color = null;
+
+	private @Nullable Color color = null;
 	private boolean isTrader;
 
 	public LlamaData() {}
@@ -39,48 +32,45 @@ public LlamaData(@Nullable Color color, boolean isTrader) {
 
 	@Override
 	protected boolean init(Literal<?>[] exprs, int matchedPattern, ParseResult parseResult) {
-		isTrader = TRADER_SUPPORT && matchedPattern > 4;
-		if (TRADER_SUPPORT && matchedPattern > 5) {
-			color = Color.values()[matchedPattern - 6];
+		isTrader = matchedPattern > 4;
+		if (matchedPattern > 5) {
+			color = LLAMA_COLORS[matchedPattern - 6];
 		} else if (matchedPattern > 0 && matchedPattern < 5) {
-			color = Color.values()[matchedPattern - 1];
+			color = LLAMA_COLORS[matchedPattern - 1];
 		}
-
+		// Sets 'matchedPattern' of 'EntityData' for proper 'toString'
+		super.matchedPattern = (color != null ? (color.ordinal() + 1) : 0) + (isTrader ? 5 : 0);
 		return true;
 	}
 
 	@Override
-	protected boolean init(@Nullable Class<? extends Llama> c, @Nullable Llama llama) {
-		if (TRADER_SUPPORT && c != null)
-			isTrader = c.isAssignableFrom(TraderLlama.class);
+	protected boolean init(@Nullable Class<? extends Llama> entityClass, @Nullable Llama llama) {
+		if (entityClass != null)
+			isTrader = TraderLlama.class.isAssignableFrom(entityClass);
 		if (llama != null) {
 			color = llama.getColor();
-			isTrader = TRADER_SUPPORT && llama instanceof TraderLlama;
+			isTrader = llama instanceof TraderLlama;
 		}
 		return true;
 	}
 
 	@Override
 	public void set(Llama entity) {
-		Color randomColor = color == null ? CollectionUtils.getRandom(Color.values()) : color;
+		Color randomColor = color == null ? CollectionUtils.getRandom(LLAMA_COLORS) : color;
 		assert randomColor != null;
 		entity.setColor(randomColor);
 	}
 
 	@Override
 	protected boolean match(Llama entity) {
-		return (TRADER_SUPPORT && isTrader == entity instanceof TraderLlama && (color == null || color == entity.getColor()))
-			|| color == null || color == entity.getColor();
+		if (isTrader && !(entity instanceof TraderLlama))
+			return false;
+		return color == null || color == entity.getColor();
 	}
 
 	@Override
 	public Class<? extends Llama> getType() {
-		// If TraderLlama does not exist, this would ALWAYS throw ClassNotFoundException
-		// (no matter if isTrader == false)
-		if (TRADER_SUPPORT)
-			return isTrader ? TraderLlama.class : Llama.class;
-		assert !isTrader; // Shouldn't be possible on this version
-		return Llama.class;
+		return isTrader ? TraderLlama.class : Llama.class;
 	}
 
 	@Override
@@ -99,18 +89,19 @@ protected int hashCode_i() {
 
 	@Override
 	protected boolean equals_i(EntityData<?> data) {
-		if (!(data instanceof LlamaData))
+		if (!(data instanceof LlamaData other))
 			return false;
-		LlamaData d = (LlamaData) data;
-		return isTrader == d.isTrader && d.color == color;
+		return isTrader == other.isTrader && other.color == color;
 	}
 
 	@Override
 	public boolean isSupertypeOf(EntityData<?> data) {
-		if (!(data instanceof LlamaData))
+		if (!(data instanceof LlamaData other))
+			return false;
+
+		if (isTrader && !other.isTrader)
 			return false;
-		LlamaData d = (LlamaData) data;
-		return isTrader == d.isTrader && (color == null || d.color == color);
+		return color == null || color == other.color;
 	}
 
 }

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

@@ -2,50 +2,55 @@
 
 import ch.njol.skript.Skript;
 import ch.njol.skript.bukkitutil.EntityUtils;
-import org.bukkit.block.Block;
-import org.bukkit.block.CreatureSpawner;
-import org.bukkit.block.TrialSpawner;
-import org.bukkit.entity.EntityType;
-import org.bukkit.event.Event;
-import org.bukkit.spawner.TrialSpawnerConfiguration;
-import org.jetbrains.annotations.Nullable;
-
-import ch.njol.skript.classes.Changer;
 import ch.njol.skript.classes.Changer.ChangeMode;
 import ch.njol.skript.doc.Description;
-import ch.njol.skript.doc.Examples;
+import ch.njol.skript.doc.Example;
 import ch.njol.skript.doc.Name;
+import ch.njol.skript.doc.RequiredPlugins;
 import ch.njol.skript.doc.Since;
 import ch.njol.skript.entity.EntityData;
 import ch.njol.skript.expressions.base.SimplePropertyExpression;
 import ch.njol.util.coll.CollectionUtils;
+import org.bukkit.block.Block;
+import org.bukkit.block.CreatureSpawner;
+import org.bukkit.block.TrialSpawner;
+import org.bukkit.entity.EntityType;
+import org.bukkit.event.Event;
+import org.bukkit.spawner.TrialSpawnerConfiguration;
+import org.jetbrains.annotations.Nullable;
 
 @Name("Spawner Type")
-@Description("Retrieves, sets, or resets the spawner's entity type")
-@Examples({
-	"on right click:",
-		"\tif event-block is spawner:",
-			"\t\tsend \"Spawner's type is %target block's entity type%\""
-})
-@Since("2.4, 2.9.2 (trial spawner)")
+@Description("""
+	The entity type of a spawner (mob spawner).
+	Change the entity type, reset it (pig) or clear it (Minecraft 1.20.0+).
+	""")
+@Example("""
+	on right click:
+		if event-block is a spawner:
+			send "Spawner's type if %spawner type of event-block%" to player
+	""")
+@Example("set the creature type of {_spawner} to a trader llama")
+@Example("reset {_spawner}'s entity type # Pig")
+@Example("clear the spawner type of {_spawner} # Minecraft 1.20.0+")
+@Since("2.4, 2.9.2 (trial spawner), INSERT VERSION (delete)")
+@RequiredPlugins("Minecraft 1.20.0+ (delete)")
 public class ExprSpawnerType extends SimplePropertyExpression<Block, EntityData> {
 
 	private static final boolean HAS_TRIAL_SPAWNER = Skript.classExists("org.bukkit.block.TrialSpawner");
+	private static final boolean RUNNING_1_20_0 = Skript.isRunningMinecraft(1, 20, 0);
 
 	static {
 		register(ExprSpawnerType.class, EntityData.class, "(spawner|entity|creature) type[s]", "blocks");
 	}
 
 	@Nullable
 	public EntityData convert(Block block) {
-		if (block.getState() instanceof CreatureSpawner) {
-			EntityType type = ((CreatureSpawner) block.getState()).getSpawnedType();
+		if (block.getState() instanceof CreatureSpawner creatureSpawner) {
+			EntityType type = creatureSpawner.getSpawnedType();
 			if (type == null)
 				return null;
 			return EntityUtils.toSkriptEntityData(type);
-		}
-		if (HAS_TRIAL_SPAWNER && block.getState() instanceof TrialSpawner) {
-			TrialSpawner trialSpawner = (TrialSpawner) block.getState();
+		} else if (HAS_TRIAL_SPAWNER && block.getState() instanceof TrialSpawner trialSpawner) {
 			EntityType type;
 			if (trialSpawner.isOminous()) {
 				type = trialSpawner.getOminousConfiguration().getSpawnedType();
@@ -58,52 +63,43 @@ public EntityData convert(Block block) {
 		}
 		return null;
 	}
-
-	@Nullable
+
 	@Override
-	public Class<?>[] acceptChange(Changer.ChangeMode mode) {
-		switch (mode) {
-			case SET:
-			case RESET:
+	public Class<?> @Nullable [] acceptChange(ChangeMode mode) {
+		if (mode == ChangeMode.SET || mode == ChangeMode.RESET) {
+			return CollectionUtils.array(EntityData.class);
+		} else if (mode == ChangeMode.DELETE) {
+			if (RUNNING_1_20_0) {
 				return CollectionUtils.array(EntityData.class);
-			default:
-				return null;
+			} else {
+				Skript.error("You can only delete the spawner type of a spawner on Minecraft 1.20.0 or newer");
+			}
 		}
+		return null;
 	}
-
-	@SuppressWarnings("null")
+
 	@Override
 	public void change(Event event, Object @Nullable [] delta, ChangeMode mode) {
+		EntityType entityType = null;
+		if (delta != null) {
+			//noinspection rawtypes
+			entityType = EntityUtils.toBukkitEntityType(((EntityData) delta[0]));
+		} else if (mode == ChangeMode.RESET) {
+			entityType = EntityType.PIG;
+		}
+
 		for (Block block : getExpr().getArray(event)) {
-			if (block.getState() instanceof CreatureSpawner) {
-				CreatureSpawner spawner = (CreatureSpawner) block.getState();
-				switch (mode) {
-					case SET:
-						assert delta != null;
-						spawner.setSpawnedType(EntityUtils.toBukkitEntityType((EntityData) delta[0]));
-						break;
-					case RESET:
-						spawner.setSpawnedType(EntityType.PIG);
-						break;
-				}
-				spawner.update(); // Actually trigger the spawner's update
-			} else if (HAS_TRIAL_SPAWNER && block.getState() instanceof TrialSpawner) {
-				TrialSpawner trialSpawner = (TrialSpawner) block.getState();
+			if (block.getState() instanceof CreatureSpawner creatureSpawner) {
+				creatureSpawner.setSpawnedType(entityType);
+				creatureSpawner.update(); // Actually trigger the spawner's update
+			} else if (HAS_TRIAL_SPAWNER && block.getState() instanceof TrialSpawner trialSpawner) {
 				TrialSpawnerConfiguration config;
 				if (trialSpawner.isOminous()) {
 					config = trialSpawner.getOminousConfiguration();
 				} else {
 					config = trialSpawner.getNormalConfiguration();
 				}
-				switch (mode) {
-					case SET:
-						assert delta != null;
-						config.setSpawnedType((EntityUtils.toBukkitEntityType((EntityData) delta[0])));
-						break;
-					case RESET:
-						config.setSpawnedType(EntityType.PIG);
-						break;
-				}
+				config.setSpawnedType(entityType);
 				trialSpawner.update();
 			}
 		}

src/test/skript/tests/regressions/7951-llama and trader llama.sk

@@ -0,0 +1,49 @@
+test "llama entity data":
+	spawn a llama at test-location:
+		set {_entity} to entity
+	assert {_entity} is a llama with "Llama should be a llama"
+	assert {_entity} is not a trader llama with "Llama should not be a trader llama"
+	clear entity within {_entity}
+
+	spawn a creamy llama at test-location:
+		set {_entity} to entity
+	assert {_entity} is a llama with "Creamy llama should be a llama"
+	assert {_entity} is a creamy llama with "Creamy llama should be a creamy llama"
+	assert {_entity} is not a gray llama with "Creamy llama should not be a gray llama"
+	assert {_entity} is not a trader llama with "Creamy llama should not be a trader llama"
+	assert {_entity} is not a creamy trader llama with "Creamy llama should not be a creamy trader llama"
+	assert {_entity} is not a gray trader llama with "Creamy llama should not be a gray trader llama"
+	clear entity within {_entity}
+
+	set {_old} to blockdata of test-block
+	set test-block to a spawner
+	set spawner type of test-block to a llama
+	assert spawner type of test-block is a llama with "Monster spawner should be a llama"
+	assert spawner type of test-block is not a trader llama with "Monster spawner should not be a trader llama"
+	set blockdata of test-block to {_old}
+	clear all entities
+
+test "trader llama entity data":
+	spawn a trader llama at test-location:
+		set {_entity} to entity
+	assert {_entity} is a llama with "Trader llama should be a llama"
+	assert {_entity} is a trader llama with "Trader llama should be a trader llama"
+	clear entity within {_entity}
+
+	spawn a creamy trader llama at test-location:
+		set {_entity} to entity
+	assert {_entity} is a llama with "Creamy trader llama should be a llama"
+	assert {_entity} is a creamy llama with "Creamy trader llama should be a creamy llama"
+	assert {_entity} is not a gray llama with "Creamy trader llama should not be a gray llama"
+	assert {_entity} is a trader llama with "Creamy trader llama should be a trader llama"
+	assert {_entity} is a creamy trader llama with "Creamy trader llama should be a creamy trader llama"
+	assert {_entity} is not a gray trader llama with "Creamy trader llama should not be a gray trader llama"
+	clear entity within {_entity}
+
+	set {_old} to blockdata of test-block
+	set test-block to a spawner
+	set spawner type of test-block to a trader llama
+	assert spawner type of test-block is a llama with "Monster spawner should be a llama"
+	assert spawner type of test-block is a trader llama with "Monster spawner should be a trader llama"
+	set blockdata of test-block to {_old}
+	clear all entities

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

@@ -21,3 +21,11 @@ test "trial spawner type" when running minecraft "1.21":
 	assert spawner type of {_block} is pig with "Spawner type was not reset correctly"
 
 	set block at {_block} to air
+
+test "delete spawner type" when running minecraft "1.20.0":
+	set test-block to spawner
+	set the spawner type of test-block to a trader llama
+	assert the spawner type of test-block is a trader llama with "Spawner type should be a trader llama"
+	clear the spawner type of test-block
+	assert the spawner type of test-block is not set with "Spawner type was not cleared"
+	set test-block to air