Skip to content

Commit

Permalink
Keyed Set Mode (#7072)
Browse files Browse the repository at this point in the history
* Add key providers/receivers.

* Add changers & list variable support.

* Clean up default method.

* Add basic tests.

* Changes.

* Fix merge gunk.
  • Loading branch information
Moderocky authored Dec 18, 2024
1 parent a44df2b commit adf289d
Show file tree
Hide file tree
Showing 7 changed files with 346 additions and 38 deletions.
8 changes: 7 additions & 1 deletion src/main/java/ch/njol/skript/classes/Changer.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,14 @@ public interface Changer<T> {

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 <code>delta</code> to be.
* <p>
Expand Down
61 changes: 32 additions & 29 deletions src/main/java/ch/njol/skript/effects/EffChange.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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++) {
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -316,5 +319,5 @@ public String toString(final @Nullable Event e, final boolean debug) {
assert false;
return "";
}

}
128 changes: 128 additions & 0 deletions src/main/java/ch/njol/skript/lang/KeyProviderExpression.java
Original file line number Diff line number Diff line change
@@ -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.
* <br/>
* Index-linking is not (currently) used with other change modes.
* <br/>
* <br/>
* <h2>Contract</h2>
* <ul>
* <li>Neither {@link #getArrayKeys(Event)} nor {@link #getAllKeys(Event)} should ever be called without
* a corresponding {@link #getArray(Event)} or {@link #getAll(Event)} call.</li>
* <li>A caller may ask only for values and does not have to invoke either {@link #getArrayKeys(Event)} or
* {@link #getAllKeys(Event)}.</li>
* <li>{@link #getArrayKeys(Event)} might be called after the corresponding {@link #getArray(Event)}</li>
* <li>{@link #getAllKeys(Event)} might be called after the corresponding {@link #getAll(Event)}</li>
* </ul>
* <br/>
* <h2>Advice on Caching</h2>
* 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.
* <br/>
* <br/>
* <h3>Caveats</h3>
* <ol>
* <li>The caller may <i>never</i> ask for {@link #getArrayKeys(Event)}.
* The cache should be disposed of in a timely manner.</li>
* <li>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
* .</li>
* </ol>
* Note that the caller may <i>never</i> ask for {@link #getArrayKeys(Event)} and so the cache should be disposed of
* in a timely manner.
* <br/>
* <pre>{@code
* Map<Event, Collection<String>> cache = new WeakHashMap<>();
*
* public Object[] getArray(Event event) {
* Set<Entry<String, T>> 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
* }
* }</pre>
*
* @see Expression
* @see KeyReceiverExpression
*/
public interface KeyProviderExpression<T> extends Expression<T> {

/**
* A set of keys, matching the length and order of the immediately-previous
* {@link #getArray(Event)} values array.
* <br/>
* This should <b>only</b> 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:
* <ul>
* <li>the method may throw an error,</li>
* <li>the method may return keys not matching any previous values,</li>
* <li>or the method may return nothing at all.</li>
* </ul>
*
* @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.
* <br/>
* This should <b>only</b> 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:
* <ul>
* <li>the method may throw an error,</li>
* <li>the method may return keys not matching any previous values,</li>
* <li>or the method may return nothing at all.</li>
* </ul>
*
* @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 <i>offer</i> 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;
}

}
31 changes: 31 additions & 0 deletions src/main/java/ch/njol/skript/lang/KeyReceiverExpression.java
Original file line number Diff line number Diff line change
@@ -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<T> extends Expression<T> {

/**
* 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);

}
Loading

0 comments on commit adf289d

Please sign in to comment.