From adf289dcef6658b2bff6f29cd8980f967fb5cfe2 Mon Sep 17 00:00:00 2001 From: Moderocky Date: Wed, 18 Dec 2024 07:58:09 +0100 Subject: [PATCH] Keyed Set Mode (#7072) * Add key providers/receivers. * Add changers & list variable support. * Clean up default method. * Add basic tests. * Changes. * Fix merge gunk. --- .../java/ch/njol/skript/classes/Changer.java | 8 +- .../ch/njol/skript/effects/EffChange.java | 61 +++++---- .../skript/lang/KeyProviderExpression.java | 128 ++++++++++++++++++ .../skript/lang/KeyReceiverExpression.java | 31 +++++ .../java/ch/njol/skript/lang/Variable.java | 59 ++++++-- .../skript/test/runner/ExprKeyValueSet.java | 68 ++++++++++ src/test/skript/tests/misc/keyed set mode.sk | 29 ++++ 7 files changed, 346 insertions(+), 38 deletions(-) create mode 100644 src/main/java/ch/njol/skript/lang/KeyProviderExpression.java create mode 100644 src/main/java/ch/njol/skript/lang/KeyReceiverExpression.java create mode 100644 src/main/java/ch/njol/skript/test/runner/ExprKeyValueSet.java create mode 100644 src/test/skript/tests/misc/keyed set mode.sk diff --git a/src/main/java/ch/njol/skript/classes/Changer.java b/src/main/java/ch/njol/skript/classes/Changer.java index 31cd2fc7442..138f6279740 100644 --- a/src/main/java/ch/njol/skript/classes/Changer.java +++ b/src/main/java/ch/njol/skript/classes/Changer.java @@ -37,8 +37,14 @@ public interface Changer { enum ChangeMode { ADD, SET, REMOVE, REMOVE_ALL, DELETE, RESET; + + public boolean supportsKeyedChange() { + return this == SET; + // ADD could be supported in future + } + } - + /** * Tests whether this changer supports the given mode, and if yes what type(s) it expects the elements of delta to be. *

diff --git a/src/main/java/ch/njol/skript/effects/EffChange.java b/src/main/java/ch/njol/skript/effects/EffChange.java index 238b494d656..ee2e2e012be 100644 --- a/src/main/java/ch/njol/skript/effects/EffChange.java +++ b/src/main/java/ch/njol/skript/effects/EffChange.java @@ -22,11 +22,7 @@ import java.util.logging.Level; import ch.njol.skript.expressions.ExprParse; -import ch.njol.skript.lang.Effect; -import ch.njol.skript.lang.Expression; -import ch.njol.skript.lang.ExpressionList; -import ch.njol.skript.lang.SkriptParser; -import ch.njol.skript.lang.Variable; +import ch.njol.skript.lang.*; import org.skriptlang.skript.lang.script.ScriptWarning; import org.bukkit.event.Event; import org.jetbrains.annotations.Nullable; @@ -82,40 +78,40 @@ public class EffChange extends Effect { {"(add|give) %objects% to %~objects%", ChangeMode.ADD}, {"increase %~objects% by %objects%", ChangeMode.ADD}, {"give %~objects% %objects%", ChangeMode.ADD}, - + {"set %~objects% to %objects%", ChangeMode.SET}, - + {"remove (all|every) %objects% from %~objects%", ChangeMode.REMOVE_ALL}, - + {"(remove|subtract) %objects% from %~objects%", ChangeMode.REMOVE}, {"(reduce|decrease) %~objects% by %objects%", ChangeMode.REMOVE}, - + {"(delete|clear) %~objects%", ChangeMode.DELETE}, - + {"reset %~objects%", ChangeMode.RESET} }); - + static { Skript.registerEffect(EffChange.class, patterns.getPatterns()); } - + @SuppressWarnings("null") private Expression changed; @Nullable private Expression changer = null; - + @SuppressWarnings("null") private ChangeMode mode; - + private boolean single; - + // private Changer c = null; - + @SuppressWarnings({"unchecked", "null"}) @Override public boolean init(final Expression[] exprs, final int matchedPattern, final Kleenean isDelayed, final ParseResult parser) { mode = patterns.getInfo(matchedPattern); - + switch (mode) { case ADD: if (matchedPattern == 0) { @@ -149,7 +145,7 @@ public boolean init(final Expression[] exprs, final int matchedPattern, final case RESET: changed = exprs[0]; } - + CountingLogHandler h = new CountingLogHandler(Level.SEVERE).start(); Class[] rs; String what; @@ -192,12 +188,12 @@ public boolean init(final Expression[] exprs, final int matchedPattern, final } return false; } - + final Class[] rs2 = new Class[rs.length]; for (int i = 0; i < rs.length; i++) rs2[i] = rs[i].isArray() ? rs[i].getComponentType() : rs[i]; final boolean allSingle = Arrays.equals(rs, rs2); - + Expression ch = changer; if (ch != null) { Expression v = null; @@ -235,7 +231,7 @@ else if (mode == ChangeMode.SET) } finally { log.stop(); } - + Class x = Utils.getSuperType(rs2); single = allSingle; for (int i = 0; i < rs.length; i++) { @@ -247,7 +243,7 @@ else if (mode == ChangeMode.SET) } assert x != null; changer = ch = v; - + if (!ch.canBeSingle() && single) { if (mode == ChangeMode.SET) Skript.error(changed + " can only be set to one " + Classes.getSuperClassInfo(x).getName() + ", not more", ErrorQuality.SEMANTIC_ERROR); @@ -278,20 +274,27 @@ else if (mode == ChangeMode.SET) } return true; } - + @Override - protected void execute(Event e) { - Object[] delta = changer == null ? null : changer.getArray(e); + protected void execute(Event event) { + Object[] delta = changer == null ? null : changer.getArray(event); delta = changer == null ? delta : changer.beforeChange(changed, delta); if ((delta == null || delta.length == 0) && (mode != ChangeMode.DELETE && mode != ChangeMode.RESET)) { if (mode == ChangeMode.SET && changed.acceptChange(ChangeMode.DELETE) != null) - changed.change(e, null, ChangeMode.DELETE); + changed.change(event, null, ChangeMode.DELETE); return; } - changed.change(e, delta, mode); + if (mode.supportsKeyedChange() && changer != null && delta != null + && changer instanceof KeyProviderExpression provider + && changed instanceof KeyReceiverExpression receiver + && provider.areKeysRecommended()) { + receiver.change(event, delta, mode, provider.getArrayKeys(event)); + } else { + changed.change(event, delta, mode); + } } - + @Override public String toString(final @Nullable Event e, final boolean debug) { final Expression changer = this.changer; @@ -316,5 +319,5 @@ public String toString(final @Nullable Event e, final boolean debug) { assert false; return ""; } - + } diff --git a/src/main/java/ch/njol/skript/lang/KeyProviderExpression.java b/src/main/java/ch/njol/skript/lang/KeyProviderExpression.java new file mode 100644 index 00000000000..cd053a3dd8c --- /dev/null +++ b/src/main/java/ch/njol/skript/lang/KeyProviderExpression.java @@ -0,0 +1,128 @@ +package ch.njol.skript.lang; + +import ch.njol.skript.classes.Changer; +import ch.njol.skript.classes.Changer.ChangeMode; +import org.bukkit.event.Event; +import org.jetbrains.annotations.NotNull; + +/** + * Represents an expression that is able to return a set of keys linked to its values. + * This can be used to return index-linked values to store in a list variable, + * using the {@link ChangeMode#SET} {@link Changer}. + * An expression can provide a set of keys to use, rather than numerical indices. + *
+ * Index-linking is not (currently) used with other change modes. + *
+ *
+ *

