Merge branch 'dev/feature' into feature/fix-input-keys-delayed

src/main/java/ch/njol/skript/Skript.java

@@ -842,6 +842,8 @@ private void runTests() {
 					classes.removeIf(Class::isLocalClass);
 					// Test that requires package access. This is only present when compiling with src/test.
 					classes.add(Class.forName("ch.njol.skript.variables.FlatFileStorageTest"));
+					classes.add(Class.forName("ch.njol.skript.config.ConfigTest"));
+					classes.add(Class.forName("ch.njol.skript.config.NodeTest"));
 					size.set(classes.size());
 					for (Class<?> clazz : classes) {
 						if (SkriptAsyncJUnitTest.class.isAssignableFrom(clazz)) {

src/main/java/ch/njol/skript/classes/Converter.java

@@ -0,0 +1,53 @@
+package ch.njol.skript.classes;
+
+import ch.njol.skript.command.Commands;
+import ch.njol.skript.util.Utils;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * <h2>WARNING! This class has been removed in this update.</h2>
+ * This class stub has been left behind to prevent loading errors from outdated addons,
+ * but its functionality has been largely removed.
+ *
+ * @deprecated Use {@link org.skriptlang.skript.lang.converter.Converter}
+ */
+@Deprecated(forRemoval = true)
+public interface Converter<F, T> extends org.skriptlang.skript.lang.converter.Converter<F, T> {
+
+	// Interfaces don't have a <clinit> so we trigger the warning notice with this
+	int $_WARNING = Utils.loadedRemovedClassWarning(Converter.class);
+
+	@Deprecated(forRemoval = true)
+	int NO_LEFT_CHAINING = org.skriptlang.skript.lang.converter.Converter.NO_LEFT_CHAINING;
+	@Deprecated(forRemoval = true)
+	int NO_RIGHT_CHAINING = org.skriptlang.skript.lang.converter.Converter.NO_RIGHT_CHAINING;
+	@Deprecated(forRemoval = true)
+	int NO_CHAINING = NO_LEFT_CHAINING | NO_RIGHT_CHAINING;
+	@Deprecated(forRemoval = true)
+	int NO_COMMAND_ARGUMENTS = Commands.CONVERTER_NO_COMMAND_ARGUMENTS;
+
+	@Deprecated(forRemoval = true)
+	@Nullable T convert(F f);
+
+	@Deprecated(forRemoval = true)
+	final class ConverterUtils {
+
+		@Deprecated(forRemoval = true)
+		public static <F, T> Converter<?, T> createInstanceofConverter(Class<F> from, Converter<F, T> conv) {
+			throw new UnsupportedOperationException();
+		}
+
+		@Deprecated(forRemoval = true)
+		public static <F, T> Converter<F, T> createInstanceofConverter(Converter<F, ?> conv, Class<T> to) {
+			throw new UnsupportedOperationException();
+		}
+
+		@Deprecated(forRemoval = true)
+		public static <F, T> Converter<?, T>
+		createDoubleInstanceofConverter(Class<F> from, Converter<F, ?> conv, Class<T> to) {
+			throw new UnsupportedOperationException();
+		}
+
+	}
+
+}

src/main/java/ch/njol/skript/config/Config.java

@@ -22,10 +22,7 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.Iterator;
-import java.util.LinkedHashSet;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
 
 /**
  * Represents a config file.
@@ -219,13 +216,41 @@ public boolean updateNodes(@NotNull Config newer) {
 		Set<Node> newNodes = discoverNodes(newer.getMainNode());
 		Set<Node> oldNodes = discoverNodes(getMainNode());
 
+		// find the nodes that are in the new config but not in the old one
 		newNodes.removeAll(oldNodes);
 		Set<Node> nodesToUpdate = new LinkedHashSet<>(newNodes);
 
 		if (nodesToUpdate.isEmpty())
 			return false;
 
 		for (Node node : nodesToUpdate) {
+			/*
+			 prevents nodes that are already in the config from being added again
+			 this happens when section nodes are added to the config, as their children
+			 are also carried over from the new config, but are also in 'nodesToUpdate'
+
+			 example:
+			 nodesToUpdate is this
+			 - x
+			 - x.y
+			 - x.z
+
+			 and if the method adds x, since x has children in the new config,
+			 it'll add the children to the to-be-updated config, so it'll add
+			 x:
+			   y: 'whatever'
+			   z: 'whatever'
+
+			 but it also wants to add x.y since that node previously did not exist,
+			 but now it does, so it duplicates it without that if statement
+			 x:
+			  y: 'whatever'
+			  y: 'whatever'
+			  z: 'whatever'
+			*/
+			if (get(node.getPathSteps()) != null)
+				continue;
+
 			Skript.debug("Updating node %s", node);
 			SectionNode newParent = node.getParent();
 			Preconditions.checkNotNull(newParent);
@@ -235,16 +260,22 @@ public boolean updateNodes(@NotNull Config newer) {
 
 			int index = node.getIndex();
 			if (index >= parent.size()) {
+				// in case we have some user-added comments or something goes wrong, to ensure index is within bounds
+
 				Skript.debug("Adding node %s to %s (size mismatch)", node, parent);
 				parent.add(node);
 				continue;
 			}
 
 			Node existing = parent.getAt(index);
-			if (existing != null) { // insert between existing
+			if (existing != null) {
+				// there's already something at the node we want to add the new node
+
 				Skript.debug("Adding node %s to %s at index %s", node, parent, index);
 				parent.add(index, node);
 			} else {
+				// there's nothing at the index we want to add the new node
+
 				Skript.debug("Adding node %s to %s", node, parent);
 				parent.add(node);
 			}

src/main/java/ch/njol/skript/config/EntryNode.java

@@ -1,6 +1,8 @@
 package ch.njol.skript.config;
 
+import java.util.Arrays;
 import java.util.Map.Entry;
+import java.util.Objects;
 
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -52,4 +54,20 @@ String save_i() {
 		return null;
 	}
 
+	@Override
+	public boolean equals(Object object) {
+		if (!(object instanceof EntryNode other))
+			return false;
+
+		// ignores comment as changing the comment would create
+		// a new node which may change the value, leading to
+		// unexpected config changes for the user
+		return Arrays.equals(this.getPathSteps(), other.getPathSteps());
+	}
+
+	@Override
+	public int hashCode() {
+		return Arrays.hashCode(this.getPathSteps());
+	}
+
 }

src/main/java/ch/njol/skript/config/Node.java

@@ -476,16 +476,13 @@ int getIndex() {
 			if (node.getKey() == null || node.getKey().isEmpty())
 				break;
 
-			path.add(0, node.getKey() + ".");
+			path.add(0, node.getKey());
 			node = node.getParent();
 		}
 
 		if (path.isEmpty())
 			return new String[0];
 
-		int lastIndex = path.size() - 1;
-		String lastValue = path.get(lastIndex);
-		path.set(lastIndex, lastValue.substring(0, lastValue.length() - 1)); // trim trailing dot
 		return path.toArray(new String[0]);
 	}
 

src/main/java/ch/njol/skript/config/SectionNode.java

@@ -80,6 +80,7 @@ public void add(final Node n) {
 	public void add(int index, @NotNull Node node) {
 		Preconditions.checkArgument(index >= 0 && index <= size(), "index out of bounds: %s", index);
 
+		node.remove();
 		nodes.add(index, node);
 		node.parent = this;
 		node.config = config;

src/main/java/ch/njol/skript/effects/EffKill.java

@@ -20,11 +20,12 @@
 import org.jetbrains.annotations.Nullable;
 
 @Name("Kill")
-@Description({"Kills an entity.",
-		"Note: This effect does not set the entity's health to 0 (which causes issues), but damages the entity by 100 times its maximum health."})
-@Examples({"kill the player",
-		"kill all creepers in the player's world",
-		"kill all endermen, witches and bats"})
+@Description("Kills an entity.")
+@Examples({
+	"kill the player",
+	"kill all creepers in the player's world",
+	"kill all endermen, witches and bats"
+})
 @Since("1.0, 2.10 (ignoring totem of undying)")
 public class EffKill extends Effect {
 

src/main/java/ch/njol/skript/events/EvtFirework.java

@@ -4,39 +4,42 @@
 import ch.njol.skript.lang.Literal;
 import ch.njol.skript.lang.SkriptEvent;
 import ch.njol.skript.lang.SkriptParser.ParseResult;
+import ch.njol.skript.lang.SyntaxStringBuilder;
 import ch.njol.skript.util.Color;
-import java.util.Arrays;
-import java.util.List;
-
 import ch.njol.skript.util.ColorRGB;
 import org.bukkit.FireworkEffect;
 import org.bukkit.event.Event;
 import org.bukkit.event.entity.FireworkExplodeEvent;
 import org.bukkit.inventory.meta.FireworkMeta;
 import org.jetbrains.annotations.Nullable;
 
+import java.util.Set;
+import java.util.stream.Collectors;
 
 public class EvtFirework extends SkriptEvent {
 
 	static {
 		if (Skript.classExists("org.bukkit.event.entity.FireworkExplodeEvent"))
 			//Making the event argument type fireworkeffects, led to Skript having troubles parsing for some reason.
-			Skript.registerEvent("Firework Explode", EvtFirework.class, FireworkExplodeEvent.class, "[a] firework explo(d(e|ing)|sion) [colo[u]red %-colors%]")
+			Skript.registerEvent("Firework Explode", EvtFirework.class, FireworkExplodeEvent.class,
+					"[a] firework explo(d(e|ing)|sion) [colo[u]red %-colors%]")
 					.description("Called when a firework explodes.")
-					.examples("on firework explode:",
-								"\tif event-colors contains red:",
-							"on firework exploding colored red, light green and black:",
-							"on firework explosion colored rgb 0, 255, 0:",
-								"\tbroadcast \"A firework colored %colors% was exploded at %location%!\"")
+					.examples(
+						"on firework explode:",
+							"\tif event-colors contains red:",
+						"on firework exploding colored red, light green and black:",
+						"on firework explosion colored rgb 0, 255, 0:",
+							"\tbroadcast \"A firework colored %colors% was exploded at %location%!\""
+					)
 					.since("2.4");
 	}
 
 	private @Nullable Literal<Color> colors;
-
-	@SuppressWarnings("unchecked")
+
 	@Override
 	public boolean init(Literal<?>[] args, int matchedPattern, ParseResult parseResult) {
 		if (args[0] != null)
+			//noinspection unchecked
 			colors = (Literal<Color>) args[0];
 		return true;
 	}
@@ -49,13 +52,14 @@ public boolean check(Event event) {
 		if (colors == null)
 			return true;
 
-		List<org.bukkit.Color> colours = colors.stream(event)
+		Set<org.bukkit.Color> colours = colors.stream(event)
 			.map(color -> {
 				if (color instanceof ColorRGB)
 					return color.asBukkitColor();
 				return color.asDyeColor().getFireworkColor();
 			})
-			.toList();
+			.collect(Collectors.toSet());
+
 		FireworkMeta meta = fireworkExplodeEvent.getEntity().getFireworkMeta();
 		for (FireworkEffect effect : meta.getEffects()) {
 			if (colours.containsAll(effect.getColors()))
@@ -65,8 +69,14 @@ public boolean check(Event event) {
 	}
 
 	@Override
-	public String toString(@Nullable Event e, boolean debug) {
-		return "Firework explode " + (colors != null ? " with colors " + colors.toString(e, debug) : "");
+	public String toString(@Nullable Event event, boolean debug) {
+		SyntaxStringBuilder builder = new SyntaxStringBuilder(event, debug);
+
+		builder.append("firework explode");
+		if (colors != null)
+			builder.append("with colors").append(colors);
+
+		return builder.toString();
 	}
 
 }

src/main/java/ch/njol/skript/lang/parser/ParserInstance.java

@@ -228,6 +228,8 @@ public void deleteCurrentEvent() {
 	 * See also {@link #isCurrentEvent(Class[])} for checking with multiple argument classes
 	 */
 	public boolean isCurrentEvent(Class<? extends Event> event) {
+		if (currentEvents == null)
+			return false;
 		for (Class<? extends Event> currentEvent : currentEvents) {
 			// check that current event is same or child of event we want
 			if (event.isAssignableFrom(currentEvent))
@@ -622,7 +624,7 @@ public interface ScriptActivityChangeEvent extends ScriptLoader.LoaderEvent, Scr
 	 *  That is, the contents of any collections will remain the same, but there is no guarantee that
 	 *  the contents themselves will remain unchanged.
 	 * @see #backup()
-	 * @see #restoreBackup(Backup) 
+	 * @see #restoreBackup(Backup)
 	 */
 	public static class Backup {
 

src/main/java/ch/njol/skript/localization/Adjective.java

@@ -57,9 +57,7 @@ protected void onValueChange() {
 	@Override
 	public String toString() {
 		validate();
-		if (Skript.testing())
-			Skript.warning("Invalid use of Adjective.toString()");
-		return "" + def;
+		return def;
 	}
 
 	public String toString(int gender, int flags) {