For each index & value loop. (#6562)

* Pick first. * Export behaviour from SecLoop instead. * Fix changes from upstream. * Update src/main/java/ch/njol/skript/sections/SecFor.java Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Patrick Miller <apickledwalrus@gmail.com> * Fix problems from merge. * Update src/main/java/ch/njol/skript/sections/SecFor.java Co-authored-by: Efnilite <35348263+Efnilite@users.noreply.github.com> --------- Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com> Co-authored-by: Patrick Miller <apickledwalrus@gmail.com> Co-authored-by: Efnilite <35348263+Efnilite@users.noreply.github.com>
SkriptLang · Dec 18, 2024 · 3bcee98 · 3bcee98
1 parent adf289d
commit 3bcee98
Show file tree

Hide file tree

Showing 4 changed files with 244 additions and 22 deletions.
diff --git a/src/main/java/ch/njol/skript/registrations/Feature.java b/src/main/java/ch/njol/skript/registrations/Feature.java
@@ -29,6 +29,7 @@
  * Experimental feature toggles as provided by Skript itself.
  */
 public enum Feature implements Experiment {
+	FOR_EACH_LOOPS("for loop", LifeCycle.EXPERIMENTAL, "for [each] loop[s]")
 	;
 
 	private final String codeName;

diff --git a/src/main/java/ch/njol/skript/sections/SecFor.java b/src/main/java/ch/njol/skript/sections/SecFor.java
@@ -0,0 +1,155 @@
+package ch.njol.skript.sections;
+
+import ch.njol.skript.Skript;
+import ch.njol.skript.SkriptAPIException;
+import ch.njol.skript.classes.Changer;
+import ch.njol.skript.config.SectionNode;
+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.lang.Expression;
+import ch.njol.skript.lang.SkriptParser.ParseResult;
+import ch.njol.skript.lang.TriggerItem;
+import ch.njol.skript.lang.Variable;
+import ch.njol.skript.lang.util.ContainerExpression;
+import ch.njol.skript.registrations.Feature;
+import ch.njol.skript.util.Container;
+import ch.njol.skript.util.Container.ContainerType;
+import ch.njol.skript.util.LiteralUtils;
+import ch.njol.util.Kleenean;
+import org.bukkit.event.Event;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.List;
+import java.util.Map;
+
+@Name("For Each Loop (Experimental)")
+@Description("""
+	A specialised loop section run for each element in a list.
+	Unlike the basic loop, this is designed for extracting the key & value from pairs.
+	The loop element's key/index and value can be stored in a variable for convenience.
+	
+	When looping a simple (non-indexed) set of values, e.g. all players, the index will be the loop counter number."""
+)
+@Examples({
+	"for each {_player} in players:",
+	"\tsend \"Hello %{_player}%!\" to {_player}",
+	"",
+	"loop {_item} in {list of items::*}:",
+	"\tbroadcast {_item}'s name",
+	"",
+	"for each key {_index} in {list of items::*}:",
+	"\tbroadcast {_index}",
+	"",
+	"loop key {_index} and value {_value} in {list of items::*}:",
+	"\tbroadcast \"%{_index}% = %{_value}%\"",
+	"",
+	"for each {_index} = {_value} in {my list::*}:",
+	"\tbroadcast \"%{_index}% = %{_value}%\"",
+})
+@Since("INSERT VERSION")
+public class SecFor extends SecLoop {
+
+	static {
+		Skript.registerSection(SecFor.class,
+			"(for [each]|loop) [value] %~object% in %objects%",
+			"(for [each]|loop) (key|index) %~object% in %objects%",
+			"(for [each]|loop) [key|index] %~object%(,| and) [value] %~object% in %objects%"
+		);
+	}
+
+	private @Nullable Expression<?> keyStore, valueStore;
+
+	@Override
+	@SuppressWarnings("unchecked")
+	public boolean init(Expression<?>[] exprs,
+						int matchedPattern,
+						Kleenean isDelayed,
+						ParseResult parseResult,
+						SectionNode sectionNode,
+						List<TriggerItem> triggerItems) {
+		if (!this.getParser().hasExperiment(Feature.FOR_EACH_LOOPS))
+			return false;
+		//<editor-fold desc="Set the key/value expressions based on the pattern" defaultstate="collapsed">
+		switch (matchedPattern) {
+			case 0:
+				this.valueStore = exprs[0];
+				this.expression = LiteralUtils.defendExpression(exprs[1]);
+				break;
+			case 1:
+				this.keyStore = exprs[0];
+				this.expression = LiteralUtils.defendExpression(exprs[1]);
+				break;
+			default:
+				this.keyStore = exprs[0];
+				this.valueStore = exprs[1];
+				this.expression = LiteralUtils.defendExpression(exprs[2]);
+		}
+		//</editor-fold>
+		//<editor-fold desc="Check our input expressions are safe/correct" defaultstate="collapsed">
+		if (keyStore != null && !(keyStore instanceof Variable)) {
+			Skript.error("The 'key' input for a for-loop must be a variable to store the value.");
+			return false;
+		}
+		if (!(valueStore instanceof Variable || valueStore == null)) {
+			Skript.error("The 'value' input for a for-loop must be a variable to store the value.");
+			return false;
+		}
+		if (!LiteralUtils.canInitSafely(expression)) {
+			Skript.error("Can't understand this loop: '" + parseResult.expr + "'");
+			return false;
+		}
+		if (Container.class.isAssignableFrom(expression.getReturnType())) {
+			ContainerType type = expression.getReturnType().getAnnotation(ContainerType.class);
+			if (type == null)
+				throw new SkriptAPIException(expression.getReturnType()
+					.getName() + " implements Container but is missing the required @ContainerType annotation");
+			this.expression = new ContainerExpression((Expression<? extends Container<?>>) expression, type.value());
+		}
+		if (expression.isSingle()) {
+			Skript.error("Can't loop '" + expression + "' because it's only a single value");
+			return false;
+		}
+		//</editor-fold>
+		this.loadOptionalCode(sectionNode);
+		super.setNext(this);
+		return true;
+	}
+
+	@Override
+	protected void store(Event event, Object next) {
+		super.store(event, next);
+		//<editor-fold desc="Store the loop index/value in the variables" defaultstate="collapsed">
+		if (next instanceof Map.Entry) {
+			//noinspection unchecked
+			Map.Entry<String, Object> entry = (Map.Entry<String, Object>) next;
+			if (keyStore != null)
+				this.keyStore.change(event, new Object[] {entry.getKey()}, Changer.ChangeMode.SET);
+			if (valueStore != null)
+				this.valueStore.change(event, new Object[] {entry.getValue()}, Changer.ChangeMode.SET);
+		} else {
+			if (keyStore != null)
+				this.keyStore.change(event, new Object[] {this.getLoopCounter(event)}, Changer.ChangeMode.SET);
+			if (valueStore != null)
+				this.valueStore.change(event, new Object[] {next}, Changer.ChangeMode.SET);
+		}
+		//</editor-fold>
+	}
+
+	@Override
+	public String toString(@Nullable Event event, boolean debug) {
+		if (keyStore != null && valueStore != null) {
+			return "for each key " + keyStore.toString(event, debug)
+				+ " and value " + valueStore.toString(event, debug) + " in "
+				+ super.expression.toString(event, debug);
+		} else if (keyStore != null) {
+			return "for each key " + keyStore.toString(event, debug)
+				+ " in " + super.expression.toString(event, debug);
+		}
+		assert valueStore != null : "How did we get here?";
+		return "for each value " + valueStore.toString(event, debug)
+			+ " in " + super.expression.toString(event, debug);
+	}
+
+}
diff --git a/src/main/java/ch/njol/skript/sections/SecLoop.java b/src/main/java/ch/njol/skript/sections/SecLoop.java
@@ -34,6 +34,7 @@
 import ch.njol.util.Kleenean;
 import org.bukkit.event.Event;
 import org.jetbrains.annotations.Nullable;
+import org.jetbrains.annotations.UnknownNullability;
 
 import java.util.Iterator;
 import java.util.List;
@@ -83,13 +84,12 @@ public class SecLoop extends LoopSection {
 		Skript.registerSection(SecLoop.class, "loop %objects%");
 	}
 
-	@SuppressWarnings("NotNullFieldNotInitialized")
-	private Expression<?> expr;
+	protected @UnknownNullability Expression<?> expression;
 
 	private final transient Map<Event, Object> current = new WeakHashMap<>();
-	private final transient Map<Event, Iterator<?>> currentIter = new WeakHashMap<>();
+	private final transient Map<Event, Iterator<?>> iteratorMap = new WeakHashMap<>();
 
-	private @Nullable TriggerItem actualNext;
+	protected @Nullable TriggerItem actualNext;
 	private boolean guaranteedToLoop;
 
 	@Override
@@ -100,25 +100,25 @@ public boolean init(Expression<?>[] exprs,
 						ParseResult parseResult,
 						SectionNode sectionNode,
 						List<TriggerItem> triggerItems) {
-		expr = LiteralUtils.defendExpression(exprs[0]);
-		if (!LiteralUtils.canInitSafely(expr)) {
+		this.expression = LiteralUtils.defendExpression(exprs[0]);
+		if (!LiteralUtils.canInitSafely(expression)) {
 			Skript.error("Can't understand this loop: '" + parseResult.expr.substring(5) + "'");
 			return false;
 		}
 
-		if (Container.class.isAssignableFrom(expr.getReturnType())) {
-			ContainerType type = expr.getReturnType().getAnnotation(ContainerType.class);
+		if (Container.class.isAssignableFrom(expression.getReturnType())) {
+			ContainerType type = expression.getReturnType().getAnnotation(ContainerType.class);
 			if (type == null)
-				throw new SkriptAPIException(expr.getReturnType().getName() + " implements Container but is missing the required @ContainerType annotation");
-			expr = new ContainerExpression((Expression<? extends Container<?>>) expr, type.value());
+				throw new SkriptAPIException(expression.getReturnType().getName() + " implements Container but is missing the required @ContainerType annotation");
+			this.expression = new ContainerExpression((Expression<? extends Container<?>>) expression, type.value());
 		}
 
-		if (expr.isSingle()) {
-			Skript.error("Can't loop '" + expr + "' because it's only a single value");
+		if (expression.isSingle()) {
+			Skript.error("Can't loop '" + expression + "' because it's only a single value");
 			return false;
 		}
 
-		guaranteedToLoop = guaranteedToLoop(expr);
+		guaranteedToLoop = guaranteedToLoop(expression);
 		loadOptionalCode(sectionNode);
 		super.setNext(this);
 
@@ -128,12 +128,12 @@ public boolean init(Expression<?>[] exprs,
 	@Override
 	@Nullable
 	protected TriggerItem walk(Event event) {
-		Iterator<?> iter = currentIter.get(event);
+		Iterator<?> iter = iteratorMap.get(event);
 		if (iter == null) {
-			iter = expr instanceof Variable<?> variable ? variable.variablesIterator(event) : expr.iterator(event);
+			iter = expression instanceof Variable variable ? variable.variablesIterator(event) : expression.iterator(event);
 			if (iter != null) {
 				if (iter.hasNext())
-					currentIter.put(event, iter);
+					iteratorMap.put(event, iter);
 				else
 					iter = null;
 			}
@@ -143,20 +143,25 @@ protected TriggerItem walk(Event event) {
 			debug(event, false);
 			return actualNext;
 		} else {
-			current.put(event, iter.next());
-			currentLoopCounter.put(event, (currentLoopCounter.getOrDefault(event, 0L)) + 1);
+			Object next = iter.next();
+			this.store(event, next);
 			return walk(event, true);
 		}
 	}
 
+	protected void store(Event event, Object next) {
+		this.current.put(event, next);
+		this.currentLoopCounter.put(event, (currentLoopCounter.getOrDefault(event, 0L)) + 1);
+	}
+
 	@Override
 	public @Nullable ExecutionIntent executionIntent() {
 		return guaranteedToLoop ? triggerExecutionIntent() : null;
 	}
 
 	@Override
 	public String toString(@Nullable Event event, boolean debug) {
-		return "loop " + expr.toString(event, debug);
+		return "loop " + expression.toString(event, debug);
 	}
 
 	@Nullable
@@ -165,7 +170,7 @@ public Object getCurrent(Event event) {
 	}
 
 	public Expression<?> getLoopedExpression() {
-		return expr;
+		return expression;
 	}
 
 	@Override
@@ -183,15 +188,15 @@ public TriggerItem getActualNext() {
 	@Override
 	public void exit(Event event) {
 		current.remove(event);
-		currentIter.remove(event);
+		iteratorMap.remove(event);
 		super.exit(event);
 	}
 
 	private static boolean guaranteedToLoop(Expression<?> expression) {
 		// If the expression is a literal, it's guaranteed to loop if it has at least one value
 		if (expression instanceof Literal<?> literal)
 			return literal.getAll().length > 0;
-		
+
 		// If the expression isn't a list, then we can't guarantee that it will loop
 		if (!(expression instanceof ExpressionList<?> list))
 			return false;

diff --git a/src/test/skript/tests/syntaxes/sections/SecFor.sk b/src/test/skript/tests/syntaxes/sections/SecFor.sk
@@ -0,0 +1,61 @@
+using for each loops
+
+test "for section":
+
+	set {_list::*} to 1, 5, and 10
+	set {_expect} to 0
+	for {_value} in {_list::*}:
+		assert {_value} is greater than 0 with "Expected value > 0, found %{_value}%"
+		set {_expect} to {_value}
+
+	assert {_value} is 10 with "Expected value = 10, found %{_value}%"
+	assert {_expect} is 10 with "Expected expect = 10, found %{_expect}%"
+
+	delete {_key}
+	delete {_value}
+
+	for {_key}, {_value} in {_list::*}:
+		assert {_key} is greater than 0 with "Expected key > 0, found %{_key}%"
+		assert {_key} is less than 4 with "Expected key < 4, found %{_key}%"
+		assert {_value} is greater than 0 with "Expected value > 0, found %{_value}%"
+
+	assert {_key} is 3 with "Expected key = 3, found %{_key}%"
+	assert {_value} is 10 with "Expected value = 10, found %{_value}%"
+
+	delete {_key}
+	delete {_value}
+
+
+	for key {_key} and value {_value} in {_list::*}:
+		assert {_key} is greater than 0 with "Expected key > 0, found %{_key}%"
+		assert {_key} is less than 4 with "Expected key < 4, found %{_key}%"
+		assert {_value} is greater than 0 with "Expected value > 0, found %{_value}%"
+
+	assert {_key} is 3 with "Expected key = 3, found %{_key}%"
+	assert {_value} is 10 with "Expected value = 10, found %{_value}%"
+
+	delete {_key}
+	delete {_value}
+
+	for {_key} and {_value} in {_list::*}:
+		assert {_key} is greater than 0 with "Expected key > 0, found %{_key}%"
+		assert {_key} is less than 4 with "Expected key < 4, found %{_key}%"
+		assert {_value} is greater than 0 with "Expected value > 0, found %{_value}%"
+
+	assert {_key} is 3 with "Expected key = 3, found %{_key}%"
+	assert {_value} is 10 with "Expected value = 10, found %{_value}%"
+
+	delete {_key}
+	delete {_value}
+
+	# 'loop' syntax alternative
+	loop {_key} and {_value} in {_list::*}:
+		assert {_key} is greater than 0 with "Expected key > 0, found %{_key}%"
+		assert {_key} is less than 4 with "Expected key < 4, found %{_key}%"
+		assert {_value} is greater than 0 with "Expected value > 0, found %{_value}%"
+
+	assert {_key} is 3 with "Expected key = 3, found %{_key}%"
+	assert {_value} is 10 with "Expected value = 10, found %{_value}%"
+
+	delete {_key}
+	delete {_value}