Fix SecModifySpawnerData and ExprSpawnerData & cleanup

Author: Burbulinis

src/main/java/org/skriptlang/skript/bukkit/spawners/elements/effects/EffSpawnerItem.java

@@ -19,8 +19,8 @@
 
 @Name("Modify Spawner Item")
 @Description("""
-	Make a mob spawner spawn items rather than entities. In this case, the mob spawner's spawn count determines how many \
-	stacks are spawned, not how many items should be in each stack.
+	Make a mob spawner spawn items rather than entities. In this case, the mob spawner's spawn count determines \
+	how many stacks are spawned, not how many items should be in each stack.
 	""")
 @Example("""
 	make event-block spawn 15 diamonds

src/main/java/org/skriptlang/skript/bukkit/spawners/elements/expressions/spawner/spawnerdata/ExprSpawnerData.java

@@ -5,11 +5,13 @@
 import ch.njol.skript.doc.Example;
 import ch.njol.skript.doc.Name;
 import ch.njol.skript.doc.RequiredPlugins;
-import ch.njol.skript.expressions.base.SimplePropertyExpression;
+import ch.njol.skript.expressions.base.PropertyExpression;
 import ch.njol.skript.lang.Expression;
 import ch.njol.skript.lang.SkriptParser.ParseResult;
+import ch.njol.skript.lang.SyntaxStringBuilder;
 import ch.njol.util.Kleenean;
 import ch.njol.util.coll.CollectionUtils;
+import org.bukkit.block.TrialSpawner;
 import org.bukkit.event.Event;
 import org.jetbrains.annotations.Nullable;
 import org.skriptlang.skript.bukkit.spawners.util.SpawnerDataType;
@@ -19,14 +21,14 @@
 import org.skriptlang.skript.bukkit.spawners.util.spawnerdata.SkriptTrialSpawnerData;
 import org.skriptlang.skript.registration.SyntaxRegistry;
 
-import java.util.StringJoiner;
+import java.util.ArrayList;
+import java.util.List;
 
 @Name("Spawner Data")
 @Description("""
 	Returns the spawner data of a spawner. Since trial spawners use different data in its ominous and regular states, \
 	you can specifically set the data of the ominous state using 'ominous trial spawner data'. Additionally, \
-	you can set the data of both states using 'ominous and regular trial spawner data', which will apply the data to \
-	both states of the trial spawner.
+	you can set the data of both states using 'ominous and regular trial spawner data'.
 	""")
 @Example("""
 	set the spawner data of event-block to the mob spawner data:
@@ -48,39 +50,79 @@
 		add {_entries::*} to the spawner entries
 		set the base entity spawn count to 12
 
-	set the trial spawner data of event-block to {_trial data} # regular data
-	set the ominous trial spawner data of event-block to {_trial data} # ominous data
+	set the trial spawner data of event-block to {_trial data} # regular state
+	set the ominous trial spawner data of event-block to {_trial data} # ominous state
 	set the ominous and regular trial spawner datas of event-block to {_trial data} # both states
 	""")
 @RequiredPlugins("Minecraft 1.21+ (for trial spawner data)")
-public class ExprSpawnerData extends SimplePropertyExpression<Object, SkriptSpawnerData> {
+public class ExprSpawnerData extends PropertyExpression<Object, SkriptSpawnerData> {
 
 	public static void register(SyntaxRegistry registry) {
 		String property = "[:mob] spawner data[s]";
 		if (SpawnerUtils.IS_RUNNING_1_21)
-			property = "[trial:[:ominous [regular:and (regular|normal]] trial|:mob] spawner data[s]";
+			property = "[trial:[:ominous|:regular|:ominous and regular] trial|:mob] spawner data[s]";
 
 		registry.register(SyntaxRegistry.EXPRESSION, infoBuilder(ExprSpawnerData.class, SkriptSpawnerData.class,
 			property, SpawnerUtils.spawnerPropertyType, false)
-				.supplier(ExprSpawnerData::new)
-				.build()
+			.supplier(ExprSpawnerData::new)
+			.build()
 		);
 	}
 
+	private enum TrialSpawnerState {
+		OMINOUS, REGULAR, BOTH;
+
+		public static TrialSpawnerState fromTags(List<String> tags) {
+			if (tags.contains("ominous")) {
+				return OMINOUS;
+			} else if (tags.contains("ominous and regular")) {
+				return BOTH;
+			} else {
+				return REGULAR;
+			}
+		}
+	}
+
 	private SpawnerDataType dataType;
-	private boolean ominous, regular;
+	private TrialSpawnerState state;
 
 	@Override
 	public boolean init(Expression<?>[] expressions, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) {
 		dataType = SpawnerDataType.fromTags(parseResult.tags);
-		ominous = parseResult.hasTag("ominous");
-		regular = parseResult.hasTag("regular");
-		return super.init(expressions, matchedPattern, isDelayed, parseResult);
+		state = TrialSpawnerState.fromTags(parseResult.tags);
+		return true;
 	}
 
 	@Override
-	public @Nullable SkriptSpawnerData convert(Object object) {
-		return SpawnerUtils.getDataFromObject(object, dataType);
+	protected SkriptSpawnerData[] get(Event event, Object[] source) {
+		List<SkriptSpawnerData> datas = new ArrayList<>();
+
+		for (Object spawnerObject : source) {
+			if (!dataType.matches(spawnerObject))
+				continue;
+
+			if (SpawnerUtils.isMobSpawner(spawnerObject)) {
+				if (SpawnerUtils.isSpawnerMinecart(spawnerObject)) {
+					datas.add(SkriptMobSpawnerData.fromSpawner(SpawnerUtils.getSpawnerMinecart(spawnerObject)));
+				} else {
+					datas.add(SkriptMobSpawnerData.fromSpawner(SpawnerUtils.getCreatureSpawner(spawnerObject)));
+				}
+
+				continue;
+			}
+
+			TrialSpawner trialSpawner = SpawnerUtils.getTrialSpawner(spawnerObject);
+			datas.addAll(switch (state) {
+				case OMINOUS -> List.of(SkriptTrialSpawnerData.fromTrialSpawner(trialSpawner, true));
+				case REGULAR -> List.of(SkriptTrialSpawnerData.fromTrialSpawner(trialSpawner, false));
+				case BOTH -> List.of(
+					SkriptTrialSpawnerData.fromTrialSpawner(trialSpawner, true),
+					SkriptTrialSpawnerData.fromTrialSpawner(trialSpawner, false)
+				);
+			});
+		}
+
+		return datas.toArray(SkriptSpawnerData[]::new);
 	}
 
 	@Override
@@ -95,34 +137,69 @@ public boolean init(Expression<?>[] expressions, int matchedPattern, Kleenean is
 	public void change(Event event, Object @Nullable [] delta, ChangeMode mode) {
 		SkriptSpawnerData data = delta != null ? (SkriptSpawnerData) delta[0] : null;
 
-		for (Object object : getExpr().getArray(event)) {
+		for (Object spawnerObject : getExpr().getArray(event)) {
+			if (!dataType.matches(spawnerObject))
+				continue;
+
 			if (data == null) {
-				if (SpawnerUtils.isCreatureSpawner(object) || SpawnerUtils.isSpawnerMinecart(object)) {
+				if (SpawnerUtils.isMobSpawner(spawnerObject)) {
 					data = new SkriptMobSpawnerData();
-				} else if (SpawnerUtils.isTrialSpawner(object)) {
+				} else if (SpawnerUtils.isTrialSpawner(spawnerObject)) {
 					data = new SkriptTrialSpawnerData();
 				}
 			}
 
 			if (data == null)
 				continue;
 
-			SpawnerUtils.applyData(data, object, dataType, ominous, regular);
+			if (data instanceof SkriptMobSpawnerData mobData) {
+				SpawnerUtils.applyToMobSpawner(spawnerObject, mobData);
+				continue;
+			}
+
+			assert data instanceof SkriptTrialSpawnerData;
+
+			SkriptTrialSpawnerData trialData = (SkriptTrialSpawnerData) data;
+			TrialSpawner trialSpawner = SpawnerUtils.getTrialSpawner(spawnerObject);
+			switch (state) {
+				case OMINOUS -> trialData.applyData(trialSpawner, true);
+				case REGULAR -> trialData.applyData(trialSpawner, false);
+				case BOTH -> {
+					trialData.applyData(trialSpawner, true);
+					trialData.applyData(trialSpawner, false);
+				}
+			}
 		}
 	}
 
+	@Override
+	public boolean isSingle() {
+		return getExpr().isSingle() && state != TrialSpawnerState.BOTH;
+	}
+
 	@Override
 	public Class<? extends SkriptSpawnerData> getReturnType() {
 		return dataType.getDataClass();
 	}
 
 	@Override
-	protected String getPropertyName() {
-		StringJoiner joiner = new StringJoiner(" ", "", "spawner data");
-		if (ominous)
-			joiner.add("ominous");
-		joiner.add(dataType.toString());
-		return joiner.toString();
+	public String toString(@Nullable Event event, boolean debug) {
+		SyntaxStringBuilder builder = new SyntaxStringBuilder(event, debug);
+
+		builder.append("the");
+		if (dataType.isTrial()) {
+			if (state == TrialSpawnerState.REGULAR) {
+				builder.append("regular");
+			} else if (state == TrialSpawnerState.OMINOUS) {
+				builder.append("ominous");
+			} else {
+				builder.append("ominous and regular");
+			}
+		}
+
+		builder.append(dataType.toString() + " spawner data of", getExpr());
+
+		return builder.toString();
 	}
 
 }

src/main/java/org/skriptlang/skript/bukkit/spawners/elements/sections/SecModifySpawnerData.java

@@ -6,6 +6,7 @@
 import ch.njol.skript.lang.util.SectionUtils;
 import ch.njol.skript.variables.Variables;
 import ch.njol.util.Kleenean;
+import org.bukkit.block.TrialSpawner;
 import org.bukkit.event.Event;
 import org.jetbrains.annotations.Nullable;
 import org.skriptlang.skript.bukkit.spawners.util.SpawnerDataType;
@@ -30,23 +31,36 @@ public static void register(SyntaxRegistry registry) {
 			.addPattern("modify [the] [:mob] spawner data of %" + SpawnerUtils.spawnerPropertyType + '%');
 
 		if (SpawnerUtils.IS_RUNNING_1_21)
-			info.addPattern("modify [the] [:ominous [regular:and (regular|normal]] trial:trial spawner data of %blocks%");
+			info.addPattern("modify [the] [:ominous|:regular|:ominous and regular] trial:trial spawner data of %blocks%");
 
 		registry.register(SyntaxRegistry.SECTION, info.build());
 	}
 
+	private enum TrialSpawnerState {
+		OMINOUS, REGULAR, BOTH;
+
+		public static TrialSpawnerState fromTags(List<String> tags) {
+			if (tags.contains("ominous")) {
+				return OMINOUS;
+			} else if (tags.contains("ominous and regular")) {
+				return BOTH;
+			} else {
+				return REGULAR;
+			}
+		}
+	}
+
 	private Expression<?> spawners;
 	private SpawnerDataType dataType;
-	private boolean ominous, regular;
+	private TrialSpawnerState state;
 
 	private Trigger trigger;
 
 	@Override
 	public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult, SectionNode sectionNode, List<TriggerItem> triggerItems) {
 		spawners = exprs[0];
 		dataType = SpawnerDataType.fromTags(parseResult.tags);
-		ominous = parseResult.hasTag("ominous");
-		regular = parseResult.hasTag("regular");
+		state = TrialSpawnerState.fromTags(parseResult.tags);
 
 		trigger = SectionUtils.loadLinkedCode("modify spawner data", (beforeLoading, afterLoading)
 			-> loadCode(sectionNode, "modify spawner data", beforeLoading, afterLoading, SpawnerDataEvent.class));
@@ -55,23 +69,44 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
 
 	@Override
 	protected @Nullable TriggerItem walk(Event event) {
-		for (Object object : spawners.getArray(event)) {
-			SkriptSpawnerData data = SpawnerUtils.getDataFromObject(object, dataType);
-
-			if (data == null)
+		for (Object spawnerObject : spawners.getArray(event)) {
+			if (!dataType.matches(spawnerObject))
 				continue;
 
-			SpawnerDataEvent dataEvent = switch (dataType) {
-				case MOB -> new MobSpawnerDataEvent((SkriptMobSpawnerData) data);
-				case TRIAL -> new TrialSpawnerDataEvent((SkriptTrialSpawnerData) data);
-				case ANY -> new SpawnerDataEvent(data, dataType);
-			};
-
-			Variables.withLocalVariables(event, dataEvent, () ->
-				TriggerItem.walk(trigger, dataEvent)
-			);
-
-			SpawnerUtils.applyData(data, object, dataType, ominous, regular);
+			SkriptSpawnerData data;
+
+			if (SpawnerUtils.isCreatureSpawner(spawnerObject)) {
+				data = SkriptMobSpawnerData.fromSpawner(SpawnerUtils.getCreatureSpawner(spawnerObject));
+			} else if (SpawnerUtils.isSpawnerMinecart(spawnerObject)) {
+				data = SkriptMobSpawnerData.fromSpawner(SpawnerUtils.getSpawnerMinecart(spawnerObject));
+			} else {
+				data = SkriptTrialSpawnerData.fromTrialSpawner(SpawnerUtils.getTrialSpawner(spawnerObject),
+					state == TrialSpawnerState.OMINOUS);
+			}
+
+			if (data instanceof SkriptMobSpawnerData mobData) {
+				MobSpawnerDataEvent mobEvent = new MobSpawnerDataEvent(mobData);
+				Variables.withLocalVariables(event, mobEvent, () -> TriggerItem.walk(trigger, mobEvent));
+				SpawnerUtils.applyToMobSpawner(spawnerObject, mobData);
+			} else if (data instanceof SkriptTrialSpawnerData trialData) {
+				TrialSpawner trialSpawner = SpawnerUtils.getTrialSpawner(spawnerObject);
+
+				TrialSpawnerDataEvent regularEvent = new TrialSpawnerDataEvent(trialData);
+				Variables.withLocalVariables(event, regularEvent, () -> TriggerItem.walk(trigger, regularEvent));
+
+				if (state == TrialSpawnerState.BOTH) {
+					// guaranteed to be the regular data here
+					trialData.applyData(trialSpawner, false);
+
+					// modify the ominous data
+					trialData = SkriptTrialSpawnerData.fromTrialSpawner(trialSpawner, true);
+					TrialSpawnerDataEvent ominousEvent = new TrialSpawnerDataEvent(trialData);
+					Variables.withLocalVariables(event, ominousEvent, () -> TriggerItem.walk(trigger, ominousEvent));
+					trialData.applyData(trialSpawner, true);
+				} else {
+					trialData.applyData(trialSpawner, state == TrialSpawnerState.OMINOUS);
+				}
+			}
 		}
 
 		return super.walk(event, false);
@@ -82,8 +117,15 @@ public String toString(@Nullable Event event, boolean debug) {
 		SyntaxStringBuilder builder = new SyntaxStringBuilder(event, debug);
 
 		builder.append("modify the");
-		if (ominous)
-			builder.append("ominous");
+		if (dataType.isTrial()) {
+			if (state == TrialSpawnerState.REGULAR) {
+				builder.append("regular");
+			} else if (state == TrialSpawnerState.OMINOUS) {
+				builder.append("ominous");
+			} else {
+				builder.append("ominous and regular");
+			}
+		}
 		builder.append(dataType.toString(), "spawner data of", spawners);
 
 		return builder.toString();

src/main/java/org/skriptlang/skript/bukkit/spawners/util/SpawnerDataType.java

@@ -65,6 +65,21 @@ public boolean isAny() {
 		return this == ANY;
 	}
 
+	/**
+	 * Checks if the given spawner object matches this spawner data type.
+	 * @param spawnerObject the spawner object to check
+	 * @return true if the spawner object matches the type, false otherwise
+	 */
+	public boolean matches(Object spawnerObject) {
+		if (isTrial()) {
+			return SpawnerUtils.isTrialSpawner(spawnerObject);
+		} else if (isMob()) {
+			return SpawnerUtils.isMobSpawner(spawnerObject);
+		}
+
+		return SpawnerUtils.isMobSpawner(spawnerObject) || SpawnerUtils.isTrialSpawner(spawnerObject);
+	}
+
 	@Override
 	public String toString() {
 		if (isAny())