Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix changing to <none> not functional #3940

Merged
merged 3 commits into from
May 4, 2021

Conversation

TPGamesNL
Copy link
Member

Description

Changes EffChange so that setting one value to another, where the second value evaluates to null (<none>), actually deletes the first value instead of not having any effect (I asked on in skript-talk in Discord if this should be the expected behaviour, this was the result).


Target Minecraft Versions: any
Requirements: none
Related Issues: #3049 (partial)

@TPGamesNL TPGamesNL added enhancement Feature request, an issue about something that could be improved, or a PR improving something. 2.6 labels May 1, 2021
@yhdev7935
Copy link

yhdev7935 commented May 3, 2021

I wrote the code using these grammatical characteristics, but after this commit, everything needs to be fixed. That's sad. 😢
I used this grammar when I set the default value.
However, this commit seems to prevent many unexpected errors. ( Especially in code that uses loop frequently. )

command /test [<text>]:
  trigger:
    set {_arg1} to "Hi" # The code length is shorter than "if arg 1 is not set" :D
    set {_arg1} to arg-1

@FranKusmiruk
Copy link
Member

I wrote the code using these grammatical characteristics, but after this commit, everything needs to be fixed. That's sad. 😢
I used this grammar when I set the default value.
However, this commit seems to prevent many unexpected errors. ( Especially in code that uses loop frequently. )

command /test [<text>]:
  trigger:
    set {_arg1} to "Hi" # The code length is shorter than "if arg 1 is not set" :D
    set {_arg1} to arg-1

I am honestly surprised someone managed to take advantage of this, however it is broken behaviour and it should be fixed accordingly.

@TPGamesNL
Copy link
Member Author

@yhdev7935 Also take a look at the default value expression: https://skriptlang.github.io/Skript/expressions.html#ExprDefaultValue

@yhdev7935
Copy link

@TPGamesNL Oh, Thanks. However, the difficulty of debugging will inevitably increase slightly.

@FranKusmiruk FranKusmiruk merged commit 024c2f6 into SkriptLang:master May 4, 2021
@TPGamesNL TPGamesNL deleted the fix/change-to-null branch May 4, 2021 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants