From caa467981f091715216213187c0e9bddd27ab078 Mon Sep 17 00:00:00 2001 From: _tud <98935832+UnderscoreTud@users.noreply.github.com> Date: Wed, 1 Apr 2026 19:35:51 +0300 Subject: [PATCH 1/6] Implement numerical index tracking to optimize adding to lists --- .../java/ch/njol/skript/lang/Variable.java | 6 + .../njol/skript/variables/VariablesMap.java | 7 +- .../skript/util/IndexTrackingTreeMap.java | 127 +++++++++ .../skript/util/IndexTrackingTreeMapTest.java | 258 ++++++++++++++++++ 4 files changed, 395 insertions(+), 3 deletions(-) create mode 100644 src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java create mode 100644 src/test/java/org/skriptlang/skript/util/IndexTrackingTreeMapTest.java diff --git a/src/main/java/ch/njol/skript/lang/Variable.java b/src/main/java/ch/njol/skript/lang/Variable.java index ef437d93040..35cc1e242c3 100644 --- a/src/main/java/ch/njol/skript/lang/Variable.java +++ b/src/main/java/ch/njol/skript/lang/Variable.java @@ -14,6 +14,7 @@ import ch.njol.skript.structures.StructVariables.DefaultVariables; import ch.njol.skript.util.StringMode; import ch.njol.skript.util.Utils; +import org.skriptlang.skript.util.IndexTrackingTreeMap; import ch.njol.skript.variables.Variables; import ch.njol.util.Kleenean; import ch.njol.util.Pair; @@ -594,6 +595,11 @@ public void change(Event event, Object @Nullable [] delta, ChangeMode mode) thro } } else { assert mode == ChangeMode.ADD; + if (map instanceof IndexTrackingTreeMap indexTrackingMap) { + for (Object value : delta) + indexTrackingMap.add(value); + return; + } int i = 1; for (Object value : delta) { if (map != null) diff --git a/src/main/java/ch/njol/skript/variables/VariablesMap.java b/src/main/java/ch/njol/skript/variables/VariablesMap.java index 9934f46eba5..bc366e7b44f 100644 --- a/src/main/java/ch/njol/skript/variables/VariablesMap.java +++ b/src/main/java/ch/njol/skript/variables/VariablesMap.java @@ -3,6 +3,7 @@ import ch.njol.skript.lang.Variable; import ch.njol.util.StringUtils; import org.jetbrains.annotations.Nullable; +import org.skriptlang.skript.util.IndexTrackingTreeMap; import java.util.Comparator; import java.util.HashMap; @@ -216,7 +217,7 @@ void setVariable(String name, @Nullable Object value) { break; } else if (value != null) { // Create child node, add it to parent and continue iteration - childNode = new TreeMap<>(VARIABLE_NAME_COMPARATOR); + childNode = new IndexTrackingTreeMap<>(VARIABLE_NAME_COMPARATOR); parent.put(childNodeName, childNode); parent = (TreeMap) childNode; @@ -269,7 +270,7 @@ void setVariable(String name, @Nullable Object value) { break; } else if (value != null) { // Need to continue iteration, create new child node and put old value in it - TreeMap newChildNodeMap = new TreeMap<>(VARIABLE_NAME_COMPARATOR); + TreeMap newChildNodeMap = new IndexTrackingTreeMap<>(VARIABLE_NAME_COMPARATOR); newChildNodeMap.put(null, childNode); // Add new child node to parent @@ -334,7 +335,7 @@ public VariablesMap copy() { */ @SuppressWarnings("unchecked") private static TreeMap copyTreeMap(TreeMap original) { - TreeMap copy = new TreeMap<>(VARIABLE_NAME_COMPARATOR); + TreeMap copy = new IndexTrackingTreeMap<>(VARIABLE_NAME_COMPARATOR); for (Entry child : original.entrySet()) { String key = child.getKey(); diff --git a/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java b/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java new file mode 100644 index 00000000000..1e5ba65b788 --- /dev/null +++ b/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java @@ -0,0 +1,127 @@ +package org.skriptlang.skript.util; + +import org.jetbrains.annotations.VisibleForTesting; + +import java.util.*; + +/** + * A {@link TreeMap} that supports automatically assigning the next available + * positive integer key, represented as a string. + * + *

In addition to arbitrary string keys, this map can be used with + * positive integer string keys such as {@code "1"}, {@code "2"}, and + * {@code "3"}. The {@link #add(Object)} method inserts a value using the + * next available integer key.

+ * + * @param the type of mapped values + */ +public class IndexTrackingTreeMap extends TreeMap { + + @VisibleForTesting + final List numericalIndices = new ArrayList<>(); + + public IndexTrackingTreeMap() { + super(); + } + + public IndexTrackingTreeMap(Comparator comparator) { + super(comparator); + } + + @Override + public V put(String key, V value) { + V previous = super.put(key, value); + if (previous == null && isInt(key)) { + try { + int index = Integer.parseInt(key); + int pos = Collections.binarySearch(numericalIndices, index); + numericalIndices.add(-pos - 1, index); + } catch (NumberFormatException ignore) {} + } else if (previous != null && value == null) { + handleRemove(key); + } + return previous; + } + + /** + * Adds the given value under the first available positive integer key. + * + * @param value the value to add + */ + public void add(V value) { + int index = nextOpenIndex(); + super.put(String.valueOf(index), value); + numericalIndices.add(index - 1, index); + } + + @Override + public V remove(Object key) { + V value = super.remove(key); + if (value != null && key instanceof String index) + handleRemove(index); + return value; + } + + @Override + public void clear() { + super.clear(); + numericalIndices.clear(); + } + + /** + * Finds the first available positive integer index that is not currently + * used as a key in this map. + * + *

This method inspects tracked numeric keys and returns the smallest + * missing index, starting at {@code 1}.

+ * + * @return the next available positive integer index + */ + public int nextOpenIndex() { + int size = numericalIndices.size(); + if (size == 0) + return 1; + + if (numericalIndices.getFirst() > 1) + return 1; + + int left = 0; + int right = size - 1; + + while (left <= right) { + int midpoint = left + (right - left >>> 1); + + if (numericalIndices.get(midpoint) == midpoint + 1) { + left = midpoint + 1; + } else { + right = midpoint - 1; + } + } + + return left + 1; + } + + private void handleRemove(String index) { + if (!isInt(index)) + return; + int pos = Collections.binarySearch(numericalIndices, Integer.parseInt(index)); + if (pos >= 0) + numericalIndices.remove(pos); + } + + private boolean isInt(String string) { + if (string == null || string.isBlank() || string.charAt(0) == '0') // Don't handle leading-zero integers + return false; + for (int i = 0; i < string.length(); i++) { + if (!isDigit(string.charAt(i))) + return false; + } + + return true; + } + + private boolean isDigit(int codepoint) { + return codepoint >= '0' && codepoint <= '9'; + } + +} diff --git a/src/test/java/org/skriptlang/skript/util/IndexTrackingTreeMapTest.java b/src/test/java/org/skriptlang/skript/util/IndexTrackingTreeMapTest.java new file mode 100644 index 00000000000..0036c560108 --- /dev/null +++ b/src/test/java/org/skriptlang/skript/util/IndexTrackingTreeMapTest.java @@ -0,0 +1,258 @@ +package org.skriptlang.skript.util; + +import org.junit.Test; + +import java.util.Map; + +import static org.junit.Assert.*; + +@SuppressWarnings("OverwrittenKey") +public class IndexTrackingTreeMapTest { + + private IndexTrackingTreeMap newMap() { + return new IndexTrackingTreeMap<>(); + } + + @Test + public void nextOpenIndexReturnsOneWhenMapIsEmpty() { + IndexTrackingTreeMap map = newMap(); + + assertEquals(1, map.nextOpenIndex()); + } + + @Test + public void addUsesKeyOneWhenMapIsEmpty() { + IndexTrackingTreeMap map = newMap(); + + map.add("value"); + + assertEquals("value", map.get("1")); + assertEquals(1, map.size()); + assertEquals(2, map.nextOpenIndex()); + } + + @Test + public void nextOpenIndexReturnsOneWhenFirstNumericKeyIsMissing() { + IndexTrackingTreeMap map = newMap(); + map.put("2", "two"); + map.put("3", "three"); + + assertEquals(1, map.nextOpenIndex()); + } + + @Test + public void nextOpenIndexReturnsNextValueForConsecutiveKeys() { + IndexTrackingTreeMap map = newMap(); + map.put("1", "one"); + map.put("2", "two"); + map.put("3", "three"); + + assertEquals(4, map.nextOpenIndex()); + } + + @Test + public void nextOpenIndexReturnsFirstGapInMiddle() { + IndexTrackingTreeMap map = newMap(); + map.put("1", "one"); + map.put("2", "two"); + map.put("4", "four"); + + assertEquals(3, map.nextOpenIndex()); + } + + @Test + public void nextOpenIndexReturnsFirstGapWhenMultipleGapsExist() { + IndexTrackingTreeMap map = newMap(); + map.put("1", "one"); + map.put("3", "three"); + map.put("5", "five"); + + assertEquals(2, map.nextOpenIndex()); + } + + @Test + public void nonNumericKeysDoNotAffectNextOpenIndex() { + IndexTrackingTreeMap map = newMap(); + map.put("foo", "foo"); + map.put("bar", "bar"); + + assertEquals(1, map.nextOpenIndex()); + } + + @Test + public void mixedNumericAndNonNumericKeysOnlyTrackNumericOnes() { + IndexTrackingTreeMap map = newMap(); + map.put("1", "one"); + map.put("foo", "foo"); + map.put("3", "three"); + map.put("bar", "bar"); + + assertEquals(2, map.nextOpenIndex()); + } + + @Test + public void overwriteExistingNumericKeyDoesNotDuplicateTracking() { + IndexTrackingTreeMap map = newMap(); + map.put("1", "one"); + map.put("1", "updated"); + + assertEquals(1, map.size()); + assertEquals("updated", map.get("1")); + assertEquals(2, map.nextOpenIndex()); + } + + @Test + public void overwriteExistingNonNumericKeyDoesNotAffectTracking() { + IndexTrackingTreeMap map = newMap(); + map.put("1", "one"); + map.put("foo", "foo"); + map.put("foo", "updated"); + + assertEquals(2, map.nextOpenIndex()); + } + + @Test + public void removeExistingNumericKeyReopensThatSlot() { + IndexTrackingTreeMap map = newMap(); + map.put("1", "one"); + map.put("2", "two"); + map.put("3", "three"); + + map.remove("2"); + + assertEquals(2, map.nextOpenIndex()); + } + + @Test + public void removeFirstNumericKeyReopensOne() { + IndexTrackingTreeMap map = newMap(); + map.put("1", "one"); + map.put("2", "two"); + + map.remove("1"); + + assertEquals(1, map.nextOpenIndex()); + } + + @Test + public void removeLastNumericKeyDoesNotAffectEarlierGapDetection() { + IndexTrackingTreeMap map = newMap(); + map.put("1", "one"); + map.put("2", "two"); + map.put("4", "four"); + + map.remove("4"); + + assertEquals(3, map.nextOpenIndex()); + } + + @Test + public void removeNonNumericKeyDoesNotAffectTracking() { + IndexTrackingTreeMap map = newMap(); + map.put("1", "one"); + map.put("foo", "foo"); + + map.remove("foo"); + + assertEquals(2, map.nextOpenIndex()); + } + + @Test + public void removeMissingKeyDoesNothing() { + IndexTrackingTreeMap map = newMap(); + map.put("1", "one"); + map.put("2", "two"); + + map.remove("9"); + + assertEquals(3, map.nextOpenIndex()); + } + + @Test + public void addReusesFirstGap() { + IndexTrackingTreeMap map = newMap(); + map.put("1", "one"); + map.put("3", "three"); + + map.add("two"); + + assertEquals("two", map.get("2")); + assertEquals(4, map.nextOpenIndex()); + } + + @Test + public void repeatedAddCreatesSequentialNumericKeys() { + IndexTrackingTreeMap map = newMap(); + + map.add("one"); + map.add("two"); + map.add("three"); + + assertEquals("one", map.get("1")); + assertEquals("two", map.get("2")); + assertEquals("three", map.get("3")); + assertEquals(4, map.nextOpenIndex()); + } + + @Test + public void zeroIsIgnoredAsNumericKey() { + IndexTrackingTreeMap map = newMap(); + map.put("0", "zero"); + + assertEquals(1, map.nextOpenIndex()); + } + + @Test + public void leadingZeroKeyBehaviorIsExplicit() { + IndexTrackingTreeMap map = newMap(); + map.put("01", "leading-zero"); + + assertEquals(1, map.nextOpenIndex()); + } + + @Test + public void alphaNumericKeyIsIgnored() { + IndexTrackingTreeMap map = newMap(); + map.put("1a", "value"); + map.put("a1", "value"); + + assertEquals(1, map.nextOpenIndex()); + } + + @Test + public void clearResetsTracking() { + IndexTrackingTreeMap map = newMap(); + map.put("1", "one"); + map.put("2", "two"); + + map.clear(); + + //noinspection ConstantValue + assertTrue(map.isEmpty()); + assertEquals(1, map.nextOpenIndex()); + } + + @Test + public void putAllKeepsTrackingCorrect() { + IndexTrackingTreeMap map = newMap(); + + map.putAll(Map.of( + "1", "one", + "3", "three", + "foo", "foo" + )); + + assertEquals(2, map.nextOpenIndex()); + } + + @Test + public void largeNonParsableIntegerStringThrows() { + IndexTrackingTreeMap map = newMap(); + map.put("999999999999999999999999", "huge"); + assertTrue(map.numericalIndices.isEmpty()); + //noinspection SimplifiableAssertion + assertTrue(map.size() == 1); + assertEquals("huge", map.get("999999999999999999999999")); + } + +} From 2b65ef59d45f9152c5f9ae2b815eeaa251b7c55f Mon Sep 17 00:00:00 2001 From: _tud <98935832+UnderscoreTud@users.noreply.github.com> Date: Wed, 1 Apr 2026 20:28:21 +0300 Subject: [PATCH 2/6] Fix variable desync --- src/main/java/ch/njol/skript/lang/Variable.java | 6 ++++-- .../org/skriptlang/skript/util/IndexTrackingTreeMap.java | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/main/java/ch/njol/skript/lang/Variable.java b/src/main/java/ch/njol/skript/lang/Variable.java index 35cc1e242c3..775ee147233 100644 --- a/src/main/java/ch/njol/skript/lang/Variable.java +++ b/src/main/java/ch/njol/skript/lang/Variable.java @@ -596,8 +596,10 @@ public void change(Event event, Object @Nullable [] delta, ChangeMode mode) thro } else { assert mode == ChangeMode.ADD; if (map instanceof IndexTrackingTreeMap indexTrackingMap) { - for (Object value : delta) - indexTrackingMap.add(value); + for (Object value : delta) { + int index = indexTrackingMap.nextOpenIndex(); + setIndex(event, String.valueOf(index), value); + } return; } int i = 1; diff --git a/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java b/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java index 1e5ba65b788..0c59f6a481f 100644 --- a/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java +++ b/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java @@ -1,5 +1,6 @@ package org.skriptlang.skript.util; +import com.google.common.base.Preconditions; import org.jetbrains.annotations.VisibleForTesting; import java.util.*; @@ -31,7 +32,7 @@ public IndexTrackingTreeMap(Comparator comparator) { @Override public V put(String key, V value) { V previous = super.put(key, value); - if (previous == null && isInt(key)) { + if (previous == null && value != null && isInt(key)) { try { int index = Integer.parseInt(key); int pos = Collections.binarySearch(numericalIndices, index); @@ -46,9 +47,10 @@ public V put(String key, V value) { /** * Adds the given value under the first available positive integer key. * - * @param value the value to add + * @param value the value to add, cannot be null */ public void add(V value) { + Preconditions.checkNotNull(value, "value"); int index = nextOpenIndex(); super.put(String.valueOf(index), value); numericalIndices.add(index - 1, index); From 47e32265a26f80675e7566e6d5d8a933eab5ad8d Mon Sep 17 00:00:00 2001 From: _tud <98935832+UnderscoreTud@users.noreply.github.com> Date: Thu, 2 Apr 2026 06:58:14 +0300 Subject: [PATCH 3/6] Optimize deleting and getting the size of a list variable --- .../njol/skript/expressions/ExprAmount.java | 10 ++- .../java/ch/njol/skript/lang/Variable.java | 46 ++++++++----- .../expressions/PropExprAmount.java | 19 ++++-- .../expressions/PropExprNumber.java | 25 ++++--- .../properties/expressions/PropExprSize.java | 19 ++++-- .../skript/util/IndexTrackingTreeMap.java | 66 +++++++++++++++++-- .../skript/util/IndexTrackingTreeMapTest.java | 62 +++++++++++++++++ 7 files changed, 201 insertions(+), 46 deletions(-) diff --git a/src/main/java/ch/njol/skript/expressions/ExprAmount.java b/src/main/java/ch/njol/skript/expressions/ExprAmount.java index 2a8d54c03e5..3290746c61b 100644 --- a/src/main/java/ch/njol/skript/expressions/ExprAmount.java +++ b/src/main/java/ch/njol/skript/expressions/ExprAmount.java @@ -11,6 +11,7 @@ import ch.njol.skript.lang.ExpressionList; import ch.njol.skript.lang.ExpressionType; import ch.njol.skript.lang.SkriptParser.ParseResult; +import ch.njol.skript.lang.Variable; import ch.njol.skript.lang.util.SimpleExpression; import ch.njol.skript.lang.util.common.AnyAmount; import ch.njol.skript.util.LiteralUtils; @@ -42,6 +43,7 @@ public class ExprAmount extends SimpleExpression { @SuppressWarnings("null") private ExpressionList exprs; private @Nullable Expression any; + private @Nullable Variable list; @Override public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) { @@ -54,7 +56,6 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye this.exprs = exprs[0] instanceof ExpressionList exprList ? exprList : new ExpressionList<>(new Expression[]{ exprs[0] }, Object.class, false); - this.exprs = (ExpressionList) LiteralUtils.defendExpression(this.exprs); if (!LiteralUtils.canInitSafely(this.exprs)) { return false; @@ -65,6 +66,9 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye return false; } + if (exprs[0] instanceof Variable variable) + this.list = variable; + return true; } @@ -72,6 +76,10 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye protected Number[] get(Event event) { if (any != null) return new Number[] {any.getOptionalSingle(event).orElse(() -> 0).amount()}; + + if (list != null) + return new Long[]{(long) list.size(event)}; + return new Long[]{(long) exprs.getArray(event).length}; } diff --git a/src/main/java/ch/njol/skript/lang/Variable.java b/src/main/java/ch/njol/skript/lang/Variable.java index 775ee147233..dc4fc40cc43 100644 --- a/src/main/java/ch/njol/skript/lang/Variable.java +++ b/src/main/java/ch/njol/skript/lang/Variable.java @@ -14,6 +14,7 @@ import ch.njol.skript.structures.StructVariables.DefaultVariables; import ch.njol.skript.util.StringMode; import ch.njol.skript.util.Utils; +import com.google.common.base.Preconditions; import org.skriptlang.skript.util.IndexTrackingTreeMap; import ch.njol.skript.variables.Variables; import ch.njol.util.Kleenean; @@ -447,7 +448,7 @@ private T[] getConvertedArray(Event event) { } private void set(Event event, @Nullable Object value) { - Variables.setVariable("" + name.toString(event), value, event, local); + Variables.setVariable(name.toString(event), value, event, local); } private void setIndex(Event event, String index, @Nullable Object value) { @@ -457,6 +458,33 @@ private void setIndex(Event event, String index, @Nullable Object value) { Variables.setVariable(name.substring(0, name.length() - 1) + index, value, event, local); } + public int size(Event event) { + Preconditions.checkState(list, "Cannot get the size of a single variable"); + Map map = (Map) getRaw(event); + if (map == null) + return 0; + + int size = map.size(); + if (map.containsKey(null)) // if we're trying to get the size of {_list::*}, exclude {_list} from being counted + size--; + + if (!(map instanceof IndexTrackingTreeMap indexTrackingMap)) { + for (Object value : map.values()) { + if (value instanceof Map sublist && !sublist.containsKey(null)) + size--; + } + return size; + } + + Collection sublistIndices = indexTrackingMap.mapIndices(); + for (String sublistIndex : sublistIndices) { + if (!((Map) map.get(sublistIndex)).containsKey(null)) + size--; + } + + return size; + } + @Override public Class @Nullable [] acceptChange(ChangeMode mode) { if (!list && mode == ChangeMode.SET) @@ -495,22 +523,6 @@ public void change(Event event, Object @NotNull [] delta, ChangeMode mode, @NotN public void change(Event event, Object @Nullable [] delta, ChangeMode mode) throws UnsupportedOperationException { switch (mode) { case DELETE: - if (list) { - ArrayList toDelete = new ArrayList<>(); - Map map = (Map) getRaw(event); - if (map == null) - return; - for (Entry entry : map.entrySet()) { - if (entry.getKey() != null){ - toDelete.add(entry.getKey()); - } - } - for (String index : toDelete) { - assert index != null; - setIndex(event, index, null); - } - } - set(event, null); break; case SET: diff --git a/src/main/java/org/skriptlang/skript/common/properties/expressions/PropExprAmount.java b/src/main/java/org/skriptlang/skript/common/properties/expressions/PropExprAmount.java index beb93f20f12..7a7567967d6 100644 --- a/src/main/java/org/skriptlang/skript/common/properties/expressions/PropExprAmount.java +++ b/src/main/java/org/skriptlang/skript/common/properties/expressions/PropExprAmount.java @@ -6,6 +6,7 @@ import ch.njol.skript.lang.Expression; import ch.njol.skript.lang.ExpressionList; import ch.njol.skript.lang.SkriptParser.ParseResult; +import ch.njol.skript.lang.Variable; import ch.njol.skript.util.LiteralUtils; import ch.njol.util.Kleenean; import org.bukkit.event.Event; @@ -40,6 +41,7 @@ public static void register(SyntaxRegistry registry, Origin origin) { } private ExpressionList exprs; + private @Nullable Variable list; private boolean useProperties; @Override @@ -48,13 +50,14 @@ public boolean init(Expression[] expressions, int matchedPattern, Kleenean is // amounts of x, y -> property // amount of x, y -> list length useProperties = parseResult.hasTag("s") || expressions[0].isSingle(); - if (useProperties) { + if (useProperties) return super.init(expressions, matchedPattern, isDelayed, parseResult); - } else { - // if exprlist or varlist, count elements - this.exprs = asExprList(expressions[0]); - return LiteralUtils.canInitSafely(this.exprs); - } + + // if exprlist or varlist, count elements + this.exprs = asExprList(expressions[0]); + if (expressions[0] instanceof Variable variable) + this.list = variable; + return LiteralUtils.canInitSafely(this.exprs); } /** @@ -78,6 +81,10 @@ public static ExpressionList asExprList(Expression expr) { protected Object @Nullable [] get(Event event) { if (useProperties) return super.get(event); + + if (list != null) + return new Long[]{(long) list.size(event)}; + return new Long[]{(long) exprs.getArray(event).length}; } diff --git a/src/main/java/org/skriptlang/skript/common/properties/expressions/PropExprNumber.java b/src/main/java/org/skriptlang/skript/common/properties/expressions/PropExprNumber.java index 7802e8f48ae..a36377c2042 100644 --- a/src/main/java/org/skriptlang/skript/common/properties/expressions/PropExprNumber.java +++ b/src/main/java/org/skriptlang/skript/common/properties/expressions/PropExprNumber.java @@ -6,6 +6,7 @@ import ch.njol.skript.lang.Expression; import ch.njol.skript.lang.ExpressionList; import ch.njol.skript.lang.SkriptParser.ParseResult; +import ch.njol.skript.lang.Variable; import ch.njol.skript.util.LiteralUtils; import ch.njol.util.Kleenean; import org.bukkit.event.Event; @@ -37,27 +38,33 @@ public static void register(SyntaxRegistry registry, Origin origin) { } private ExpressionList exprs; + private @Nullable Variable list; private boolean useProperties; @Override public boolean init(Expression[] expressions, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) { - // size[s] of x -> property - // sizes of x, y -> property - // size of x, y -> list length + // number[s] of x -> property + // numbers of x, y -> property + // number of x, y -> list length useProperties = parseResult.hasTag("s") || expressions[0].isSingle(); - if (useProperties) { + if (useProperties) return super.init(expressions, matchedPattern, isDelayed, parseResult); - } else { - // if exprlist or varlist, count elements - this.exprs = PropExprAmount.asExprList(expressions[0]); - return LiteralUtils.canInitSafely(this.exprs); - } + + // if exprlist or varlist, count elements + this.exprs = PropExprAmount.asExprList(expressions[0]); + if (expressions[0] instanceof Variable variable) + this.list = variable; + return LiteralUtils.canInitSafely(this.exprs); } @Override protected Object @Nullable [] get(Event event) { if (useProperties) return super.get(event); + + if (list != null) + return new Long[]{(long) list.size(event)}; + return new Long[]{(long) exprs.getArray(event).length}; } diff --git a/src/main/java/org/skriptlang/skript/common/properties/expressions/PropExprSize.java b/src/main/java/org/skriptlang/skript/common/properties/expressions/PropExprSize.java index 10ccf726f3b..00c1b3cd8bc 100644 --- a/src/main/java/org/skriptlang/skript/common/properties/expressions/PropExprSize.java +++ b/src/main/java/org/skriptlang/skript/common/properties/expressions/PropExprSize.java @@ -6,6 +6,7 @@ import ch.njol.skript.lang.Expression; import ch.njol.skript.lang.ExpressionList; import ch.njol.skript.lang.SkriptParser.ParseResult; +import ch.njol.skript.lang.Variable; import ch.njol.skript.util.LiteralUtils; import ch.njol.util.Kleenean; import org.bukkit.event.Event; @@ -37,6 +38,7 @@ public static void register(SyntaxRegistry registry, Origin origin) { } private ExpressionList exprs; + private @Nullable Variable list; private boolean useProperties; @Override @@ -45,19 +47,24 @@ public boolean init(Expression[] expressions, int matchedPattern, Kleenean is // sizes of x, y -> property // size of x, y -> list length useProperties = parseResult.hasTag("s") || expressions[0].isSingle(); - if (useProperties) { + if (useProperties) return super.init(expressions, matchedPattern, isDelayed, parseResult); - } else { - // if exprlist or varlist, count elements - this.exprs = PropExprAmount.asExprList(expressions[0]); - return LiteralUtils.canInitSafely(this.exprs); - } + + // if exprlist or varlist, count elements + this.exprs = PropExprAmount.asExprList(expressions[0]); + if (expressions[0] instanceof Variable variable) + this.list = variable; + return LiteralUtils.canInitSafely(this.exprs); } @Override protected Object @Nullable [] get(Event event) { if (useProperties) return super.get(event); + + if (list != null) + return new Long[]{(long) list.size(event)}; + return new Long[]{(long) exprs.getArray(event).length}; } diff --git a/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java b/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java index 0c59f6a481f..43e0db4fb35 100644 --- a/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java +++ b/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java @@ -1,6 +1,7 @@ package org.skriptlang.skript.util; import com.google.common.base.Preconditions; +import org.jetbrains.annotations.UnmodifiableView; import org.jetbrains.annotations.VisibleForTesting; import java.util.*; @@ -20,6 +21,7 @@ public class IndexTrackingTreeMap extends TreeMap { @VisibleForTesting final List numericalIndices = new ArrayList<>(); + private final Set mapIndices = new HashSet<>(); public IndexTrackingTreeMap() { super(); @@ -32,15 +34,30 @@ public IndexTrackingTreeMap(Comparator comparator) { @Override public V put(String key, V value) { V previous = super.put(key, value); + + if (previous != null && value == null) { + handleRemove(key, previous); + return previous; + } + if (previous == null && value != null && isInt(key)) { try { int index = Integer.parseInt(key); - int pos = Collections.binarySearch(numericalIndices, index); - numericalIndices.add(-pos - 1, index); + if (consecutive() && index > numericalIndices.size()) { + numericalIndices.add(index); + } else { + int pos = Collections.binarySearch(numericalIndices, index); + numericalIndices.add(-pos - 1, index); + } } catch (NumberFormatException ignore) {} - } else if (previous != null && value == null) { - handleRemove(key); } + + if (value instanceof Map) { + mapIndices.add(key); + } else if (previous instanceof Map) { + mapIndices.remove(key); + } + return previous; } @@ -52,15 +69,18 @@ public V put(String key, V value) { public void add(V value) { Preconditions.checkNotNull(value, "value"); int index = nextOpenIndex(); - super.put(String.valueOf(index), value); + String key = String.valueOf(index); + super.put(key, value); numericalIndices.add(index - 1, index); + if (value instanceof Map) + mapIndices.add(key); } @Override public V remove(Object key) { V value = super.remove(key); if (value != null && key instanceof String index) - handleRemove(index); + handleRemove(index, value); return value; } @@ -68,6 +88,14 @@ public V remove(Object key) { public void clear() { super.clear(); numericalIndices.clear(); + mapIndices.clear(); + } + + public boolean consecutive() { + int size = numericalIndices.size(); + if (size == 0) + return true; + return numericalIndices.getLast() == size; } /** @@ -87,6 +115,9 @@ public int nextOpenIndex() { if (numericalIndices.getFirst() > 1) return 1; + if (consecutive()) + return numericalIndices.size() + 1; + int left = 0; int right = size - 1; @@ -103,7 +134,28 @@ public int nextOpenIndex() { return left + 1; } - private void handleRemove(String index) { + /** + * Returns an unmodifiable view of the tracked numerical indices. + * + * @return a collection of all tracked positive integer keys + */ + public @UnmodifiableView Collection numericalIndices() { + return Collections.unmodifiableCollection(numericalIndices); + } + + /** + * Returns an unmodifiable view of the keys that map to other {@link Map} instances. + * + * @return a collection of all keys pointing to a map + */ + public @UnmodifiableView Collection mapIndices() { + return Collections.unmodifiableCollection(mapIndices); + } + + private void handleRemove(String index, Object previous) { + if (previous instanceof Map) + mapIndices.remove(index); + if (!isInt(index)) return; int pos = Collections.binarySearch(numericalIndices, Integer.parseInt(index)); diff --git a/src/test/java/org/skriptlang/skript/util/IndexTrackingTreeMapTest.java b/src/test/java/org/skriptlang/skript/util/IndexTrackingTreeMapTest.java index 0036c560108..1811de9fd67 100644 --- a/src/test/java/org/skriptlang/skript/util/IndexTrackingTreeMapTest.java +++ b/src/test/java/org/skriptlang/skript/util/IndexTrackingTreeMapTest.java @@ -2,6 +2,7 @@ import org.junit.Test; +import java.util.Collection; import java.util.Map; import static org.junit.Assert.*; @@ -219,6 +220,67 @@ public void alphaNumericKeyIsIgnored() { assertEquals(1, map.nextOpenIndex()); } + @Test + public void numericalIndicesViewIsCorrect() { + IndexTrackingTreeMap map = newMap(); + map.put("1", "one"); + map.put("3", "three"); + map.put("foo", "foo"); + + Collection indices = map.numericalIndices(); + assertEquals(2, indices.size()); + assertTrue(indices.contains(1)); + assertTrue(indices.contains(3)); + + assertThrows(UnsupportedOperationException.class, () -> indices.add(2)); + } + + @Test + public void mapIndicesViewIsCorrect() { + IndexTrackingTreeMap map = new IndexTrackingTreeMap<>(); + map.put("1", "one"); + Map subMap = Map.of("a", "b"); + map.put("sub", subMap); + + Collection indices = map.mapIndices(); + assertEquals(1, indices.size()); + assertTrue(indices.contains("sub")); + + assertThrows(UnsupportedOperationException.class, () -> indices.add("other")); + } + + @Test + public void putNullValueRemovesMapping() { + IndexTrackingTreeMap map = newMap(); + map.put("1", "one"); + map.put("1", null); + + assertNull(map.get("1")); + assertEquals(1, map.nextOpenIndex()); + assertTrue(map.numericalIndices().isEmpty()); + } + + @Test + public void addMapValueUpdatesMapIndices() { + IndexTrackingTreeMap map = new IndexTrackingTreeMap<>(); + Map subMap = Map.of("a", "b"); + map.add(subMap); + + assertEquals(subMap, map.get("1")); + assertTrue(map.mapIndices().contains("1")); + } + + @Test + public void removeMapValueUpdatesMapIndices() { + IndexTrackingTreeMap map = new IndexTrackingTreeMap<>(); + Map subMap = Map.of("a", "b"); + map.put("sub", subMap); + assertTrue(map.mapIndices().contains("sub")); + + map.remove("sub"); + assertFalse(map.mapIndices().contains("sub")); + } + @Test public void clearResetsTracking() { IndexTrackingTreeMap map = newMap(); From 40065744e7f4c2b9ed7d39fca3f08c70564d7e79 Mon Sep 17 00:00:00 2001 From: _tud <98935832+UnderscoreTud@users.noreply.github.com> Date: Fri, 3 Apr 2026 14:34:54 +0300 Subject: [PATCH 4/6] Implement a different approach to find an open index --- .../skript/util/IndexTrackingTreeMap.java | 135 ++++++++---------- .../skript/util/IndexTrackingTreeMapTest.java | 21 +-- 2 files changed, 59 insertions(+), 97 deletions(-) diff --git a/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java b/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java index 43e0db4fb35..a32894ecedf 100644 --- a/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java +++ b/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java @@ -2,7 +2,6 @@ import com.google.common.base.Preconditions; import org.jetbrains.annotations.UnmodifiableView; -import org.jetbrains.annotations.VisibleForTesting; import java.util.*; @@ -19,8 +18,7 @@ */ public class IndexTrackingTreeMap extends TreeMap { - @VisibleForTesting - final List numericalIndices = new ArrayList<>(); + private final NavigableSet freeIndices = new TreeSet<>(); private final Set mapIndices = new HashSet<>(); public IndexTrackingTreeMap() { @@ -35,27 +33,12 @@ public IndexTrackingTreeMap(Comparator comparator) { public V put(String key, V value) { V previous = super.put(key, value); - if (previous != null && value == null) { + if (previous == null && value != null) { + handleInsert(key, value); + } else if (previous != null && value == null) { handleRemove(key, previous); - return previous; - } - - if (previous == null && value != null && isInt(key)) { - try { - int index = Integer.parseInt(key); - if (consecutive() && index > numericalIndices.size()) { - numericalIndices.add(index); - } else { - int pos = Collections.binarySearch(numericalIndices, index); - numericalIndices.add(-pos - 1, index); - } - } catch (NumberFormatException ignore) {} - } - - if (value instanceof Map) { - mapIndices.add(key); - } else if (previous instanceof Map) { - mapIndices.remove(key); + } else if (previous != null) { + handleReplace(key, previous, value); } return previous; @@ -68,10 +51,14 @@ public V put(String key, V value) { */ public void add(V value) { Preconditions.checkNotNull(value, "value"); - int index = nextOpenIndex(); + int index = freeIndices.removeFirst(); String key = String.valueOf(index); + super.put(key, value); - numericalIndices.add(index - 1, index); + + if (freeIndices.isEmpty()) + freeIndices.add(index + 1); + if (value instanceof Map) mapIndices.add(key); } @@ -87,17 +74,11 @@ public V remove(Object key) { @Override public void clear() { super.clear(); - numericalIndices.clear(); + freeIndices.clear(); + freeIndices.add(1); mapIndices.clear(); } - public boolean consecutive() { - int size = numericalIndices.size(); - if (size == 0) - return true; - return numericalIndices.getLast() == size; - } - /** * Finds the first available positive integer index that is not currently * used as a key in this map. @@ -108,39 +89,7 @@ public boolean consecutive() { * @return the next available positive integer index */ public int nextOpenIndex() { - int size = numericalIndices.size(); - if (size == 0) - return 1; - - if (numericalIndices.getFirst() > 1) - return 1; - - if (consecutive()) - return numericalIndices.size() + 1; - - int left = 0; - int right = size - 1; - - while (left <= right) { - int midpoint = left + (right - left >>> 1); - - if (numericalIndices.get(midpoint) == midpoint + 1) { - left = midpoint + 1; - } else { - right = midpoint - 1; - } - } - - return left + 1; - } - - /** - * Returns an unmodifiable view of the tracked numerical indices. - * - * @return a collection of all tracked positive integer keys - */ - public @UnmodifiableView Collection numericalIndices() { - return Collections.unmodifiableCollection(numericalIndices); + return freeIndices.first(); } /** @@ -152,26 +101,56 @@ public int nextOpenIndex() { return Collections.unmodifiableCollection(mapIndices); } - private void handleRemove(String index, Object previous) { + public void handleInsert(String key, V value) { + if (value instanceof Map) + mapIndices.add(key); + + int index = parsePositiveInt(key); + if (index < 0) + return; + + freeIndices.remove(index); + + if (!containsKey(String.valueOf(index + 1))) + freeIndices.add(index + 1); + } + + public void handleReplace(String key, V previous, V value) { + if (value instanceof Map) { + mapIndices.add(key); + } else if (previous instanceof Map) { + mapIndices.remove(key); + } + } + + private void handleRemove(String key, V previous) { if (previous instanceof Map) - mapIndices.remove(index); + mapIndices.remove(key); - if (!isInt(index)) + int index = parsePositiveInt(key); + if (index < 0) return; - int pos = Collections.binarySearch(numericalIndices, Integer.parseInt(index)); - if (pos >= 0) - numericalIndices.remove(pos); + + freeIndices.add(Integer.parseInt(key)); } - private boolean isInt(String string) { + private int parsePositiveInt(String string) { if (string == null || string.isBlank() || string.charAt(0) == '0') // Don't handle leading-zero integers - return false; - for (int i = 0; i < string.length(); i++) { - if (!isDigit(string.charAt(i))) - return false; + return -1; + + int value = 0; + try { + for (int i = 0; i < string.length(); i++) { + char c = string.charAt(i); + if (!isDigit(c)) + return -1; + value = Math.addExact(value * 10, c - '0'); + } + } catch (ArithmeticException e) { // overflow + return -1; } - return true; + return value; } private boolean isDigit(int codepoint) { diff --git a/src/test/java/org/skriptlang/skript/util/IndexTrackingTreeMapTest.java b/src/test/java/org/skriptlang/skript/util/IndexTrackingTreeMapTest.java index 1811de9fd67..488ed049a00 100644 --- a/src/test/java/org/skriptlang/skript/util/IndexTrackingTreeMapTest.java +++ b/src/test/java/org/skriptlang/skript/util/IndexTrackingTreeMapTest.java @@ -220,21 +220,6 @@ public void alphaNumericKeyIsIgnored() { assertEquals(1, map.nextOpenIndex()); } - @Test - public void numericalIndicesViewIsCorrect() { - IndexTrackingTreeMap map = newMap(); - map.put("1", "one"); - map.put("3", "three"); - map.put("foo", "foo"); - - Collection indices = map.numericalIndices(); - assertEquals(2, indices.size()); - assertTrue(indices.contains(1)); - assertTrue(indices.contains(3)); - - assertThrows(UnsupportedOperationException.class, () -> indices.add(2)); - } - @Test public void mapIndicesViewIsCorrect() { IndexTrackingTreeMap map = new IndexTrackingTreeMap<>(); @@ -257,7 +242,6 @@ public void putNullValueRemovesMapping() { assertNull(map.get("1")); assertEquals(1, map.nextOpenIndex()); - assertTrue(map.numericalIndices().isEmpty()); } @Test @@ -311,9 +295,8 @@ public void putAllKeepsTrackingCorrect() { public void largeNonParsableIntegerStringThrows() { IndexTrackingTreeMap map = newMap(); map.put("999999999999999999999999", "huge"); - assertTrue(map.numericalIndices.isEmpty()); - //noinspection SimplifiableAssertion - assertTrue(map.size() == 1); + assertEquals(1, map.size()); + assertEquals(1, map.nextOpenIndex()); assertEquals("huge", map.get("999999999999999999999999")); } From 26e5e17e224fac4b41409f19e57d30d031d000e8 Mon Sep 17 00:00:00 2001 From: _tud <98935832+UnderscoreTud@users.noreply.github.com> Date: Fri, 3 Apr 2026 19:41:47 +0300 Subject: [PATCH 5/6] Implement a different approach to find an open index (keep track of one open index) --- .../skript/util/IndexTrackingTreeMap.java | 50 ++++++++++++++----- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java b/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java index a32894ecedf..f430b57a383 100644 --- a/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java +++ b/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java @@ -18,9 +18,12 @@ */ public class IndexTrackingTreeMap extends TreeMap { - private final NavigableSet freeIndices = new TreeSet<>(); private final Set mapIndices = new HashSet<>(); + private final Set numericalIndices = new HashSet<>(); + private int nextIndex = 1; + private int maxIndex = -1; + public IndexTrackingTreeMap() { super(); } @@ -51,13 +54,13 @@ public V put(String key, V value) { */ public void add(V value) { Preconditions.checkNotNull(value, "value"); - int index = freeIndices.removeFirst(); - String key = String.valueOf(index); + String key = String.valueOf(nextIndex); super.put(key, value); + numericalIndices.add(nextIndex); - if (freeIndices.isEmpty()) - freeIndices.add(index + 1); + maxIndex = Math.max(nextIndex, maxIndex); + advanceNextIndex(); if (value instanceof Map) mapIndices.add(key); @@ -74,9 +77,10 @@ public V remove(Object key) { @Override public void clear() { super.clear(); - freeIndices.clear(); - freeIndices.add(1); + numericalIndices.clear(); mapIndices.clear(); + nextIndex = 1; + maxIndex = -1; } /** @@ -89,7 +93,11 @@ public void clear() { * @return the next available positive integer index */ public int nextOpenIndex() { - return freeIndices.first(); + return nextIndex; + } + + public boolean consecutive() { + return nextIndex == maxIndex + 1; } /** @@ -109,10 +117,10 @@ public void handleInsert(String key, V value) { if (index < 0) return; - freeIndices.remove(index); + numericalIndices.add(index); - if (!containsKey(String.valueOf(index + 1))) - freeIndices.add(index + 1); + maxIndex = Math.max(maxIndex, index); + advanceNextIndex(); } public void handleReplace(String key, V previous, V value) { @@ -131,7 +139,25 @@ private void handleRemove(String key, V previous) { if (index < 0) return; - freeIndices.add(Integer.parseInt(key)); + numericalIndices.remove(index); + + if (index == maxIndex) + recomputeMaxIndex(); + nextIndex = Math.min(nextIndex, index); + } + + private void advanceNextIndex() { + if (nextIndex == maxIndex) { + nextIndex++; + return; + } + while (numericalIndices.contains(nextIndex)) + nextIndex++; + } + + private void recomputeMaxIndex() { + while (maxIndex >= 0 && !numericalIndices.contains(maxIndex)) + maxIndex--; } private int parsePositiveInt(String string) { From d94648112aeb5c20015774082eddfdd5e77e7478 Mon Sep 17 00:00:00 2001 From: _tud <98935832+UnderscoreTud@users.noreply.github.com> Date: Fri, 3 Apr 2026 20:03:35 +0300 Subject: [PATCH 6/6] Make `#add` use `#handleInsert` --- .../skript/util/IndexTrackingTreeMap.java | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java b/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java index f430b57a383..f480d580a95 100644 --- a/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java +++ b/src/main/java/org/skriptlang/skript/util/IndexTrackingTreeMap.java @@ -37,7 +37,7 @@ public V put(String key, V value) { V previous = super.put(key, value); if (previous == null && value != null) { - handleInsert(key, value); + handleInsert(key, parsePositiveInt(key), value); } else if (previous != null && value == null) { handleRemove(key, previous); } else if (previous != null) { @@ -57,13 +57,7 @@ public void add(V value) { String key = String.valueOf(nextIndex); super.put(key, value); - numericalIndices.add(nextIndex); - - maxIndex = Math.max(nextIndex, maxIndex); - advanceNextIndex(); - - if (value instanceof Map) - mapIndices.add(key); + handleInsert(key, nextIndex, value); } @Override @@ -109,11 +103,10 @@ public boolean consecutive() { return Collections.unmodifiableCollection(mapIndices); } - public void handleInsert(String key, V value) { + private void handleInsert(String key, int index, V value) { if (value instanceof Map) mapIndices.add(key); - int index = parsePositiveInt(key); if (index < 0) return; @@ -123,7 +116,7 @@ public void handleInsert(String key, V value) { advanceNextIndex(); } - public void handleReplace(String key, V previous, V value) { + private void handleReplace(String key, V previous, V value) { if (value instanceof Map) { mapIndices.add(key); } else if (previous instanceof Map) {