Skript: Local variables are local to the event, but not the trigger

[Pikachu920]

https://github.com/bensku/Skript/blob/master/src/main/java/ch/njol/skript/variables/Variables.java#L243

on click:
  set {_oops} to "this is a bug"

on click:
  wait 1 second
  broadcast {_oops}

load this code, then click. Because both triggers are running under the same event and the localVariables map is not dependent on trigger, "this is a bug" will be broadcasted even though {_oops} shouldn't be set in the second trigger.

[bensku]

That is really, really bad. Can break stuff without anyone noticing in subtle ways.

[Govindass]

really waiting for this fix, have had many problems due to it already

[Pikachu920]

Can't be too rash with fixing this either, because the localVariables map can't actually be dependent on a Trigger since there are cases where there is no trigger (i.e. effect commands)

[Syst3ms]

Do effect commands even matter in this case?

[Pikachu920]

realistically no, however things like it should still function correctly as there are ways code can run without a trigger

[Syst3ms]

Like what, though ?

[ghost]

There is some other issue with that:
If I have something like this:

permission: gminer.server trigger: slot 0 of player is iron_sword or chorus_fruit set {_item} to slot 0 of player set amount of item {_item} to 1 set {wsystem.waffenitem::%arg 1%} to {_item}

(is adding an item to storage to give it player in a modified way later)

Then later in a function...

set {_waffe} to {wsystem.waffenitem::%{_type}%} set {_serial} to random integer between 1000000 and 9999999 set lore of {_waffe} to "%lore of {_waffe}%||&0SN: %{_serial}%" give {_waffe} to {_player}

= THIS is changing and adding the lore with "SN" more and more to the item. Seems that this linking the slot of the player with the local variables everywhere. Really creepy and not the way it should work, I think.

[ghost]

+++
this is overwriting the global variable {system.waffenitem::%{type}%}, when I set the lore of the local {_waffe}-variable.

[Snow-Pyon]

@GermanMinerDE that's just because of the way slots are implemented, not really related to this issue nor an issue itself. Just like blocks, Skript updates the state of the variable when the object gets an update.

[ghost]

Okay - but how can I unlink such things? I am storing at to an other variable to NOT overwrite the original. If I want to overwrite the original I can do "set lore of slot X..."

But I think I know in which way you mean it.

[bensku]

If you assign a slot to another variable, it is still a slot. That means it will reflect the changes in inventory. I'm not sure if there is any way around this currently. I suppose parsing slot as item type could work...

[bensku]

Might have fixed it but due to #1161 I can't check until maybe tomorrow.

[Pikachu920]

will this not cause an issue if an AsyncEffect is encountered?

[bensku]

Yeah. And delays, too, probably. I need to figure out something better.

Might have to make local variables truly local to trigger. But that is way harder than just clearing them...

[bensku]

Skript's variable system does not understand concept of trigger at all currently. Or concept of scope.

Solving this issue will require large-scale rewrites of the variable backend. Not something that I can do very near in future.

[Syst3ms]

@bensku Sure, you'd need to rewrite stuff a lot inside skript.variables, but wouldn't that just mean changing Event to Trigger in a bunch of places ? I know the difficulty would be changing all of that, but isn't it easy in theory ?

[bensku]

No, because local variable "removal" relies on Java garbage collector. WeakHashMap with events as keys only works because events are not referenced from elsewhere (well, I hope so) after they have been handled. Triggers are persistent until a script is unloaded. Alternative way of clearing local variables would need to be implemented.

[Syst3ms]

Let's just make a Skript GC

[Anoniempje1234]

Try:
broadcast "%{_oops}%"

[Pikachu920]

It would have the same result

[bensku]

It seems I finally got a working fix done. Expect it to be in the next release.