improve: cache clearing improvements (#88)

* improve: cache clearing improvements

* improve: add EffContinue test for while loops

* improve: change the Expression javadocs to be more informative

Co-authored-by: Mwexim <[email protected]>

Author: Mwexim

src/main/java/io/github/syst3ms/skriptparser/Skript.java

@@ -4,6 +4,7 @@
 import io.github.syst3ms.skriptparser.lang.SkriptEvent;
 import io.github.syst3ms.skriptparser.lang.Statement;
 import io.github.syst3ms.skriptparser.lang.Trigger;
+import io.github.syst3ms.skriptparser.lang.base.ExecutableExpression;
 import io.github.syst3ms.skriptparser.registration.SkriptAddon;
 import io.github.syst3ms.skriptparser.util.ThreadUtils;
 import io.github.syst3ms.skriptparser.util.Time;
@@ -70,4 +71,9 @@ public void finishedLoading() {
             ThreadUtils.runPeriodically(() -> Statement.runAll(trigger, ctx), initialDelay, Duration.ofDays(1));
         }
     }
+
+    @Override
+    public void walkingForward() {
+        ExecutableExpression.getCachedValues().clear();
+    }
 }

src/main/java/io/github/syst3ms/skriptparser/effects/EffAsync.java

@@ -44,7 +44,7 @@ public boolean init(Expression<?>[] expressions, int matchedPattern, ParseContex
 
     @Override
     public void execute(TriggerContext ctx) {
-        ThreadUtils.runAsync(() -> effect.walk(ctx, true));
+        ThreadUtils.runAsync(() -> effect.walk(ctx));
     }
 
     @Override

src/main/java/io/github/syst3ms/skriptparser/effects/EffChange.java

@@ -163,9 +163,10 @@ public void execute(TriggerContext ctx) {
         if (changeWith == null) {
             changed.change(ctx, new Object[0], mode);
         } else {
-            if (changeWith.getValues(ctx).length == 0)
+            var values= changeWith.getValues(ctx);
+            if (values.length == 0)
                 return;
-            changed.change(ctx, changeWith.getValues(ctx), mode);
+            changed.change(ctx, values, mode);
         }
     }
 }

src/main/java/io/github/syst3ms/skriptparser/effects/EffContinue.java

@@ -58,8 +58,8 @@ public void execute(TriggerContext ctx) {
     }
 
     @Override
