-
-
Notifications
You must be signed in to change notification settings - Fork 373
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
Enhance & Fix Sign Text Expression #5465
Conversation
if (matchedPattern == 1) { | ||
line = (Expression<Number>) exprs[0]; | ||
} else { | ||
new SimpleLiteral<>(parseResult.mark, false); | ||
} |
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.
you call new SimpleLiteral
here but never set line
to this, also a little lost on the change here outside of better readability
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.
Oops.
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 merged this with ternary but LimeGlass disagreed, so I reverted back to this.
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.
alright, then that's all
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.
Something I missed in my once over
Skript.registerExpression(ExprSignText.class, String.class, ExpressionType.PROPERTY, | ||
"[the] line %number% [of %block%]", "[the] (1¦1st|1¦first|2¦2nd|2¦second|3¦3rd|3¦third|4¦4th|4¦fourth) line [of %block%]"); | ||
"[the] lines [of %block%]", | ||
"[the] line %number% [of %block%]", | ||
"[the] (1¦1st|1¦first|2¦2nd|2¦second|3¦3rd|3¦third|4¦4th|4¦fourth) line [of %block%]" | ||
); |
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.
could the last pattern not be (1:(1st|first)|2:(2nd|second)|3:(3rd|third)|4:(4th|fourth))
for the matching?
we no longer need pipe lines and should stay back from those as ParseMark supports colons.
Do double check with the pattern tho but it should work like that
Closing in favour of #5797 |
Description
A broom from Wal-Mart arrived to clean up ExprSignText which also allows for plural lines support!
Target Minecraft Versions: any
Requirements: any
Related Issues: #4576 (lines)