Skip to content

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

Merged
merged 5 commits into from
Feb 19, 2021
Merged

Conversation

Mwexim
Copy link
Owner

@Mwexim Mwexim commented Jan 2, 2021

This pull request adds something that was not yet added to vanilla Skript: the switch control flow.

  • Added SecSwitch which only allows MatchingElement syntax classes (SecCase and EffCase)
  • SecCase to run multiple lines of code and EffCase to run just one effect.
  • There are some differences with the Java switch statement:
    • Instead of running all effects until a break is reached, it will run all effects inside matching cases. This means that each case is executed separately.
    • If no match was found in any case, all (yes, there can be multiple) default statements are executed.
  • You can use 'or' lists to match multiple literals at the same time
  • Two identical case statements will not result in errors.

I have a few discussion questions ready because I know some people will not agree with the implementation:

  • Is this necessary? The syntax isn't English-like! Yes it is, and it is very useful in my opinion. I hate the fact that script never added something like this.
  • I don't like EffCase. It is quite ugly... If enough people agree with this, I will revert the second commit. I added this as extra utility, but I can imagine this is a step too far in creating non-English syntax.
  • I think allowing identical switch statements/multiple default ones should be prohibited. If enough people agree with this, I will change this as well.
  • Default cases should always be placed at the end. Again, if enough people find this necessary, I'll change it.

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.

@Mwexim
Copy link
Owner Author

Mwexim commented Jan 28, 2021

Latest commit added certain features discussed in here:

  • Added by default and when patterns.
  • Removed match and similar patterns, because they did not feel right here.
  • Added some more strict rules:
    • Only one default section/effect for each switch
    • Default section (if used) needs to be placed as the last option.
  • Improved error messages
  • Removed the odd MatchingElement inner-class.

@Mwexim Mwexim requested a review from Syst3ms January 28, 2021 18:15
* @author Mwexim
* @see SecSwitch
*/
public class EffCase extends Effect {
Copy link
Contributor

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.

Copy link
Contributor

@Olyno Olyno Feb 6, 2021

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%"
Copy link
Contributor

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).

Copy link
Contributor

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

Copy link
Contributor

@WeeskyBDW WeeskyBDW Feb 6, 2021

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!"

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

Copy link
Contributor

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.

@Mwexim Mwexim merged commit 36c850b into master Feb 19, 2021
@Mwexim Mwexim deleted the feature/switch-syntax branch February 19, 2021 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants