Skip to content

Commit

Permalink
Fix supportsLoopPeeking
Browse files Browse the repository at this point in the history
  • Loading branch information
TheAbsolutionism committed Nov 28, 2024
1 parent 0e11d17 commit 273279c
Show file tree
Hide file tree
Showing 9 changed files with 92 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public boolean init(Expression<?>[] vars, int matchedPattern, Kleenean isDelayed
return false;
}
if (selectedState == LoopState.NEXT && !loop.supportsPeeking()) {
Skript.error("This loop does not allow the usage of 'next loop-" + s + "'");
Skript.error("The expression '" + loop.getExpression().toString() + "' does not allow the usage of 'next loop-" + s + "'.");
return false;
}
if (loop.getLoopedExpression() instanceof Variable) {
Expand Down
25 changes: 1 addition & 24 deletions src/main/java/ch/njol/skript/lang/Expression.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
* @see SimpleExpression
* @see SyntaxElement
*/
public interface Expression<T> extends SyntaxElement, Debuggable {
public interface Expression<T> extends SyntaxElement, Debuggable, Loopable<T> {

/**
* Get the single value of this expression.
Expand Down Expand Up @@ -257,25 +257,6 @@ default boolean canReturn(Class<?> returnType) {
*/
boolean isDefault();

/**
* Returns the same as {@link #getArray(Event)} but as an iterator. This method should be overriden by expressions intended to be looped to increase performance.
*
* @param event The event to be used for evaluation
* @return An iterator to iterate over all values of this expression which may be empty and/or null, but must not return null elements.
*/
@Nullable Iterator<? extends T> iterator(Event event);

/**
* Checks whether the given 'loop-...' expression should match this loop, e.g. loop-block matches any loops that loop through blocks and loop-argument matches an
* argument loop.
* <p>
* You should usually just return false as e.g. loop-block will automatically match the expression if its returnType is Block or a subtype of it.
*
* @param input The entered input string (the blank in loop-___)
* @return Whether this loop matches the given string
*/
boolean isLoopOf(String input);

/**
* Returns the original expression that was parsed, i.e. without any conversions done.
* <p>
Expand Down Expand Up @@ -381,8 +362,4 @@ default Map<ChangeMode, Class<?>[]> getAcceptedChangeModes() {
return newDelta == null ? delta : newDelta;
}

default boolean isLoopPeeking() {
return true;
};

}
39 changes: 39 additions & 0 deletions src/main/java/ch/njol/skript/lang/Loopable.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package ch.njol.skript.lang;

import ch.njol.skript.lang.util.SimpleExpression;
import org.bukkit.event.Event;
import org.jetbrains.annotations.Nullable;

import java.util.Iterator;

public interface Loopable<T> {

/**
* Returns the same as {@link Expression#getArray(Event)} but as an iterator. This method should be overridden by expressions intended to be looped to increase performance.
* @see SimpleExpression#iterator(Event)
*
* @param event The event to be used for evaluation
* @return An iterator to iterate over all values of this expression which may be empty and/or null, but must not return null elements.
*/
@Nullable Iterator<? extends T> iterator(Event event);

/**
* Checks whether the given 'loop-...' expression should match this loop, e.g. loop-block matches any loops that loop through blocks and loop-argument matches an
* argument loop.
* <p>
* You should usually just return false as e.g. loop-block will automatically match the expression if its returnType is Block or a subtype of it.
*
* @param input The entered input string (the blank in loop-___)
* @return Whether this loop matches the given string
*/
boolean isLoopOf(String input);

/**
* Checks whether the expression allows using 'next loop-value' when provided as the iterator for a SecLoop.
* @return
*/
default boolean supportsLoopPeeking() {
return false;
};

}
5 changes: 5 additions & 0 deletions src/main/java/ch/njol/skript/lang/Variable.java
Original file line number Diff line number Diff line change
Expand Up @@ -694,4 +694,9 @@ public Expression<? extends T> simplify() {
return this;
}

@Override
public boolean supportsLoopPeeking() {
return true;
}

}
20 changes: 12 additions & 8 deletions src/main/java/ch/njol/skript/lang/util/SimpleExpression.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@
import ch.njol.skript.classes.ClassInfo;
import ch.njol.skript.lang.Expression;
import ch.njol.skript.lang.ExpressionType;
import ch.njol.skript.lang.Loopable;
import ch.njol.skript.registrations.Classes;
import ch.njol.skript.util.Utils;
import ch.njol.util.Checker;
import ch.njol.util.Kleenean;
import ch.njol.util.coll.CollectionUtils;
import ch.njol.util.coll.iterator.ArrayIterator;
import com.google.common.collect.PeekingIterator;
import org.bukkit.event.Event;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand All @@ -51,7 +53,6 @@
public abstract class SimpleExpression<T> implements Expression<T> {

private int time = 0;
private boolean loopPeeking = true;

protected SimpleExpression() {}

Expand Down Expand Up @@ -353,6 +354,14 @@ public boolean isLoopOf(String input) {
return false;
}

/**
* {@inheritDoc}
* Overriding this method, returning an {@link Iterator}, ensure to override {@link Loopable#supportsLoopPeeking()}
* Or returning an {@link PeekingIterator}, ensure to set up {@link PeekingIterator#peek()}
*
* @param event The event to be used for evaluation
* @return {@link ArrayIterator}
*/
@Override
public @Nullable Iterator<? extends T> iterator(Event event) {
return new ArrayIterator<>(getArray(event));
Expand All @@ -378,13 +387,8 @@ public boolean getAnd() {
return true;
}

protected final void setLoopPeeking(boolean loopPeeking) {
this.loopPeeking = loopPeeking;
}

@Override
public boolean isLoopPeeking() {
return loopPeeking;
public boolean supportsLoopPeeking() {
return true;
}

}
14 changes: 9 additions & 5 deletions src/main/java/ch/njol/skript/sections/SecLoop.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import ch.njol.skript.util.Container.ContainerType;
import ch.njol.skript.util.LiteralUtils;
import ch.njol.util.Kleenean;
import com.google.common.collect.PeekingIterator;
import org.bukkit.event.Event;
import org.jetbrains.annotations.Nullable;

Expand Down Expand Up @@ -78,13 +79,12 @@ public class SecLoop extends LoopSection {

private final transient Map<Event, Object> current = new WeakHashMap<>();
private final transient Map<Event, Iterator<?>> currentIter = new WeakHashMap<>();
private final transient Map<Event, Object> next = new WeakHashMap<>();
private final transient Map<Event, Object> previous = new WeakHashMap<>();

private @Nullable TriggerItem actualNext;
private boolean guaranteedToLoop;
private Object nextValue = null;
private boolean loopPeeking = true;
private boolean loopPeeking;

@Override
@SuppressWarnings("unchecked")
Expand All @@ -111,9 +111,7 @@ public boolean init(Expression<?>[] exprs,
Skript.error("Can't loop '" + expr + "' because it's only a single value");
return false;
}


loopPeeking = exprs[0].isLoopPeeking();
loopPeeking = exprs[0].supportsLoopPeeking();

guaranteedToLoop = guaranteedToLoop(expr);
loadOptionalCode(sectionNode);
Expand Down Expand Up @@ -172,6 +170,8 @@ public String toString(@Nullable Event event, boolean debug) {
Iterator<?> iter = currentIter.get(event);
if (iter == null || !iter.hasNext())
return null;
if (iter instanceof PeekingIterator<?> peekingIterator)
return peekingIterator.peek();
nextValue = iter.next();
return nextValue;
}
Expand Down Expand Up @@ -236,4 +236,8 @@ public boolean supportsPeeking() {
return loopPeeking;
}

public Expression<?> getExpression() {
return expr;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ public class ExprTestLoopPeeking extends SimpleExpression<Object> {
"test loop peeking enabled");
}

private boolean toPeek;

@Override
public boolean init(Expression<?>[] expressions, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) {
if (matchedPattern == 0)
setLoopPeeking(false);
toPeek = matchedPattern == 1;
return true;
}

Expand All @@ -39,6 +40,12 @@ public Class<Object> getReturnType() {
return Object.class;
}

@Override
public boolean supportsLoopPeeking() {
Skript.adminBroadcast("Test Loop Peeking");
return toPeek;
}

@Override
public String toString(@Nullable Event event, boolean debug) {
return "test loop peeking";
Expand Down
18 changes: 13 additions & 5 deletions src/main/java/ch/njol/util/coll/iterator/ArrayIterator.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,17 @@
*/
package ch.njol.util.coll.iterator;

import java.util.Iterator;
import java.util.NoSuchElementException;

import com.google.common.collect.PeekingIterator;
import org.jetbrains.annotations.Nullable;

import java.util.NoSuchElementException;

/**
* A simple iterator to iterate over an array.
*
* @author Peter Güttinger
*/
public class ArrayIterator<T> implements Iterator<T> {
public class ArrayIterator<T> implements PeekingIterator<T> {

@Nullable
private final T[] array;
Expand All @@ -51,7 +51,15 @@ public boolean hasNext() {
return false;
return index < array.length;
}


@Override
public T peek() {
int peekIndex = index + 1;
if (array == null || peekIndex >= array.length)
return null;
return array[peekIndex];
}

@Override
@Nullable
public T next() {
Expand Down
5 changes: 3 additions & 2 deletions src/test/skript/tests/syntaxes/expressions/ExprLoopValue.sk
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,10 @@ test "loop peeking":
parse:
loop test loop peeking disabled:
set {_next} to next loop-value
assert last parse logs contains "This loop does not allow the usage of 'next loop-value'" with "Disabled loop peeking did not error."
broadcast last parse logs
assert last parse logs contains "The expression 'test loop peeking' does not allow the usage of 'next loop-value'." with "Disabled loop peeking did not error."

parse:
loop test loop peeking enabled:
set {_next} to next loop-value
assert last parse logs does not contain "This loop does not allow the usage of 'next loop-value'" with "Enabled loop peeking should not error."
assert last parse logs does not contain "The expression 'test loop peeking' does not allow the usage of 'next loop-value'." with "Enabled loop peeking should not error."

0 comments on commit 273279c

Please sign in to comment.