Merge branch 'dev/feature' into feature/remove-inf-value

Author: APickledWalrus

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

@@ -5,6 +5,7 @@
 import ch.njol.skript.config.SectionNode;
 import ch.njol.skript.config.SimpleNode;
 import ch.njol.skript.events.bukkit.PreScriptLoadEvent;
+import ch.njol.skript.lang.ExecutionIntent;
 import ch.njol.skript.lang.Section;
 import ch.njol.skript.lang.SkriptParser;
 import ch.njol.skript.lang.Statement;
@@ -20,7 +21,7 @@
 import ch.njol.skript.util.SkriptColor;
 import ch.njol.skript.util.Task;
 import ch.njol.skript.util.Timespan;
-import ch.njol.skript.variables.TypeHints;
+import ch.njol.skript.variables.HintManager;
 import ch.njol.util.NonNullPair;
 import ch.njol.util.OpenCloseable;
 import ch.njol.util.StringUtils;
@@ -951,6 +952,14 @@ public static ArrayList<TriggerItem> loadItems(SectionNode node) {
 
 		ArrayList<TriggerItem> items = new ArrayList<>();
 
+		// Begin local variable type hints
+		parser.getHintManager().enterScope(true);
+		// Track if the scope has been frozen
+		// This is the case when a statement that stops further execution is encountered
+		// Further statements would not run, meaning the hints from them are inaccurate
+		// When true, before exiting the scope, its hints are cleared to avoid passing them up
+		boolean freezeScope = false;
+
 		boolean executionStops = false;
 		for (Node subNode : node) {
 			parser.setNode(subNode);
@@ -983,11 +992,18 @@ public static ArrayList<TriggerItem> loadItems(SectionNode node) {
 
 				items.add(item);
 			} else if (subNode instanceof SectionNode subSection) {
-				TypeHints.enterScope(); // Begin conditional type hints
 
 				RetainingLogHandler handler = SkriptLogger.startRetainingLog();
 				find_section:
 				try {
+					// enter capturing scope
+					// it is possible that the line may successfully parse and initialize (via init), but some other issue
+					// prevents it from being able to load. for example:
+					// - a statement with a section that has no expression to claim the section
+					// - a statement with a section that has multiple expressions attempting to claim the section
+					// thus, hints may be added, but we do not want to save them as the line is invalid
+					parser.getHintManager().enterScope(false);
+
 					item = Section.parse(expr, "Can't understand this section: " + expr, subSection, items);
 					if (item != null)
 						break find_section;
@@ -1014,6 +1030,13 @@ public static ArrayList<TriggerItem> loadItems(SectionNode node) {
 					}
 					continue;
 				} finally {
+					// exit hint scope (see above)
+					HintManager hintManager = parser.getHintManager();
+					if (item == null) { // unsuccessful, wipe out hints
+						hintManager.clearScope(0, false);
+					}
+					hintManager.exitScope();
+
 					RetainingLogHandler afterParse = handler.backup();
 					handler.clear();
 					handler.printLog();
@@ -1023,9 +1046,6 @@ public static ArrayList<TriggerItem> loadItems(SectionNode node) {
 				}
 
 				items.add(item);
-
-				// Destroy these conditional type hints
-				TypeHints.exitScope();
 			} else {
 				continue;
 			}
@@ -1037,7 +1057,24 @@ public static ArrayList<TriggerItem> loadItems(SectionNode node) {
 				Skript.warning("Unreachable code. The previous statement stops further execution.");
 			}
 			executionStops = item.executionIntent() != null;
+
+			if (executionStops && !freezeScope) {
+				freezeScope = true;
+				// Execution might stop for some sections but not all
+				// We want to pass hints up to the scope that execution resumes in
+				if (item.executionIntent() instanceof ExecutionIntent.StopSections intent) {
+					parser.getHintManager().mergeScope(0, intent.levels(), true);
+				}
+			}
+		}
+
+		// If the scope was frozen, then we need to clear it to prevent passing up inaccurate hints
+		// They will have already been copied as necessary
+		if (freezeScope) {
+			parser.getHintManager().clearScope(0, false);
 		}
+		// Destroy local variable type hints for this section
+		parser.getHintManager().exitScope();
 
 		for (int i = 0; i < items.size() - 1; i++)
 			items.get(i).setNext(items.get(i + 1));

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

@@ -1264,7 +1264,6 @@ public void onDisable() {
 		if (disabled)
 			return;
 		disabled = true;
-		this.experimentRegistry = null;
 
 		if (!partDisabled) {
 			beforeDisable();
@@ -1279,6 +1278,8 @@ public void onDisable() {
 				Skript.exception(e, "An error occurred while shutting down.", "This might or might not cause any issues.");
 			}
 		}
+
+		this.experimentRegistry = null;
 	}
 
 	// ================ CONSTANTS, OPTIONS & OTHER ================

src/main/java/ch/njol/skript/aliases/ItemType.java

@@ -1,5 +1,6 @@
 package ch.njol.skript.aliases;
 
+import ch.njol.skript.Skript;
 import ch.njol.skript.aliases.ItemData.OldItemData;
 import ch.njol.skript.bukkitutil.BukkitUnsafe;
 import ch.njol.skript.bukkitutil.ItemUtils;
@@ -21,6 +22,7 @@
 import ch.njol.yggdrasil.Fields;
 import ch.njol.yggdrasil.Fields.FieldContext;
 import ch.njol.yggdrasil.YggdrasilSerializable.YggdrasilExtendedSerializable;
+import com.google.common.collect.Iterators;
 import org.bukkit.*;
 import org.bukkit.block.Block;
 import org.bukkit.block.BlockState;
@@ -50,6 +52,8 @@
 public class ItemType implements Unit, Iterable<ItemData>, Container<ItemStack>, YggdrasilExtendedSerializable,
 	AnyNamed, AnyAmount {
 
+	private static final boolean IS_RUNNING_1_21 = Skript.isRunningMinecraft(1, 21);
+
 	static {
 		// This handles updating ItemType and ItemData variable records
 		Variables.yggdrasil.registerFieldHandler(new FieldHandler() {
@@ -710,20 +714,18 @@ public static ItemStack[] getCopiedContents(Inventory invi) {
 	}
 
 	/**
-	 * Gets copy of storage contents, i.e. ignores armor and off hand. This is due to Spigot 1.9
-	 * added armor slots, and off hand to default inventory index.
-	 * @param invi Inventory
+	 * Gets copy of storage contents, i.e. ignores armor and off hand.
+	 * This method simply calls {@link Inventory#getStorageContents()} and clones the items contained within the array.
+	 * @param inventory The inventory to obtain contents from.
 	 * @return Copied storage contents
 	 */
-	public static ItemStack[] getStorageContents(final Inventory invi) {
-		if (invi instanceof PlayerInventory) {
-			ItemStack[] buf = invi.getContents();
-			ItemStack[] tBuf = new ItemStack[36];
-			for (int i = 0; i < 36; i++)
-				if (buf[i] != null)
-					tBuf[i] = buf[i].clone();
-			return tBuf;
-		} else return getCopiedContents(invi);
+	public static ItemStack[] getStorageContents(Inventory inventory) {
+		ItemStack[] buf = inventory.getStorageContents();
+		for (int i = 0; i < buf.length; i++) {
+			if (buf[i] != null)
+				buf[i] = buf[i].clone();
+		}
+		return buf;
 	}
 
 	/**
@@ -969,33 +971,46 @@ public void addTo(final List<ItemStack> list) {
 	/**
 	 * Tries to add this ItemType to the given inventory. Does not call updateInventory for players.
 	 *
-	 * @param invi
+	 * @param inventory The inventory to add this the {@link ItemStack}(s) represented by this ItemType to.
 	 * @return Whether everything could be added to the inventory
 	 */
-	public boolean addTo(final Inventory invi) {
-		// important: don't use inventory.add() - it ignores max stack sizes
-		ItemStack[] buf = invi.getContents();
-
-		ItemStack[] tBuf = buf.clone();
-		if (invi instanceof PlayerInventory) {
-			buf = new ItemStack[36];
-			for(int i = 0; i < 36; ++i) {
-				buf[i] = tBuf[i];
+	public boolean addTo(Inventory inventory) {
+		// TODO remove this when applicable
+		// On newer versions, such as 1.21.6, this legacy method of manually rewriting inventory content arrays risks
+		//  accidental item deletion and fails to respect properties such as stack size.
+		// Thus, we switch to use the API methods. However, these API methods do not work properly on older versions
+		//  such as 1.20.6. For those versions, we continue to use this legacy method.
+		// See https://github.com/SkriptLang/Skript/pull/7986
+		if (!IS_RUNNING_1_21) {
+			// important: don't use inventory.add() - it ignores max stack sizes
+			ItemStack[] buf = inventory.getContents();
+
+			ItemStack[] tBuf = buf.clone();
+			if (inventory instanceof PlayerInventory) {
+				buf = new ItemStack[36];
+				for(int i = 0; i < 36; ++i) {
+					buf[i] = tBuf[i];
+				}
 			}
-		}
 
-		final boolean b = addTo(buf);
+			final boolean b = addTo(buf);
 
-		if (invi instanceof PlayerInventory) {
-			buf = Arrays.copyOf(buf, tBuf.length);
-			for (int i = tBuf.length - 5; i < tBuf.length; ++i) {
-				buf[i] = tBuf[i];
+			if (inventory instanceof PlayerInventory) {
+				buf = Arrays.copyOf(buf, tBuf.length);
+				for (int i = tBuf.length - 5; i < tBuf.length; ++i) {
+					buf[i] = tBuf[i];
+				}
 			}
-		}
 
-		assert buf != null;
-		invi.setContents(buf);
-		return b;
+			assert buf != null;
+			inventory.setContents(buf);
+			return b;
+		}
+		if (!isAll()) {
+			ItemStack random = getItem().getRandom();
+			return random == null || inventory.addItem(random).isEmpty();
+		}
+		return inventory.addItem(Iterators.toArray(getItem().getAll().iterator(), ItemStack.class)).isEmpty();
 	}
 
 	private static boolean addTo(@Nullable ItemStack is, ItemStack[] buf) {

src/main/java/ch/njol/skript/command/Argument.java

@@ -130,6 +130,10 @@ public void set(final ScriptCommandEvent e, final Object[] o) {
 	public T[] getCurrent(final Event e) {
 		return current.get(e);
 	}
+
+	public @Nullable String getName() {
+		return name;
+	}
 
 	public Class<T> getType() {
 		return type.getC();

src/main/java/ch/njol/skript/command/ScriptCommand.java

@@ -8,7 +8,9 @@
 import ch.njol.skript.lang.Expression;
 import ch.njol.skript.lang.SkriptParser;
 import ch.njol.skript.lang.Trigger;
+import ch.njol.skript.lang.Variable;
 import ch.njol.skript.lang.VariableString;
+import ch.njol.skript.lang.parser.ParserInstance;
 import ch.njol.skript.lang.util.SimpleEvent;
 import ch.njol.skript.lang.util.SimpleLiteral;
 import ch.njol.skript.localization.Language;
@@ -23,6 +25,7 @@
 import ch.njol.skript.util.Utils;
 import ch.njol.skript.util.chat.BungeeConverter;
 import ch.njol.skript.util.chat.MessageComponent;
+import ch.njol.skript.variables.HintManager;
 import ch.njol.skript.variables.Variables;
 import ch.njol.util.StringUtils;
 import com.google.common.base.Preconditions;
@@ -204,8 +207,24 @@ public ScriptCommand(
 		this.pattern = pattern;
 		this.arguments = arguments;
 
-		trigger = new Trigger(script, "command /" + name, new SimpleEvent(), ScriptLoader.loadItems(node));
-		trigger.setLineNumber(node.getLine());
+		HintManager hintManager = ParserInstance.get().getHintManager();
+		try {
+			hintManager.enterScope(false);
+			for (Argument<?> argument : arguments) {
+				String hintName = argument.getName();
+				if (hintName == null) {
+					continue;
+				}
+				if (!argument.isSingle()) {
+					hintName += Variable.SEPARATOR + "*";
+				}
+				hintManager.set(hintName, argument.getType());
+			}
+			trigger = new Trigger(script, "command /" + name, new SimpleEvent(), ScriptLoader.loadItems(node));
+			trigger.setLineNumber(node.getLine());
+		} finally {
+			hintManager.exitScope();
+		}
 
 		bukkitCommand = setupBukkitCommand();
 	}

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

@@ -15,6 +15,7 @@
 import ch.njol.skript.lang.SyntaxStringBuilder;
 import ch.njol.skript.lang.Variable;
 import ch.njol.skript.util.LiteralUtils;
+import ch.njol.skript.variables.HintManager;
 import org.skriptlang.skript.lang.script.ScriptWarning;
 import org.bukkit.event.Event;
 import org.jetbrains.annotations.Nullable;
@@ -180,6 +181,12 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
 			flatAcceptedTypes[i] = type;
 		}
 
+		// Hint handling for deleting
+		if (changed instanceof Variable<?> variable && mode == ChangeMode.DELETE && HintManager.canUseHints(variable)) {
+			// Remove type hints in this scope only for a deleted variable
+			getParser().getHintManager().delete(variable);
+		}
+
 		if (changer == null) { // Safe to reset/delete
 			return true;
 		}
@@ -271,13 +278,24 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
 			}
 
 			// Print warning if attempting to save a non-serializable type in a global variable
-			if (!variable.isLocal() && (mode == ChangeMode.SET || (variable.isList() && mode == ChangeMode.ADD))) {
-				ClassInfo<?> changerInfo = Classes.getSuperClassInfo(changer.getReturnType());
-				if (changerInfo.getC() != Object.class && changerInfo.getSerializer() == null && changerInfo.getSerializeAs() == null
-					&& !SkriptConfig.disableObjectCannotBeSavedWarnings.value()
-					&& getParser().isActive() && !getParser().getCurrentScript().suppressesWarning(ScriptWarning.VARIABLE_SAVE)) {
-					Skript.warning(changerInfo.getName().withIndefiniteArticle() + " cannot be saved. That is, the contents of the variable "
-						+ changed.toString(null, Skript.debug()) + " will be lost when the server stops.");
+			if (mode == ChangeMode.SET || (variable.isList() && mode == ChangeMode.ADD)) {
+				if (HintManager.canUseHints(variable)) { // Hint handling
+					HintManager hintManager = getParser().getHintManager();
+					Class<?>[] hints = changer.possibleReturnTypes();
+					if (mode == ChangeMode.SET) { // Override existing hints in scope
+						hintManager.set(variable, hints);
+					} else {
+						hintManager.add(variable, hints);
+					}
+				}
+				if (!variable.isLocal()) {
+					ClassInfo<?> changerInfo = Classes.getSuperClassInfo(changer.getReturnType());
+					if (changerInfo.getC() != Object.class && changerInfo.getSerializer() == null && changerInfo.getSerializeAs() == null
+						&& !SkriptConfig.disableObjectCannotBeSavedWarnings.value()
+						&& getParser().isActive() && !getParser().getCurrentScript().suppressesWarning(ScriptWarning.VARIABLE_SAVE)) {
+						Skript.warning(changerInfo.getName().withIndefiniteArticle() + " cannot be saved. That is, the contents of the variable "
+							+ changed.toString(null, Skript.debug()) + " will be lost when the server stops.");
+					}
 				}
 			}
 		}

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

@@ -13,6 +13,7 @@
 import ch.njol.skript.lang.SkriptParser.ParseResult;
 import ch.njol.skript.lang.Variable;
 import ch.njol.skript.registrations.Classes;
+import ch.njol.skript.variables.HintManager;
 import ch.njol.skript.variables.Variables;
 import ch.njol.util.Kleenean;
 import org.bukkit.event.Event;
@@ -64,6 +65,16 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
 				return false;
 			}
 		}
+
+		// set type hints for destination(s) if applicable
+		Class<?>[] sourceHints = source.possibleReturnTypes();
+		HintManager hintManager = getParser().getHintManager();
+		for (Variable<?> destination : destinations) {
+			if (HintManager.canUseHints(destination)) {
+				hintManager.set(destination, sourceHints);
+			}
+		}
+
 		return true;
 	}