Skript
Skript copied to clipboard
Introduces toggle effect & expression
Description
This pull request introduces the toggle effect and expression.
set {_var} to true
toggle {_var} # {_var} is now false
set {_var} to inverse of {_var} # {_var} is now true
Target Minecraft Versions: / Requirements: / Related Issues: /
I have the same question I had with Delta’s PR initially: why would you add a new ChangeMode value? Toggling a boolean is essentially equal to setting the boolean to its opposite value.
If anyone can give me one type that could use toggling except booleans, then go ahead, but in my opinion this should just be added as a separate effect and use the set-operation to perform the action.
why would you add a new ChangeMode value?
It's more intuitive and makes more sense than toggle the value manually. For example:
set flight mode to false # Bruh
toggle flight mode # This is good
Toggling a boolean is essentially equal to setting the boolean to its opposite value
Yes it is, but what's the most intuitive for users? Having a boolean "state" expression, or use a simple toggle?
in my opinion this should just be added as a separate effect and use the set-operation to perform the action
I did that in a first time, but try to reuse an effect in addons, this is just impossible or really tricky/hard to do. What we would like is a simple api for addons developers that anyone could use it easily. Adding a mode makes things easier than adding an effect, and trying to use this effect. I would like add too a changer is already an effect. Wouldn't be better to group up all changer effects at the same place? 😃
You're understanding me wrong. What I'm saying is that the internal structure of this feature does not make sense to me.
- You're adding a new ChangeMode while toggling is essentially setting the value to its opposite:
Ex.
toggle flying stateThis should not be handled as 'ChangeMode.TOGGLE' but rather as 'ChangeMode.SET'. - Only booleans can be toggled, unless someone can give me an example where another object could be toggled. The pattern should become
toggle %booleans%, which essentially means that whenever a boolean expression can be set to something, it can also be set to its opposite, which is essentially what toggling means.
The pattern should become toggle %booleans%
Yeah, but here is the problem: how do you manage that with addons?
The pattern should become toggle %booleans%
Yeah, but here is the problem: how do you manage that with addons?
Addons that register boolean expressions can just accept SET as a valid change mode?
Addons that register boolean expressions can just accept SET as a valid change mode?
Hey, I don't really get what you mean, why shouldn't TOGGLE be a mode?
also do you mean the TOGGLE should be like set {variable} to toggle {variable} or what?
Addons that register boolean expressions can just accept SET as a valid change mode?
Hey, I don't really get what you mean, why shouldn't TOGGLE be a mode? also do you mean the TOGGLE should be like
set {variable} to toggle {variable}or what?
Not really. I am advocating for the removal of this ChangeMode because toggling equals setting a value to its opposite. This means that all boolean expressions that accept 'set' as a valid ChangeMode, can be toggled, since, again, toggling equals setting a value to its opposite.
I think both ways are useful as a GhangeMode and as an expression.
because toggling equals setting a value to its opposite.
I don't think this is correct, it is technically setting to it's opposite but to do this manually you will need a work-around and it will be longer that just toggle ... or toggled ... which makes it much easier if you use it a lot.
I think both ways are useful as a GhangeMode and as an expression.
because toggling equals setting a value to its opposite.
I don't think this is correct, it is technically setting to it's opposite but to do this manually you will need a work-around and it will be longer that just
toggle ...ortoggled ...which makes it much easier if you use it a lot.
You are not understanding me correctly but at this point I think it doesn’t matter anymore.
You are not understanding me correctly but at this point I think it doesn’t matter anymore.
I would like to understand your opinion of course, let me know more. It does matter :)
My point is that you can toggle using current SET mode like this
set flight state of player to false if flight state of player is true else true
# OR
if flight state of player is true:
set flight state of player to false
else:
set flight state of player to true
Which works perfectly but adding
toggle flight state of player
send "Flight state has been set to %toggle flight state of player%"
# OR
set flight state toggled {Flight::%uuid of player%}
Makes it easier in some cases if not many, Am I correct?
@Mwexim's point is that toggle flight state of player does not need a new ChangeMode, but can be done with using get methods and the SET ChangeMode
@Mwexim's point is that
toggle flight state of playerdoes not need a new ChangeMode, but can be done with using get methods and the SET ChangeMode
But with that it won't be possible to do toggle flight state of player because toggle is GhangeMode here right?
But with that it won't be possible to do
toggle flight state of playerbecausetoggleis ChangeMode here right?
Yes that would be possible, TOGGLE is not a ChangeMode in that code: https://pastebin.com/mEvXw429
Yes that would be possible, TOGGLE is not a ChangeMode in that code: https://pastebin.com/mEvXw429
If that works well I think it is a better idea, because it will be easier and no need to add TOGGLE ChangeMode to all boolean type expressions manually.
expression.change(e, new Boolean[] {!b}, ChangeMode.SET);
Oh okay, that's what I didn't understand, how we could do that without a mode. It seems to be a good way to do it. I'm gonna probably change for that (and adding a new expression if the community wants to)
This pull request is not ready to be review due to some changes to do. I convert it to draft during this time.
Hi everyone 👋🏻 Time to finish this pull request after months of inactivity. Hopefully this time is the right one!
Testing scripts are failing @Olyno
Any updates on this nice PR? :)
Tests are failing due to
Failed: change modes (on 4 environments) {_list::} should be set to false and {_block} (on paper-1.17.1) {_list::} should be set to false and {_block} (on paper-1.18.2) {_list::} should be set to false and {_block} (on paper-1.19.3) {_list::} should be set to false and {_block} (on paper-1.19.4)
Indeed, I fix that in the week
Ready for review @AyhamAl-Ali
Description example should adapt the new syntax change inverse|opposite
I didn't understand your comment about the description, could you please explain it again?
I didn't understand your comment about the description, could you please explain it again?
In this PR's description, you wrote this example
set {_var} to toggled {_var} # {_var} is now true
which should now be
set {_var} to inverse of {_var} # {_var} is now true
Due to the change in syntax in this PR
Description is updated 👍🏻
i think turn %objects% %boolean% should be included too
i think
turn %objects% %boolean%should be included too
Putting two inputs next to each other will create a parsing nightmare.
i think
turn %objects% %boolean%should be included tooPutting two inputs next to each other will create a parsing nightmare.
The syntax is also not the most sounding. turn player's flight mode yes doesn't make any sense