Contract

+ * + *
+ *

Advice on Caching

+ * As long as callers are built sensibly and follow API advice, it should be safe to cache a key-list during a values + * call. + * E.g. if an expression is returning data from a map, it could request the whole entry-set during + * {@link #getArray(Event)} + * and return the keys during {@link #getArrayKeys(Event)} (provided the cache is weak, safe and event-linked). + * This is not necessary, but it may be helpful for some expressions where the set of keys could potentially change + * between repeated calls, or is expensive to access. + *
+ *
+ *

Caveats

+ *
    + *
  1. The caller may never ask for {@link #getArrayKeys(Event)}. + * The cache should be disposed of in a timely manner.
    2. + *
  2. It is (theoretically) possible for two separate calls to occur simultaneously + * (asking for the value/key sets separately) so it is recommended to link any cache system to the event instance + * .
    3. + *
+ * Note that the caller may never ask for {@link #getArrayKeys(Event)} and so the cache should be disposed of + * in a timely manner. + *
+ * 
{@code
+ * Map> cache = new WeakHashMap<>();
+ *
+ * public Object[] getArray(Event event) {
+ *     Set> entries = something.entrySet();
+ *     cache.put(event, List.copyOf(something.keySet()));
+ *     return something.values().toArray(...);
+ * }
+ *
+ * public String[] getArrayKeys(Event event) {
+ *     if (!cache.containsKey(event))
+ *         throw new IllegalStateException();
+ *     return cache.remove(event).toArray(new String[0]);
+ *     // this should never be absent/null
+ * }
+ * }
+ * + * @see Expression + * @see KeyReceiverExpression + */ +public interface KeyProviderExpression extends Expression { + + /** + * A set of keys, matching the length and order of the immediately-previous + * {@link #getArray(Event)} values array. + *
+ * This should only be called immediately after a {@link #getArray(Event)} invocation. + * If it is called without a matching values request (or after a delay) then the behaviour + * is undefined, in which case: + *
    + *
  • the method may throw an error,
    • + *
  • the method may return keys not matching any previous values,
    • + *
  • or the method may return nothing at all.
    • + *
+ * + * @param event The event context + * @return A set of keys, of the same length as {@link #getArray(Event)} + * @throws IllegalStateException If this was not called directly after a {@link #getArray(Event)} call + */ + @NotNull String @NotNull [] getArrayKeys(Event event) throws IllegalStateException; + + /** + * A set of keys, matching the length and order of the immediately-previous + * {@link #getAll(Event)} values array. + *
+ * This should only be called immediately after a {@link #getAll(Event)} invocation. + * If it is called without a matching values request (or after a delay) then the behaviour + * is undefined, in which case: + *
    + *
  • the method may throw an error,
    • + *
  • the method may return keys not matching any previous values,
    • + *
  • or the method may return nothing at all.
    • + *
+ * + * @param event The event context + * @return A set of keys, of the same length as {@link #getAll(Event)} + * @throws IllegalStateException If this was not called directly after a {@link #getAll(Event)} call + */ + default @NotNull String @NotNull [] getAllKeys(Event event) { + return this.getArrayKeys(event); + } + + /** + * Keyed expressions should never be single. + */ + @Override + default boolean isSingle() { + return false; + } + + /** + * While all keyed expressions offer their keys, + * some may prefer that they are not used unless strictly required (e.g. variables). + * + * @return Whether the caller is recommended to ask for keys after asking for values + */ + default boolean areKeysRecommended() { + return true; + } + +} diff --git a/src/main/java/ch/njol/skript/lang/KeyReceiverExpression.java b/src/main/java/ch/njol/skript/lang/KeyReceiverExpression.java new file mode 100644 index 00000000000..e954c2ac5e0 --- /dev/null +++ b/src/main/java/ch/njol/skript/lang/KeyReceiverExpression.java @@ -0,0 +1,31 @@ +package ch.njol.skript.lang; + +import ch.njol.skript.classes.Changer; +import ch.njol.skript.classes.Changer.ChangeMode; +import org.bukkit.event.Event; +import org.jetbrains.annotations.NotNull; + +/** + * Represents an expression that is able to accept a set of keys linked to values + * during the {@link ChangeMode#SET} {@link Changer}. + * + * @see Expression + * @see KeyReceiverExpression + */ +public interface KeyReceiverExpression extends Expression { + + /** + * An alternative changer method that provides a set of keys as well as a set of values. + * This is only ever called for {@link ChangeMode#supportsKeyedChange()} safe change modes, + * where a set of values is provided. + * (This will never be called for valueless {@link ChangeMode#DELETE} or {@link ChangeMode#RESET} changers, + * for example.) + * + * @param event The current event context + * @param delta The change values + * @param mode The key-safe change mode {@link ChangeMode#SET} + * @param keys The keys, matching the length and order of the values array + */ + void change(Event event, Object @NotNull [] delta, ChangeMode mode, @NotNull String @NotNull [] keys); + +} diff --git a/src/main/java/ch/njol/skript/lang/Variable.java b/src/main/java/ch/njol/skript/lang/Variable.java index 423286455ff..4a9efed7684 100644 --- a/src/main/java/ch/njol/skript/lang/Variable.java +++ b/src/main/java/ch/njol/skript/lang/Variable.java @@ -45,6 +45,7 @@ import org.bukkit.World; import org.bukkit.entity.Player; import org.bukkit.event.Event; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.skriptlang.skript.lang.arithmetic.Arithmetics; import org.skriptlang.skript.lang.arithmetic.OperationInfo; @@ -56,17 +57,11 @@ import org.skriptlang.skript.lang.script.ScriptWarning; import java.lang.reflect.Array; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Iterator; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.NoSuchElementException; -import java.util.TreeMap; import java.util.function.Function; -public class Variable implements Expression { +public class Variable implements Expression, KeyReceiverExpression, KeyProviderExpression { private final static String SINGLE_SEPARATOR_CHAR = ":"; public final static String SEPARATOR = SINGLE_SEPARATOR_CHAR + SINGLE_SEPARATOR_CHAR; @@ -457,6 +452,32 @@ private void setIndex(Event event, String index, @Nullable Object value) { return CollectionUtils.array(Object[].class); } + @Override + public void change(Event event, Object @NotNull [] delta, ChangeMode mode, @NotNull String @NotNull [] keys) { + if (!list) { + this.change(event, delta, mode); + return; + } + if (mode == ChangeMode.SET) { + assert delta.length == keys.length; + this.set(event, null); + int length = Math.min(delta.length, keys.length); + for (int index = 0; index < length; index++) { + Object value = delta[index]; + String key = keys[index]; + if (value instanceof Object[] array) { + for (int j = 0; j < array.length; j++) + this.setIndex(event, key + SEPARATOR + (j + 1), array[j]); + } else { + this.setIndex(event, key, value); + } + } + return; + } + // no other modes are supported right now + this.change(event, delta, mode); + } + @Override @SuppressWarnings({"unchecked", "rawtypes"}) public void change(Event event, Object @Nullable [] delta, ChangeMode mode) throws UnsupportedOperationException { @@ -650,6 +671,28 @@ public void changeInPlace(Event event, Function changeFunction) { return getConverted(event); } + @Override + public @NotNull String @NotNull [] getArrayKeys(Event event) throws SkriptAPIException { + if (!list) + throw new SkriptAPIException("Invalid call to getArrayKeys on non-list"); + String name = StringUtils.substring(this.name.toString(event), 0, -1); + Object value = Variables.getVariable(name + "*", event, local); + if (value == null) + return new String[0]; + assert value instanceof Map; + return ((Map) value).keySet().toArray(new String[0]); + } + + @Override + public @NotNull String @NotNull [] getAllKeys(Event event) { + return this.getArrayKeys(event); + } + + @Override + public boolean areKeysRecommended() { + return false; // We want `set {list::*} to {other::*}` reset numbering! + } + @Override public T[] getArray(Event event) { return getAll(event); diff --git a/src/main/java/ch/njol/skript/test/runner/ExprKeyValueSet.java b/src/main/java/ch/njol/skript/test/runner/ExprKeyValueSet.java new file mode 100644 index 00000000000..31e77dbef62 --- /dev/null +++ b/src/main/java/ch/njol/skript/test/runner/ExprKeyValueSet.java @@ -0,0 +1,68 @@ +package ch.njol.skript.test.runner; + +import ch.njol.skript.Skript; +import ch.njol.skript.doc.NoDoc; +import ch.njol.skript.lang.*; +import ch.njol.skript.lang.util.SimpleExpression; +import ch.njol.util.Kleenean; +import org.bukkit.event.Event; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Arrays; +import java.util.Map; + +@NoDoc +public class ExprKeyValueSet extends SimpleExpression implements KeyProviderExpression { + + static { + if (TestMode.ENABLED) + Skript.registerExpression(ExprKeyValueSet.class, Object.class, ExpressionType.SIMPLE, + "test key values of %~objects%", + "test key values" + ); + } + + private static final Map testSet = Map.of("hello", "there", "foo", "bar", "a", "b"); + + private Variable variable; + @Override + public boolean init(Expression[] expressions, int matchedPattern, Kleenean isDelayed, SkriptParser.ParseResult parseResult) { + if (matchedPattern == 0) { + Expression expression = expressions[0]; + if (!(expression instanceof Variable variable) || !variable.isList()) { + Skript.error("The expression '" + expression + "' is not a list variable."); + return false; + } + this.variable = variable; + } + return true; + } + + @Override + public @NotNull String @NotNull [] getArrayKeys(Event event) throws IllegalStateException { + if (variable == null) + return testSet.keySet().toArray(new String[0]); + return variable.getArrayKeys(event); + } + + @Override + protected Object @Nullable [] get(Event event) { + if (variable == null) + return testSet.values().toArray(); + return variable.getArray(event); + } + + @Override + public Class getReturnType() { + return Object.class; + } + + @Override + public String toString(@Nullable Event event, boolean debug) { + if (variable != null) + return "test key values of " + variable.toString(event, debug); + return "test key values"; + } + +} diff --git a/src/test/skript/tests/misc/keyed set mode.sk b/src/test/skript/tests/misc/keyed set mode.sk new file mode 100644 index 00000000000..513f897fedd --- /dev/null +++ b/src/test/skript/tests/misc/keyed set mode.sk @@ -0,0 +1,29 @@ +test "keyed set mode": + # we have a `test key values [of %list%]` syntax for testing key preservation + + set {_list::*} to test key values + + # expecting hello -> there, foo -> bar, a -> b + assert size of {_list::*} is 3 with "something wasn't set properly" + assert {_list::hello} is "there" with "key/value set not matched: %indices of {_list::*}% -> %{_list::*}%" + assert {_list::foo} is "bar" with "key/value set not matched: %indices of {_list::*}% -> %{_list::*}%" + assert {_list::a} is "b" with "key/value set not matched: %indices of {_list::*}% -> %{_list::*}%" + + # expecting reversion to numeric indices + set {_list 2::*} to {_list::*} + assert size of {_list 2::*} is 3 with "something wasn't carried properly" + assert {_list 2::hello} is not "there" with "keys were preserved in list -> list set" + assert {_list 2::foo} is not "bar" with "keys were preserved in list -> list set" + assert {_list 2::a} is not "b" with "keys were preserved in list -> list set" + + delete {_list::*} + delete {_list 2::*} + + set {_list::alice} to "bob" + set {_list::hello} to "world" + set {_list 2::*} to test key values of {_list::*} + + # expecting preservation of keys + assert size of {_list 2::*} is 2 with "something wasn't carried properly" + assert {_list 2::alice} is "bob" with "keys were not preserved, %indices of {_list 2::*}% -> %{_list 2::*}%" + assert {_list 2::hello} is "world" with "keys were not preserved, %indices of {_list 2::*}% -> %{_list 2::*}%"