🍒 [2.7] Structure API Finalization (#5669) (#5981)

Structure API Finalization (#5669)

Co-authored-by: Patrick Miller <[email protected]>

Author: AyhamAl-Ali

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

@@ -516,67 +516,89 @@ private static CompletableFuture<ScriptInfo> loadScripts(List<Config> configs, O
 				try {
 					openCloseable.open();
 
-					scripts.stream()
-						.flatMap(pair -> { // Flatten each entry down to a stream of Script-Structure pairs
-							return pair.getSecond().stream()
-								.map(structure -> new NonNullPair<>(pair, structure));
-						})
-						.sorted(Comparator.comparing(pair -> pair.getSecond().getPriority()))
-						.forEach(pair -> {
-							Script script = pair.getFirst().getFirst();
-							Structure structure = pair.getSecond();
-
-							parser.setActive(script);
-							parser.setCurrentStructure(structure);
-							parser.setNode(structure.getEntryContainer().getSource());
-
-							try {
-								if (!structure.preLoad())
-									pair.getFirst().getSecond().remove(structure);
-							} catch (Exception e) {
-								//noinspection ThrowableNotThrown
-								Skript.exception(e, "An error occurred while trying to load a Structure.");
+					// build sorted list
+					// this nest of pairs is terrible, but we need to keep the reference to the modifiable structures list
+					List<NonNullPair<NonNullPair<Script, List<Structure>>, Structure>> pairs = scripts.stream()
+							.flatMap(pair -> { // Flatten each entry down to a stream of Script-Structure pairs
+								return pair.getSecond().stream()
+										.map(structure -> new NonNullPair<>(pair, structure));
+							})
+							.sorted(Comparator.comparing(pair -> pair.getSecond().getPriority()))
+							.collect(Collectors.toCollection(ArrayList::new));
+
+					// pre-loading
+					pairs.removeIf(pair -> {
+						Structure structure = pair.getSecond();
+
+						parser.setActive(pair.getFirst().getFirst());
+						parser.setCurrentStructure(structure);
+						parser.setNode(structure.getEntryContainer().getSource());
+
+						try {
+							if (!structure.preLoad()) {
 								pair.getFirst().getSecond().remove(structure);
+								return true;
 							}
-						});
-
+						} catch (Exception e) {
+							//noinspection ThrowableNotThrown
+							Skript.exception(e, "An error occurred while trying to preLoad a Structure.");
+							pair.getFirst().getSecond().remove(structure);
+							return true;
+						}
+						return false;
+					});
 					parser.setInactive();
 
-					// TODO in the future, Structure#load should be split across multiple threads if parallel loading is enabled.
+					// TODO in the future, Structure#load/Structure#postLoad should be split across multiple threads if parallel loading is enabled.
 					// However, this is not possible right now as reworks in multiple areas will be needed.
 					// For example, the "Commands" class still uses a static list for currentArguments that is cleared between loads.
 					// Until these reworks happen, limiting main loading to asynchronous (not parallel) is the only choice we have.
-					for (NonNullPair<Script, List<Structure>> pair : scripts) {
-						parser.setActive(pair.getFirst());
-						pair.getSecond().removeIf(structure -> {
-							parser.setCurrentStructure(structure);
-							parser.setNode(structure.getEntryContainer().getSource());
-							try {
-								return !structure.load();
-							} catch (Exception e) {
-								//noinspection ThrowableNotThrown
-								Skript.exception(e, "An error occurred while trying to load a Structure.");
+
+					// loading
+					pairs.removeIf(pair -> {
+						Structure structure = pair.getSecond();
+
+						parser.setActive(pair.getFirst().getFirst());
+						parser.setCurrentStructure(structure);
+						parser.setNode(structure.getEntryContainer().getSource());
+
+						try {
+							if (!structure.load()) {
+								pair.getFirst().getSecond().remove(structure);
 								return true;
 							}
-						});
-					}
-
+						} catch (Exception e) {
+							//noinspection ThrowableNotThrown
+							Skript.exception(e, "An error occurred while trying to load a Structure.");
+							pair.getFirst().getSecond().remove(structure);
+							return true;
+						}
+						return false;
+					});
 					parser.setInactive();
 
-					for (NonNullPair<Script, List<Structure>> pair : scripts) {
-						parser.setActive(pair.getFirst());
-						pair.getSecond().removeIf(structure -> {
-							parser.setCurrentStructure(structure);
-							parser.setNode(structure.getEntryContainer().getSource());
-							try {
-								return !structure.postLoad();
-							} catch (Exception e) {
-								//noinspection ThrowableNotThrown
-								Skript.exception(e, "An error occurred while trying to load a Structure.");
+					// post-loading
+					pairs.removeIf(pair -> {
+						Structure structure = pair.getSecond();
+
+						parser.setActive(pair.getFirst().getFirst());
+						parser.setCurrentStructure(structure);
+						parser.setNode(structure.getEntryContainer().getSource());
+
+						try {
+							if (!structure.postLoad()) {
+								pair.getFirst().getSecond().remove(structure);
 								return true;
 							}
-						});
-					}
+						} catch (Exception e) {
+							//noinspection ThrowableNotThrown
+							Skript.exception(e, "An error occurred while trying to postLoad a Structure.");
+							pair.getFirst().getSecond().remove(structure);
+							return true;
+						}
+						return false;
+					});
+					parser.setInactive();
 
 					return scriptInfo;
 				} catch (Exception e) {
@@ -593,7 +615,7 @@ private static CompletableFuture<ScriptInfo> loadScripts(List<Config> configs, O
 	/**
 	 * Creates a script and loads the provided config into it.
 	 * @param config The config to load into a script.
-	 * @return The script that was loaded.
+	 * @return A pair containing the script that was loaded and a modifiable version of the structures list.
 	 */
 	// Whenever you call this method, make sure to also call PreScriptLoadEvent
 	private static NonNullPair<Script, List<Structure>> loadScript(Config config) {
@@ -1009,20 +1031,6 @@ public static FileFilter getDisabledScriptsFilter() {
 	 * by new methods in this class.
 	 */
 
-	/**
-	 * Reloads a single script.
-	 * @param scriptFile The file representing the script to reload.
-	 * @return Future of statistics of the newly loaded script.
-	 * @deprecated Use {@link #reloadScript(Script, OpenCloseable)}.
-	 */
-	@Deprecated
-	public static CompletableFuture<ScriptInfo> reloadScript(File scriptFile, OpenCloseable openCloseable) {
-		Script script = getScript(scriptFile);
-		if (script == null)
-			return CompletableFuture.completedFuture(new ScriptInfo());
-		return reloadScript(script, openCloseable);
-	}
-
 	/**
 	 * Unloads the provided script.
 	 * @param scriptFile The file representing the script to unload.
@@ -1049,6 +1057,18 @@ private static ScriptInfo unloadScripts(File folder) {
 		return unloadScripts(getScripts(folder));
 	}
 
+	/**
+	 * Reloads a single script.
+	 * @param scriptFile The file representing the script to reload.
+	 * @return Future of statistics of the newly loaded script.
+	 * @deprecated Use {@link #reloadScript(Script, OpenCloseable)}.
+	 */
+	@Deprecated
+	public static CompletableFuture<ScriptInfo> reloadScript(File scriptFile, OpenCloseable openCloseable) {
+		unloadScript(scriptFile);
+		return loadScripts(scriptFile, openCloseable);
+	}
+
 	/**
 	 * Reloads all scripts in the given folder and its subfolders.
 	 * @param folder A folder.
@@ -1058,7 +1078,7 @@ private static ScriptInfo unloadScripts(File folder) {
 	@Deprecated
 	public static CompletableFuture<ScriptInfo> reloadScripts(File folder, OpenCloseable openCloseable) {
 		unloadScripts(folder);
-		return loadScripts(loadStructures(folder), openCloseable);
+		return loadScripts(folder, openCloseable);
 	}
 
 	/**

src/main/java/ch/njol/skript/structures/StructCommand.java

@@ -96,7 +96,7 @@ public class StructCommand extends Structure {
 				.addEntry("description", "", true)
 				.addEntry("prefix", null, true)
 				.addEntry("permission", "", true)
-				.addEntryData(new VariableStringEntryData("permission message", null, true, ScriptCommandEvent.class))
+				.addEntryData(new VariableStringEntryData("permission message", null, true))
 				.addEntryData(new KeyValueEntryData<List<String>>("aliases", new ArrayList<>(), true) {
 					private final Pattern pattern = Pattern.compile("\\s*,\\s*/?");
 
@@ -131,9 +131,9 @@ protected Integer getValue(String value) {
 					}
 				})
 				.addEntryData(new LiteralEntryData<>("cooldown", null, true, Timespan.class))
-				.addEntryData(new VariableStringEntryData("cooldown message", null, true, ScriptCommandEvent.class))
+				.addEntryData(new VariableStringEntryData("cooldown message", null, true))
 				.addEntry("cooldown bypass", null, true)
-				.addEntryData(new VariableStringEntryData("cooldown storage", null, true, StringMode.VARIABLE_NAME, ScriptCommandEvent.class))
+				.addEntryData(new VariableStringEntryData("cooldown storage", null, true, StringMode.VARIABLE_NAME))
 				.addSection("trigger", false)
 				.unexpectedEntryMessage(key ->
 					"Unexpected entry '" + key + "'. Check that it's spelled correctly, and ensure that you have put all code into a trigger."
@@ -152,7 +152,6 @@ public boolean init(Literal<?>[] args, int matchedPattern, ParseResult parseResu
 	}
 
 	@Override
-	@SuppressWarnings("unchecked")
 	public boolean load() {
 		getParser().setCurrentEvent("command", ScriptCommandEvent.class);
 
@@ -272,8 +271,8 @@ public boolean load() {
 		if (permissionMessage != null && permission.isEmpty())
 			Skript.warning("command /" + command + " has a permission message set, but not a permission");
 
-		List<String> aliases = (List<String>) entryContainer.get("aliases", true);
-		int executableBy = (Integer) entryContainer.get("executable by", true);
+		List<String> aliases = entryContainer.get("aliases", List.class,true);
+		int executableBy = entryContainer.get("executable by", Integer.class, true);
 
 		Timespan cooldown = entryContainer.getOptional("cooldown", Timespan.class, false);
 		VariableString cooldownMessage = entryContainer.getOptional("cooldown message", VariableString.class, false);

src/main/java/ch/njol/skript/structures/StructOptions.java

@@ -32,6 +32,7 @@
 import org.bukkit.event.Event;
 import org.jetbrains.annotations.Nullable;
 import org.skriptlang.skript.lang.entry.EntryContainer;
+import org.skriptlang.skript.lang.script.Script;
 import org.skriptlang.skript.lang.script.ScriptData;
 import org.skriptlang.skript.lang.structure.Structure;
 
@@ -75,20 +76,16 @@ public class StructOptions extends Structure {
 	public boolean init(Literal<?>[] args, int matchedPattern, ParseResult parseResult, EntryContainer entryContainer) {
 		SectionNode node = entryContainer.getSource();
 		node.convertToEntries(-1);
-
-		OptionsData optionsData = new OptionsData();
-		loadOptions(node, "", optionsData.options);
-		getParser().getCurrentScript().addData(optionsData);
-
+		loadOptions(node, "", getParser().getCurrentScript().getData(OptionsData.class, OptionsData::new).options);
 		return true;
 	}
 
 	private void loadOptions(SectionNode sectionNode, String prefix, Map<String, String> options) {
-		for (Node n : sectionNode) {
-			if (n instanceof EntryNode) {
-				options.put(prefix + n.getKey(), ((EntryNode) n).getValue());
-			} else if (n instanceof SectionNode) {
-				loadOptions((SectionNode) n, prefix + n.getKey() + ".", options);
+		for (Node node : sectionNode) {
+			if (node instanceof EntryNode) {
+				options.put(prefix + node.getKey(), ((EntryNode) node).getValue());
+			} else if (node instanceof SectionNode) {
+				loadOptions((SectionNode) node, prefix + node.getKey() + ".", options);
 			} else {
 				Skript.error("Invalid line in options");
 			}
@@ -111,7 +108,7 @@ public Priority getPriority() {
 	}
 
 	@Override
-	public String toString(@Nullable Event e, boolean debug) {
+	public String toString(@Nullable Event event, boolean debug) {
 		return "options";
 	}
 

src/main/java/org/skriptlang/skript/lang/entry/EntryContainer.java

@@ -89,11 +89,11 @@ public List<Node> getUnhandledNodes() {
 	 * @return The entry's value.
 	 * @throws RuntimeException If the entry's value is null, or if it is not of the expected type.
 	 */
-	public <T> T get(String key, Class<T> expectedType, boolean useDefaultValue) {
-		T parsed = getOptional(key, expectedType, useDefaultValue);
-		if (parsed == null)
+	public <E, R extends E> R get(String key, Class<E> expectedType, boolean useDefaultValue) {
+		R value = getOptional(key, expectedType, useDefaultValue);
+		if (value == null)
 			throw new RuntimeException("Null value for asserted non-null value");
-		return parsed;
+		return value;
 	}
 
 	/**
@@ -124,13 +124,13 @@ public Object get(String key, boolean useDefaultValue) {
 	 */
 	@Nullable
 	@SuppressWarnings("unchecked")
-	public <T> T getOptional(String key, Class<T> expectedType, boolean useDefaultValue) {
+	public <E, R extends E> R getOptional(String key, Class<E> expectedType, boolean useDefaultValue) {
 		Object parsed = getOptional(key, useDefaultValue);
 		if (parsed == null)
 			return null;
 		if (!expectedType.isInstance(parsed))
 			throw new RuntimeException("Expected entry with key '" + key + "' to be '" + expectedType + "', but got '" + parsed.getClass() + "'");
-		return (T) parsed;
+		return (R) parsed;
 	}
 
 	/**