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

Improve UnparsedLiteral Handling #6360

Merged
merged 8 commits into from Feb 1, 2024

Conversation

APickledWalrus
Copy link
Member

Description

This is an experimental fix for some issues with UnparsedLiterals that have gotten more exposure due to the changes in Arithmetic parsing. One now problematic expression is the following:
index of event-slot - 9
This would typically be interpreted (and expected to be interpreted as) (index of event-slot) - (9). Skript is correctly identifying this as ExprArithmetic, but the left and right expressions are wrong. Skript will move from left to right trying to determine what group of characters makes up the expression. When Skript gets to the following, things go wrong: (index of event)-(slot - 9) Skript gets to this point and TypePatternElement does not continue as index of event and slot - 9 are both valid UnparsedLiterals. ExprArithmetic then fails due to encountering UnparsedLiterals it cannot convert.

This PR addresses this issue by having TypePatternElement continue matching in the event it gets a MatchResult containing UnparsedLiterals that cannot be converted by regular means (classinfo parsing). It saves a backup of the first valid MatchResult and falls back to that if further searching yields nothing valid.

This solution may need tweaking if further issues arise.


Target Minecraft Versions: any
Requirements: none
Related Issues:

@APickledWalrus APickledWalrus added bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. needs testing Needs testing to determine current status or issue validity, or for WIP feature pulls. labels Jan 22, 2024
@APickledWalrus APickledWalrus changed the base branch from master to dev/patch January 23, 2024 16:49
Significantly improves the conversion of UnparsedLiterals and return type resolution
@APickledWalrus
Copy link
Member Author

I've updated this PR to now include some Arithmetic-specific changes to hopefully improve some of the incorrect parsing/false errors that have been reported. Specifically, Arithmetic attempts to convert UnparsedLiterals to more specific types (from what information it can gather). Return type resolution has also been improved to avoid Object if possible

Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice if we could also improve the error messages a bit but seems like a solid fix!

	loop 9 times:
		loop 9 times:
			set {_num} to loop-value - 1

image

@APickledWalrus APickledWalrus added 2.8 Targeting a 2.8.X version release and removed needs testing Needs testing to determine current status or issue validity, or for WIP feature pulls. labels Feb 1, 2024
Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com>
@APickledWalrus APickledWalrus merged commit 6be9375 into dev/patch Feb 1, 2024
5 checks passed
@APickledWalrus APickledWalrus deleted the fix/unparsed-literal-issues branch February 1, 2024 23:09
ShaneBeee pushed a commit to ShaneBeee/Skript that referenced this pull request Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.8 Targeting a 2.8.X version release bug An issue that needs to be fixed. Alternatively, a PR fixing an issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants