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

Allow look-behind for headless effect sections during init. #6556

Merged
merged 2 commits into from
May 8, 2024

Conversation

Moderocky
Copy link
Member

Description

When a Section inits, it is given a list of the preceding trigger items.
This is how an else: block searches for the if..: it belongs to.
image

This list basically contains information about the lines before a syntax.

However, effect sections do not get given this information unless they are heading a section (i.e. if they're used as inline effects rather than sections with a :).
This means that

spawn a zombie:
    # blah

gets the list of trigger items, whereas

spawn a zombie

does not, despite both being the same syntax.

Presumably, this was historically done when EffectSections were made to avoid changing the signature of the parse method they use.

What I changed

This change gives the trigger items to effect sections whether-or-not they are starting a section. The SectionNode will still be null, of course, since they don't have a section, but they are now able to look behind at the previous syntax elements.

This change is mainly targeted at providing API compatibility for future (or addon) effect sections and should have no effect on existing syntax.

Does this break anything?

It shouldn't do, since I duplicated the method with its original signature, in case something exists that uses it directly.

I don't expect this to be a breaking change for any addon, since it is inconceivable that syntax would be conditionally reliant on the triggerItems parameter being null without also being dependent on, for example, the isSection method or the SectionNode parameter.


Target Minecraft Versions: any
Requirements: none
Related Issues: none

@sovdeeth sovdeeth added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Apr 12, 2024
@sovdeeth sovdeeth added feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. 2.9 Targeting a 2.9.X version release labels May 6, 2024
@UnderscoreTud
Copy link
Member

Would it be better to provide this info in a parser instance, so you can get the list of previous items from anywhere during initialization?

@Moderocky
Copy link
Member Author

Would it be better to provide this info in a parser instance, so you can get the list of previous items from anywhere during initialization?

Probably yes but it's out of scope for this, which is just a parity change for some unexpected behaviour. A larger-scale PR could revamp this system entirely into a better and safer state.

@Moderocky Moderocky merged commit fcb92e8 into SkriptLang:dev/feature May 8, 2024
5 checks passed
@Moderocky Moderocky deleted the lookbehind-in-init branch May 8, 2024 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.9 Targeting a 2.9.X version release enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants