-
Notifications
You must be signed in to change notification settings - Fork 14
feature: SecSwitch and SecCase/EffCase #92
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
Conversation
Latest commit added certain features discussed in here:
|
* @author Mwexim | ||
* @see SecSwitch | ||
*/ | ||
public class EffCase extends Effect { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm against inlined effects like these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, we should make them as a section, not a inline effect.
Edit: Well the code section already exists here, and a one line effect can be a nice-to-have too. I would like to have both ways.
static { | ||
Parser.getMainRegistration().addSection( | ||
SecSwitch.class, | ||
"(switch|for) %object%" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, following the discussion, it would be nice to find a more englishy pattern that would go along with "when" ("for" is bound to confuse people btw).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of "when" for the section syntax, we could use with
as
with {_role}:
when "MEMBER":
send "Welcome back!"
when "ADMIN":
send "Nice to see you again admin!"
otherwise:
send "Welcome new player!"
Plus, if {_role} is empty, the section should run anyway, but execute directly the otherwise
section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of "when" for the section syntax, we could use
with
aswith {_role}: when "MEMBER": send "Welcome back!" when "ADMIN": send "Nice to see you again admin!" otherwise: send "Welcome new player!"
Hum imo it can confused, because with is frequently used to speak of a thing used for (idk if i'm clear). I suggest loop cases:
Plus, if {_role} is empty, the section should run anyway, but execute directly the
otherwise
section
I agree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Loop
is for loop values, it's not the same than a switch
. So we need another keyword for that.
This pull request adds something that was not yet added to vanilla Skript: the switch control flow.
MatchingElement
syntax classes (SecCase
andEffCase
)SecCase
to run multiple lines of code and EffCase to run just one effect.I have a few discussion questions ready because I know some people will not agree with the implementation:
Furthermore, this fixes some problems with restrictions in the ParserState and with Effects. There was also a small fix for the logger while parsing literal lists.
Feedback is always welcome.