-    protected Optional<? extends Statement> walk(TriggerContext ctx) {
-        loop.walk(ctx, true);
+	public Optional<? extends Statement> walk(TriggerContext ctx) {
+        loop.walk(ctx);
         return Optional.empty();
     }
 

src/main/java/io/github/syst3ms/skriptparser/effects/EffDoIf.java

@@ -53,7 +53,7 @@ public boolean init(Expression<?>[] expressions, int matchedPattern, ParseContex
     @Override
     public void execute(TriggerContext ctx) {
         if (condition.getSingle(ctx).filter(b -> b).isPresent())
-            effect.walk(ctx, true);
+            effect.walk(ctx);
     }
 
     @Override

src/main/java/io/github/syst3ms/skriptparser/effects/EffWait.java

@@ -67,9 +67,10 @@ public Optional<? extends Statement> walk(TriggerContext ctx) {
             return Optional.empty();
 
         if (isConditional) {
+            var cond = condition.getSingle(ctx);
             // The code we want to run each check.
             Consumer<ExecutorService> code = exec -> {
-                if (condition.getSingle(ctx).filter(b -> negated == b).isPresent()) {
+                if (cond.filter(b -> negated == b).isPresent()) {
                     Statement.runAll(getNext().get(), ctx);
                     exec.shutdownNow();
                 }

src/main/java/io/github/syst3ms/skriptparser/expressions/CondExprCompare.java

@@ -244,8 +244,7 @@ private boolean initialize() {
                 : ClassUtils.getCommonSuperclass(second.getReturnType(), third.getReturnType());
         if (f == Object.class || s == Object.class)
             return true;
-        comp = (Comparator<Object, Object>) Comparators.getComparator(f, s).orElse(null);
-        return comp != null;
+        return (comp = (Comparator<Object, Object>) Comparators.getComparator(f, s).orElse(null)) != null;
     }
 
     /*

src/main/java/io/github/syst3ms/skriptparser/lang/Expression.java

@@ -29,6 +29,7 @@
 public interface Expression<T> extends SyntaxElement {
     /**
      * Retrieves all values of this Expression.
+     * Note that this method may have possible side-effects.
      * @param ctx the event
      * @return an array of the values
      */

src/main/java/io/github/syst3ms/skriptparser/lang/Statement.java

@@ -1,6 +1,7 @@
 package io.github.syst3ms.skriptparser.lang;
 
 import io.github.syst3ms.skriptparser.lang.base.ExecutableExpression;
+import io.github.syst3ms.skriptparser.registration.SkriptAddon;
 import org.jetbrains.annotations.Nullable;
 
 import java.util.Optional;
@@ -26,7 +27,7 @@ public static boolean runAll(Statement start, TriggerContext context) {
         Optional<? extends Statement> item = Optional.of(start);
         try {
             while (item.isPresent())
-                item = item.flatMap(i -> i.walk(context, true));
+                item = item.flatMap(i -> i.walk(context));
             return true;
         } catch (StackOverflowError so) {
             System.err.println("The script repeated itself infinitely !");
@@ -88,25 +89,15 @@ public Statement setNext(@Nullable Statement next) {
     /**
      * By default, runs {@link #run(TriggerContext)} ; returns {@link #getNext()} if it returns true, or {@code null} otherwise.
      * Note that if this method is overridden, then the implementation of {@linkplain #run(TriggerContext)} doesn't matter.
+     * <br>
+     * This method also calls {@link SkriptAddon#walkingForward()} for all registered addons. This
+     * implementation is not particularly required, unless you are make extensive use of {@link ExecutableExpression}
+     * and cache that needs to be cleared after each statement.
      * @param ctx the event
-     * @param clearCache whether or not some of the ongoing cache should be cleared
      * @return the next item to be ran, or {@code null} if this is the last item to be executed
      */
-    public final Optional<? extends Statement> walk(TriggerContext ctx, boolean clearCache) {
-        if (clearCache) {
-            // Clear all cache needed to be cleared.
-            ExecutableExpression.getCachedValues().clear();
-        }
-        return walk(ctx);
-    }
-
-    /**
-     * By default, runs {@link #run(TriggerContext)} ; returns {@link #getNext()} if it returns true, or {@code null} otherwise.
-     * Note that if this method is overridden, then the implementation of {@linkplain #run(TriggerContext)} doesn't matter.
-     * @param ctx the event
-     * @return the next item to be ran, or {@code null} if this is the last item to be executed
-     */
-    protected Optional<? extends Statement> walk(TriggerContext ctx) {
+    public Optional<? extends Statement> walk(TriggerContext ctx) {
+        SkriptAddon.getAddons().forEach(SkriptAddon::walkingForward);
         var proceed = run(ctx);
         if (proceed) {
             return getNext();

src/main/java/io/github/syst3ms/skriptparser/lang/lambda/SkriptConsumer.java

@@ -41,7 +41,7 @@ public void accept(TriggerContext ctx, Object... args) {
         var item = starterFunction.apply(section);
         while (item.isPresent()) {
             var cur = item.orElse(null);
-            item = item.flatMap(s -> s.walk(ctx, true));
+            item = item.flatMap(s -> s.walk(ctx));
             if (stepFunction.test(section, cur)) {
                 break;
             }

src/main/java/io/github/syst3ms/skriptparser/lang/lambda/SkriptFunction.java

@@ -45,7 +45,7 @@ public Optional<T[]> apply(TriggerContext ctx, Object... args) {
         var item = starterFunction.apply(section);
         while (item.isPresent()) {
             var cur = item.orElse(null);
-            item = item.flatMap(s -> s.walk(ctx, true));
+            item = item.flatMap(s -> s.walk(ctx));
             var stepResult = stepFunction.apply(section, cur);
             if (stepResult.isPresent()) {
                 result = stepResult.get();

src/main/java/io/github/syst3ms/skriptparser/registration/SkriptAddon.java

@@ -36,6 +36,12 @@ public static List<SkriptAddon> getAddons() {
      */
     public void finishedLoading() {}
 
+    /**
+     * Is called when moving to the next statement of the parser. Optionally overridable.
+     * Note that this is not always called, only in most occasions.
+     */
+    public void walkingForward() {}
+
     /**
      * Checks to see whether the given event has been registered by this SkriptAddon ; a basic way to filter out
      * triggers you aren't able to deal with in {@link SkriptAddon#handleTrigger(Trigger)}.

src/main/java/io/github/syst3ms/skriptparser/sections/SecAsync.java

@@ -51,7 +51,7 @@ public Optional<? extends Statement> walk(TriggerContext ctx) {
         Optional<? extends Statement>[] item = new Optional[]{getFirst()};
         ThreadUtils.runAsync(() -> {
             while (!item[0].equals(getNext())) // Calling equals() on optionals calls equals() on their values
-                item[0] = item[0].flatMap(i -> i.walk(ctx, true));
+                item[0] = item[0].flatMap(i -> i.walk(ctx));
         });
         return getNext();
     }

src/test/java/io/github/syst3ms/skriptparser/syntax/SecBirth.java

@@ -54,7 +54,7 @@ public boolean init(Expression<?>[] expressions, int matchedPattern, ParseContex
     public Optional<? extends Statement> walk(TriggerContext ctx) {
         var item = getFirst();
         while (!item.equals(getNext())) {
-            item = item.flatMap(val -> val.walk(ctx, true));
+            item = item.flatMap(val -> val.walk(ctx));
         }
         if (currentDeaths.isEmpty()) {
             SyntaxParserTest.addError(new SkriptRuntimeException(

src/test/resources/effects/EffContinue.txt

@@ -2,11 +2,22 @@
 #	- CHarcoalToast
 # 	- Mwexim
 # Date: 2020/10/31
+#	- 2020/12/27
 
 test:
-	{var} = 0
+	set {var} to 0
 	loop 5 times:
 		if loop-value < 3:
 			continue
 		add 1 to {var}
-	assert {var} = 3 with "'continue'-statement didn't work properly: %{var}% != 3"
+	assert {var} = 3 with "{var} (in loop) should equal 3: %{var}%"
+
+	set {var} to 0
+	set {flag} to 0
+	while {var} < 1:
+		if {flag} < 5:
+			add 1 to {flag}
+			continue
+		add 1 to {var}
+	assert {var} = 1 with "{var} (in while) should equal 1: %{var}%"
+	assert {flag} = 4 with "{flag} (in while) should equal 4: %{flag}%"