From 29c6764f66d884183d90760bd3dbd9c2643a9030 Mon Sep 17 00:00:00 2001 From: sovdee <10354869+sovdeeth@users.noreply.github.com> Date: Sun, 1 Sep 2024 18:04:34 -0400 Subject: [PATCH] Add SecFilter, refactor Variable and Variables slightly to accomodate. (#6912) * Add SecFilter, refactor Variable and Variables slightly to accomodate. * Messed up merge * realllly bad at merging * generify convertIfOldPlayer * use SectionNode#isEmpty() * small edits and implement variableIterator#remove() * Update src/main/java/ch/njol/skript/sections/SecFilter.java Co-authored-by: Patrick Miller --------- Co-authored-by: Patrick Miller --- .../njol/skript/expressions/ExprFilter.java | 2 +- .../java/ch/njol/skript/lang/Variable.java | 58 +----- .../ch/njol/skript/sections/SecFilter.java | 192 ++++++++++++++++++ .../ch/njol/skript/variables/Variables.java | 71 ++++++- .../tests/syntaxes/sections/SecFilter.sk | 35 ++++ 5 files changed, 305 insertions(+), 53 deletions(-) create mode 100644 src/main/java/ch/njol/skript/sections/SecFilter.java create mode 100644 src/test/skript/tests/syntaxes/sections/SecFilter.sk diff --git a/src/main/java/ch/njol/skript/expressions/ExprFilter.java b/src/main/java/ch/njol/skript/expressions/ExprFilter.java index a9747bbc3fc..738fb1c9ea9 100644 --- a/src/main/java/ch/njol/skript/expressions/ExprFilter.java +++ b/src/main/java/ch/njol/skript/expressions/ExprFilter.java @@ -51,7 +51,7 @@ import java.util.regex.Pattern; import java.util.stream.StreamSupport; -@Name("Filter") +@Name("Filter (Expression)") @Description({ "Filters a list based on a condition. ", "For example, if you ran 'broadcast \"something\" and \"something else\" where [string input is \"something\"]', ", diff --git a/src/main/java/ch/njol/skript/lang/Variable.java b/src/main/java/ch/njol/skript/lang/Variable.java index a9cfa472000..5ce5cc4a1c0 100644 --- a/src/main/java/ch/njol/skript/lang/Variable.java +++ b/src/main/java/ch/njol/skript/lang/Variable.java @@ -308,7 +308,7 @@ public Variable getConvertedExpression(Class... to) { // prevents e.g. {%expr%} where "%expr%" ends with "::*" from returning a Map if (name.endsWith(Variable.SEPARATOR + "*") != list) return null; - Object value = !list ? convertIfOldPlayer(name, event, Variables.getVariable(name, event, local)) : Variables.getVariable(name, event, local); + Object value = !list ? convertIfOldPlayer(name, local, event, Variables.getVariable(name, event, local)) : Variables.getVariable(name, event, local); if (value != null) return value; @@ -346,7 +346,7 @@ public Variable getConvertedExpression(Class... to) { else value = variable.getValue(); if (value != null) - convertedValues.add(convertIfOldPlayer(name + variable.getKey(), event, value)); + convertedValues.add(convertIfOldPlayer(name + variable.getKey(), local, event, value)); } } return convertedValues.toArray(); @@ -357,13 +357,13 @@ public Variable getConvertedExpression(Class... to) { * because the player object inside the variable will be a (kinda) dead variable * as a new player object has been created by the server. */ - @Nullable Object convertIfOldPlayer(String key, Event event, @Nullable Object object) { - if (SkriptConfig.enablePlayerVariableFix.value() && object instanceof Player) { - Player oldPlayer = (Player) object; + public static @Nullable T convertIfOldPlayer(String key, boolean local, Event event, @Nullable T object) { + if (SkriptConfig.enablePlayerVariableFix.value() && object instanceof Player oldPlayer) { if (!oldPlayer.isValid() && oldPlayer.isOnline()) { Player newPlayer = Bukkit.getPlayer(oldPlayer.getUniqueId()); Variables.setVariable(key, newPlayer, event, local); - return newPlayer; + //noinspection unchecked + return (T) newPlayer; } } return object; @@ -372,48 +372,7 @@ public Variable getConvertedExpression(Class... to) { public Iterator> variablesIterator(Event event) { if (!list) throw new SkriptAPIException("Looping a non-list variable"); - String name = StringUtils.substring(this.name.toString(event), 0, -1); - Object val = Variables.getVariable(name + "*", event, local); - if (val == null) - return new EmptyIterator<>(); - assert val instanceof TreeMap; - // temporary list to prevent CMEs - @SuppressWarnings("unchecked") - Iterator keys = new ArrayList<>(((Map) val).keySet()).iterator(); - return new Iterator<>() { - private @Nullable String key; - private @Nullable Object next = null; - - @Override - public boolean hasNext() { - if (next != null) - return true; - while (keys.hasNext()) { - key = keys.next(); - if (key != null) { - next = convertIfOldPlayer(name + key, event, Variables.getVariable(name + key, event, local)); - if (next != null && !(next instanceof TreeMap)) - return true; - } - } - next = null; - return false; - } - - @Override - public Pair next() { - if (!hasNext()) - throw new NoSuchElementException(); - Pair n = new Pair<>(key, next); - next = null; - return n; - } - - @Override - public void remove() { - throw new UnsupportedOperationException(); - } - }; + return Variables.getVariableIterator(name.toString(event), local, event); } @Override @@ -441,8 +400,9 @@ public boolean hasNext() { @Nullable String key = keys.next(); if (key != null) { next = Converters.convert(Variables.getVariable(name + key, event, local), types); + //noinspection unchecked - next = (T) convertIfOldPlayer(name + key, event, next); + next = (T) convertIfOldPlayer(name + key, local, event, next); if (next != null && !(next instanceof TreeMap)) return true; } diff --git a/src/main/java/ch/njol/skript/sections/SecFilter.java b/src/main/java/ch/njol/skript/sections/SecFilter.java new file mode 100644 index 00000000000..58d7abcdb6f --- /dev/null +++ b/src/main/java/ch/njol/skript/sections/SecFilter.java @@ -0,0 +1,192 @@ +package ch.njol.skript.sections; + +import ch.njol.skript.ScriptLoader; +import ch.njol.skript.Skript; +import ch.njol.skript.config.Node; +import ch.njol.skript.config.SectionNode; +import ch.njol.skript.doc.Description; +import ch.njol.skript.doc.Examples; +import ch.njol.skript.doc.Name; +import ch.njol.skript.doc.Since; +import ch.njol.skript.expressions.ExprInput; +import ch.njol.skript.lang.Condition; +import ch.njol.skript.lang.Expression; +import ch.njol.skript.lang.InputSource; +import ch.njol.skript.lang.Section; +import ch.njol.skript.lang.SkriptParser.ParseResult; +import ch.njol.skript.lang.TriggerItem; +import ch.njol.skript.lang.Variable; +import ch.njol.skript.lang.parser.ParserInstance; +import ch.njol.skript.variables.Variables; +import ch.njol.util.Kleenean; +import ch.njol.util.Pair; +import ch.njol.util.StringUtils; +import org.bukkit.event.Event; +import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.UnknownNullability; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.Spliterator; +import java.util.Spliterators; +import java.util.stream.StreamSupport; + +@Name("Filter (Section)") +@Description({ + "Filters a variable list based on the supplied conditions. Unlike the filter expression, this effect " + + "maintains the indices of the filtered list.", + "It also supports filtering based on meeting any of the given criteria, rather than all, like multi-line if statements." +}) +@Examples({ + "set {_a::*} to integers between -10 and 10", + "", + "filter {_a::*} to match:", + "\tinput is a number", + "\tmod(input, 2) = 0", + "\tinput > 0", + "", + "send {_a::*} # sends 2, 4, 6, 8, and 10", +}) +@Since("INSERT VERSION") +public class SecFilter extends Section implements InputSource { + + static { + Skript.registerSection(SecFilter.class, + "filter %objects% to match [:any|all]"); + if (!ParserInstance.isRegistered(InputSource.InputData.class)) + ParserInstance.registerData(InputSource.InputData.class, InputSource.InputData::new); + } + + @UnknownNullability + private Variable unfilteredObjects; + private final List conditions = new ArrayList<>(); + private boolean isAny; + + @Nullable + private Object currentValue; + @UnknownNullability + private String currentIndex; + private final Set> dependentInputs = new HashSet<>(); + + @Override + public boolean init(Expression[] expressions, int matchedPattern, Kleenean isDelayed, ParseResult parseResult, SectionNode sectionNode, List triggerItems) { + if (expressions[0].isSingle() || !(expressions[0] instanceof Variable)) { + Skript.error("You can only filter list variables!"); + return false; + } + unfilteredObjects = (Variable) expressions[0]; + isAny = parseResult.hasTag("any"); + + // Code pulled from SecConditional + ParserInstance parser = getParser(); + if (sectionNode.isEmpty()) { + Skript.error("filter sections must contain at least one condition"); + return false; + } + InputSource.InputData inputData = getParser().getData(InputSource.InputData.class); + InputSource originalSource = inputData.getSource(); + inputData.setSource(this); + try { + for (Node childNode : sectionNode) { + if (childNode instanceof SectionNode) { + Skript.error("filter sections may not contain other sections"); + return false; + } + String childKey = childNode.getKey(); + if (childKey != null) { + childKey = ScriptLoader.replaceOptions(childKey); + parser.setNode(childNode); + Condition condition = Condition.parse(childKey, "Can't understand this condition: '" + childKey + "'"); + parser.setNode(sectionNode); + // if this condition was invalid, don't bother parsing the rest + if (condition == null) + return false; + conditions.add(condition); + } + } + } finally { + inputData.setSource(originalSource); + } + + return true; + } + + @Override + protected @Nullable TriggerItem walk(Event event) { + // get the name only once to avoid issues where the name may change between evaluations. + String varName = unfilteredObjects.getName().toString(event); + String varSubName = StringUtils.substring(varName, 0, -1); + boolean local = unfilteredObjects.isLocal(); + + // not ideal to get this AND the iterator, but using this value could be unreliable due to name change issue from above. + // since we just use it for a length optimization at the end, it's ok to be a little unreliable. + var rawVariable = ((Map) unfilteredObjects.getRaw(event)); + if (rawVariable == null) + return getNext(); + int initialSize = rawVariable.size(); + + // we save both because we don't yet know which will be cheaper to use. + List> toKeep = new ArrayList<>(); + List toRemove = new ArrayList<>(); + + var variableIterator = Variables.getVariableIterator(varName, local, event); + var stream = StreamSupport.stream(Spliterators.spliteratorUnknownSize(variableIterator, Spliterator.ORDERED), false); + if (isAny) { + stream.forEach(pair -> { + currentValue = pair.getValue(); + currentIndex = pair.getKey(); + if (conditions.stream().anyMatch(c -> c.check(event))) { + toKeep.add(pair); + } else { + toRemove.add(pair.getKey()); + } + }); + } else { + stream.forEach(pair -> { + currentValue = pair.getValue(); + currentIndex = pair.getKey(); + if (conditions.stream().allMatch(c -> c.check(event))) { + toKeep.add(pair); + } else { + toRemove.add(pair.getKey()); + } + }); + } + + // optimize by either removing or clearing + adding depending on which is fewer operations + // for instances where only a handful of values are removed from a large list, this can be a 400% speedup + if (toKeep.size() < initialSize / 2) { + Variables.setVariable(varName, null, event, local); + for (Pair pair : toKeep) + Variables.setVariable(varSubName + pair.getKey(), pair.getValue(), event, local); + } else { + for (String index : toRemove) + Variables.setVariable(varSubName + index, null, event, local); + } + return getNext(); + } + + @Override + public Set> getDependentInputs() { + return dependentInputs; + } + + @Override + public @Nullable Object getCurrentValue() { + return currentValue; + } + + @Override + public @UnknownNullability String getCurrentIndex() { + return currentIndex; + } + + @Override + public String toString(@Nullable Event event, boolean debug) { + return "filter " + unfilteredObjects.toString(event, debug) + " to match " + (isAny ? "any" : "all"); + } + +} diff --git a/src/main/java/ch/njol/skript/variables/Variables.java b/src/main/java/ch/njol/skript/variables/Variables.java index 792e20a5659..745784b5e1e 100644 --- a/src/main/java/ch/njol/skript/variables/Variables.java +++ b/src/main/java/ch/njol/skript/variables/Variables.java @@ -32,8 +32,13 @@ import ch.njol.skript.variables.SerializedVariable.Value; import ch.njol.util.Kleenean; import ch.njol.util.NonNullPair; +import ch.njol.util.Pair; +import ch.njol.util.StringUtils; import ch.njol.util.SynchronizedReference; +import ch.njol.util.coll.iterator.EmptyIterator; import ch.njol.yggdrasil.Yggdrasil; +import com.google.common.collect.HashMultimap; +import com.google.common.collect.Multimap; import org.bukkit.Bukkit; import org.bukkit.configuration.serialization.ConfigurationSerializable; import org.bukkit.configuration.serialization.ConfigurationSerialization; @@ -42,18 +47,17 @@ import org.eclipse.jdt.annotation.Nullable; import org.skriptlang.skript.lang.converter.Converters; -import com.google.common.collect.HashMultimap; -import com.google.common.collect.Multimap; - import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Map.Entry; +import java.util.NoSuchElementException; import java.util.Optional; import java.util.Queue; import java.util.TreeMap; @@ -474,6 +478,67 @@ public static Object getVariable(String name, @Nullable Event event, boolean loc } } + /** + * Returns an iterator over the values of this list variable. + * + * @param name the variable's name. This must be the name of a list variable, ie. it must end in *. + * @param event if {@code local} is {@code true}, this is the event + * the local variable resides in. + * @param local if this variable is a local or global variable. + * @return an {@link Iterator} of {@link Pair}s, containing the {@link String} index and {@link Object} value of the + * elements of the list. An empty iterator is returned if the variable does not exist. + */ + public static Iterator> getVariableIterator(String name, boolean local, @Nullable Event event) { + assert name.endsWith("*"); + Object val = getVariable(name, event, local); + String subName = StringUtils.substring(name, 0, -1); + + if (val == null) + return new EmptyIterator<>(); + assert val instanceof TreeMap; + // temporary list to prevent CMEs + @SuppressWarnings("unchecked") + Iterator keys = new ArrayList<>(((Map) val).keySet()).iterator(); + return new Iterator<>() { + @Nullable + private String key; + @Nullable + private Object next = null; + + @Override + public boolean hasNext() { + if (next != null) + return true; + while (keys.hasNext()) { + key = keys.next(); + if (key != null) { + next = Variable.convertIfOldPlayer(subName + key, local, event, Variables.getVariable(subName + key, event, local)); + if (next != null && !(next instanceof TreeMap)) + return true; + } + } + next = null; + return false; + } + + @Override + public Pair next() { + if (!hasNext()) + throw new NoSuchElementException(); + Pair n = new Pair<>(key, next); + next = null; + return n; + } + + @Override + public void remove() { + if (key == null) + throw new IllegalStateException(); + Variables.deleteVariable(key, event, local); + } + }; + } + /** * Deletes a variable. * diff --git a/src/test/skript/tests/syntaxes/sections/SecFilter.sk b/src/test/skript/tests/syntaxes/sections/SecFilter.sk new file mode 100644 index 00000000000..11b0edd23e0 --- /dev/null +++ b/src/test/skript/tests/syntaxes/sections/SecFilter.sk @@ -0,0 +1,35 @@ +test "filter": + set {_a::*} to integers between -10 and 10 + + assert size of {_a::*} is 21 with "invalid starting size" + + filter {_a::*} to match: + mod(input, 2) = 0 + + assert size of {_a::*} is 11 with "failed to filter on mod()" + assert {_a::*} is -10, -8, -6, -4 ,-2, 0, 2, 4, 6, 8, 10 with "failed to filter on mod()" + + + set {_a::*} to integers between -10 and 10 + + assert size of {_a::*} is 21 with "invalid starting size" + + filter {_a::*} to match: + input is a number + mod(input, 2) = 0 + input > 0 + + assert size of {_a::*} is 5 with "failed to filter on mod(), >0, is number" + assert {_a::*} is 2, 4, 6, 8, 10 with "failed to filter on mod(), >0, is number" + + + set {_a::*} to integers between -10 and 10 + + assert size of {_a::*} is 21 with "invalid starting size" + + filter {_a::*} to match any: + mod(input, 2) = 0 + input > 0 + + assert size of {_a::*} is 16 with "failed to filter on any of mod(), >0" + assert {_a::*} is -10, -8, -6, -4, -2, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 with "failed to filter on any of mod(), >0"