Skip to content

Commit d80b0bf

Browse files
authored
Fix 'cannot be saved' on emphemeral vars. (#8024)
* patch fix for can't be saved warnings * switch to proper ephemeral api
1 parent 12662f3 commit d80b0bf

File tree

4 files changed

+70
-52
lines changed

4 files changed

+70
-52
lines changed

src/main/java/ch/njol/skript/effects/EffChange.java

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,5 @@
11
package ch.njol.skript.effects;
22

3-
import java.util.ArrayList;
4-
import java.util.Arrays;
5-
import java.util.List;
6-
import java.util.logging.Level;
7-
8-
import ch.njol.skript.expressions.ExprParse;
9-
import ch.njol.skript.lang.Effect;
10-
import ch.njol.skript.lang.Expression;
11-
import ch.njol.skript.lang.ExpressionList;
12-
import ch.njol.skript.lang.KeyProviderExpression;
13-
import ch.njol.skript.lang.KeyReceiverExpression;
14-
import ch.njol.skript.lang.SkriptParser;
15-
import ch.njol.skript.lang.SyntaxStringBuilder;
16-
import ch.njol.skript.lang.Variable;
17-
import ch.njol.skript.util.LiteralUtils;
18-
import ch.njol.skript.variables.HintManager;
19-
import org.skriptlang.skript.lang.script.ScriptWarning;
20-
import org.bukkit.event.Event;
21-
import org.jetbrains.annotations.Nullable;
22-
233
import ch.njol.skript.Skript;
244
import ch.njol.skript.SkriptConfig;
255
import ch.njol.skript.classes.Changer;
@@ -29,13 +9,25 @@
299
import ch.njol.skript.doc.Examples;
3010
import ch.njol.skript.doc.Name;
3111
import ch.njol.skript.doc.Since;
12+
import ch.njol.skript.expressions.ExprParse;
13+
import ch.njol.skript.lang.*;
3214
import ch.njol.skript.lang.SkriptParser.ParseResult;
3315
import ch.njol.skript.log.CountingLogHandler;
3416
import ch.njol.skript.log.ErrorQuality;
3517
import ch.njol.skript.log.ParseLogHandler;
3618
import ch.njol.skript.registrations.Classes;
19+
import ch.njol.skript.util.LiteralUtils;
3720
import ch.njol.skript.util.Patterns;
21+
import ch.njol.skript.variables.HintManager;
3822
import ch.njol.util.Kleenean;
23+
import org.bukkit.event.Event;
24+
import org.jetbrains.annotations.Nullable;
25+
import org.skriptlang.skript.lang.script.ScriptWarning;
26+
27+
import java.util.ArrayList;
28+
import java.util.Arrays;
29+
import java.util.List;
30+
import java.util.logging.Level;
3931

4032
@Name("Change: Set/Add/Remove/Remove All/Delete/Reset")
4133
@Description({
@@ -288,7 +280,7 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
288280
hintManager.add(variable, hints);
289281
}
290282
}
291-
if (!variable.isLocal()) {
283+
if (!variable.isLocal() && !variable.isEphemeral()) {
292284
ClassInfo<?> changerInfo = Classes.getSuperClassInfo(changer.getReturnType());
293285
if (changerInfo.getC() != Object.class && changerInfo.getSerializer() == null && changerInfo.getSerializeAs() == null
294286
&& !SkriptConfig.disableObjectCannotBeSavedWarnings.value()

src/main/java/ch/njol/skript/lang/Variable.java

Lines changed: 54 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,12 @@
11
package ch.njol.skript.lang;
22

3-
import java.lang.reflect.Array;
4-
import java.util.*;
5-
import java.util.Map.Entry;
6-
import java.util.NoSuchElementException;
7-
import java.util.TreeMap;
8-
import java.util.function.Predicate;
9-
import java.util.function.Function;
10-
113
import ch.njol.skript.Skript;
124
import ch.njol.skript.SkriptAPIException;
135
import ch.njol.skript.SkriptConfig;
146
import ch.njol.skript.classes.Changer;
157
import ch.njol.skript.classes.Changer.ChangeMode;
168
import ch.njol.skript.classes.Changer.ChangerUtils;
179
import ch.njol.skript.classes.ClassInfo;
18-
import com.google.common.collect.Iterators;
19-
import org.apache.commons.lang3.ArrayUtils;
20-
import org.skriptlang.skript.lang.arithmetic.Arithmetics;
21-
import org.skriptlang.skript.lang.arithmetic.OperationInfo;
22-
import org.skriptlang.skript.lang.arithmetic.Operator;
2310
import ch.njol.skript.lang.SkriptParser.ParseResult;
2411
import ch.njol.skript.lang.parser.ParserInstance;
2512
import ch.njol.skript.lang.util.SimpleExpression;
@@ -34,23 +21,34 @@
3421
import ch.njol.util.coll.CollectionUtils;
3522
import ch.njol.util.coll.iterator.EmptyIterator;
3623
import ch.njol.util.coll.iterator.SingleItemIterator;
24+
import com.google.common.collect.Iterators;
25+
import org.apache.commons.lang3.ArrayUtils;
3726
import org.bukkit.Bukkit;
38-
import org.bukkit.World;
3927
import org.bukkit.entity.Player;
4028
import org.bukkit.event.Event;
4129
import org.jetbrains.annotations.NotNull;
4230
import org.jetbrains.annotations.Nullable;
31+
import org.skriptlang.skript.lang.arithmetic.Arithmetics;
32+
import org.skriptlang.skript.lang.arithmetic.OperationInfo;
33+
import org.skriptlang.skript.lang.arithmetic.Operator;
4334
import org.skriptlang.skript.lang.comparator.Comparators;
4435
import org.skriptlang.skript.lang.comparator.Relation;
4536
import org.skriptlang.skript.lang.converter.Converters;
4637
import org.skriptlang.skript.lang.script.Script;
4738
import org.skriptlang.skript.lang.script.ScriptWarning;
4839

40+
import java.lang.reflect.Array;
41+
import java.util.*;
42+
import java.util.Map.Entry;
43+
import java.util.function.Function;
44+
import java.util.function.Predicate;
45+
4946
public class Variable<T> implements Expression<T>, KeyReceiverExpression<T>, KeyProviderExpression<T> {
5047

5148
private final static String SINGLE_SEPARATOR_CHAR = ":";
5249
public final static String SEPARATOR = SINGLE_SEPARATOR_CHAR + SINGLE_SEPARATOR_CHAR;
5350
public final static String LOCAL_VARIABLE_TOKEN = "_";
51+
public static final String EPHEMERAL_VARIABLE_TOKEN = "-";
5452
private static final char[] reservedTokens = {'~', '.', '+', '$', '!', '&', '^', '*'};
5553

5654
/**
@@ -67,13 +65,14 @@ public class Variable<T> implements Expression<T>, KeyReceiverExpression<T>, Key
6765
private final Class<? extends T>[] types;
6866

6967
private final boolean local;
68+
private final boolean ephemeral;
7069
private final boolean list;
7170

7271
private final @Nullable Variable<?> source;
7372
private final Map<Event, String[]> cache = new WeakHashMap<>();
7473

7574
@SuppressWarnings("unchecked")
76-
private Variable(VariableString name, Class<? extends T>[] types, boolean local, boolean list, @Nullable Variable<?> source) {
75+
private Variable(VariableString name, Class<? extends T>[] types, boolean local, boolean ephemeral, boolean list, @Nullable Variable<?> source) {
7776
assert types.length > 0;
7877

7978
assert name.isSimple() || name.getMode() == StringMode.VARIABLE_NAME;
@@ -83,6 +82,7 @@ private Variable(VariableString name, Class<? extends T>[] types, boolean local,
8382
this.script = parser.isActive() ? parser.getCurrentScript() : null;
8483

8584
this.local = local;
85+
this.ephemeral = ephemeral;
8686
this.list = list;
8787

8888
this.name = name;
@@ -160,10 +160,14 @@ else if (character == '%')
160160
}
161161

162162
/**
163-
* Prints errors
163+
* Creates a new variable instance with the given name and types. Prints errors.
164+
* @param name The raw name of the variable.
165+
* @param types The types this variable is expected to be.
166+
* @return A new variable instance, or null if the name is invalid or the variable could not be created.
167+
* @param <T> The supertype the variable is expected to be.
164168
*/
165169
public static <T> @Nullable Variable<T> newInstance(String name, Class<? extends T>[] types) {
166-
name = "" + name.trim();
170+
name = name.trim();
167171
if (!isValidVariableName(name, true, true))
168172
return null;
169173
VariableString variableString = VariableString.newInstance(
@@ -172,17 +176,28 @@ else if (character == '%')
172176
return null;
173177

174178
boolean isLocal = name.startsWith(LOCAL_VARIABLE_TOKEN);
179+
boolean isEphemeral = name.startsWith(EPHEMERAL_VARIABLE_TOKEN);
175180
boolean isPlural = name.endsWith(SEPARATOR + "*");
176181

177182
ParserInstance parser = ParserInstance.get();
178183
Script currentScript = parser.isActive() ? parser.getCurrentScript() : null;
184+
185+
// check for 'starting with expression' warning
179186
if (currentScript != null
180187
&& !SkriptConfig.disableVariableStartingWithExpressionWarnings.value()
181-
&& !currentScript.suppressesWarning(ScriptWarning.VARIABLE_STARTS_WITH_EXPRESSION)
182-
&& (isLocal ? name.substring(LOCAL_VARIABLE_TOKEN.length()) : name).startsWith("%")) {
183-
Skript.warning("Starting a variable's name with an expression is discouraged ({" + name + "}). " +
184-
"You could prefix it with the script's name: " +
185-
"{" + StringUtils.substring(currentScript.getConfig().getFileName(), 0, -3) + SEPARATOR + name + "}");
188+
&& !currentScript.suppressesWarning(ScriptWarning.VARIABLE_STARTS_WITH_EXPRESSION)) {
189+
190+
String strippedName = name;
191+
if (isLocal) {
192+
strippedName = strippedName.substring(LOCAL_VARIABLE_TOKEN.length());
193+
} else if (isEphemeral) {
194+
strippedName = strippedName.substring(EPHEMERAL_VARIABLE_TOKEN.length());
195+
}
196+
if (strippedName.startsWith("%")) {
197+
Skript.warning("Starting a variable's name with an expression is discouraged ({" + name + "}). " +
198+
"You could prefix it with the script's name: " +
199+
"{" + StringUtils.substring(currentScript.getConfig().getFileName(), 0, -3) + SEPARATOR + name + "}");
200+
}
186201
}
187202

188203
// Check for local variable type hints
@@ -191,7 +206,7 @@ else if (character == '%')
191206
if (!hints.isEmpty()) { // Type hint(s) available
192207
if (types[0] == Object.class) { // Object is generic, so we initialize with the hints instead
193208
//noinspection unchecked
194-
return new Variable<>(variableString, hints.toArray(new Class[0]), true, isPlural, null);
209+
return new Variable<>(variableString, hints.toArray(new Class[0]), true, isEphemeral, isPlural, null);
195210
}
196211

197212
List<Class<? extends T>> potentialTypes = new ArrayList<>();
@@ -205,7 +220,7 @@ else if (character == '%')
205220
}
206221
if (!potentialTypes.isEmpty()) { // Hint matches, use variable with exactly correct type
207222
//noinspection unchecked
208-
return new Variable<>(variableString, potentialTypes.toArray(Class[]::new), true, isPlural, null);
223+
return new Variable<>(variableString, potentialTypes.toArray(Class[]::new), true, isEphemeral, isPlural, null);
209224
}
210225

211226
// Hint exists and does NOT match any types requested
@@ -223,18 +238,31 @@ else if (character == '%')
223238
}
224239
}
225240

226-
return new Variable<>(variableString, types, isLocal, isPlural, null);
241+
return new Variable<>(variableString, types, isLocal, isEphemeral, isPlural, null);
227242
}
228243

229244
@Override
230245
public boolean init(Expression<?>[] expressions, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) {
231246
throw new UnsupportedOperationException();
232247
}
233248

249+
/**
250+
* @return Whether this variable is a local variable, i.e. starts with {@link #LOCAL_VARIABLE_TOKEN}.
251+
*/
234252
public boolean isLocal() {
235253
return local;
236254
}
237255

256+
/**
257+
* @return Whether this variable is an ephemeral variable, i.e. starts with {@link #EPHEMERAL_VARIABLE_TOKEN}.
258+
*/
259+
public boolean isEphemeral() {
260+
return ephemeral;
261+
}
262+
263+
/**
264+
* @return Whether this variable is a list variable, i.e. ends with {@link #SEPARATOR + "*"}.
265+
*/
238266
public boolean isList() {
239267
return list;
240268
}
@@ -296,7 +324,7 @@ public <R> Variable<R> getConvertedExpression(Class<R>... to) {
296324
if (!converterExists) {
297325
return null;
298326
}
299-
return new Variable<>(name, to, local, list, this);
327+
return new Variable<>(name, to, local, ephemeral, list, this);
300328
}
301329

302330
/**

src/main/java/ch/njol/skript/variables/FlatFileStorage.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ public final void saveVariables(boolean finalSave) {
456456
*/
457457
@SuppressWarnings("unchecked")
458458
private void save(PrintWriter pw, String parent, TreeMap<String, Object> map) {
459-
if (parent.startsWith(Variables.EPHEMERAL_VARIABLE_PREFIX))
459+
if (parent.startsWith(Variable.EPHEMERAL_VARIABLE_TOKEN))
460460
// Skip ephemeral variables
461461
return;
462462

@@ -475,7 +475,7 @@ private void save(PrintWriter pw, String parent, TreeMap<String, Object> map) {
475475
// Remove variable separator if needed
476476
String name = childKey == null ? parent.substring(0, parent.length() - Variable.SEPARATOR.length()) : parent + childKey;
477477

478-
if (name.startsWith(Variables.EPHEMERAL_VARIABLE_PREFIX))
478+
if (name.startsWith(Variable.EPHEMERAL_VARIABLE_TOKEN))
479479
// Skip ephemeral variables
480480
continue;
481481

src/main/java/ch/njol/skript/variables/Variables.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,6 @@ public class Variables {
8282
*/
8383
private static final String CONFIGURATION_SERIALIZABLE_PREFIX = "ConfigurationSerializable_";
8484

85-
public static final String EPHEMERAL_VARIABLE_PREFIX = "-";
86-
8785
private final static Multimap<Class<? extends VariablesStorage>, String> TYPES = HashMultimap.create();
8886

8987
// Register some things with Yggdrasil
@@ -889,7 +887,7 @@ public static SerializedVariable serialize(String name, @Nullable Object value)
889887
* @param value the value of the variable.
890888
*/
891889
private static void saveVariableChange(String name, @Nullable Object value) {
892-
if (name.startsWith(Variables.EPHEMERAL_VARIABLE_PREFIX))
890+
if (name.startsWith(Variable.EPHEMERAL_VARIABLE_TOKEN))
893891
return;
894892
saveQueue.add(serialize(name, value));
895893
}

0 commit comments

Comments
 (0)