Skip to content

Conversation

@APickledWalrus
Copy link
Member

Problem

Currently, ExprArithmetic does not properly consider Converters (Converted Arithmetic Infos) when determining return types along with UnparsedLiteral resolutions. This led to some unexpected test failures when working on #7892

Solution

I added two new methods, lookupLeftOperations and lookupRightOperations for performing OperationInfo lookups (with converters) when only partial information is available. I used lookup as that method seems to be used when Converters are considered, whereas get is for exact looksup.

Testing Completed

I have added a test case in ExprArithmetic.sk that tests the behavior against Experience. Here's what goes wrong currently:

When parsing for the line starts, the Operation information is Experience + UnparsedLiteral (Object). When searching for Experience + <anything> operations, Skript finds nothing, as Converters are not attempted. However, it should find Number + Number as Experience -> Number is a registered Converter. Thus, by checking conversions, this line now parses successfully.

Supporting Information


Completes: none
Related: none

@APickledWalrus APickledWalrus requested review from a team and UnderscoreTud as code owners June 27, 2025 20:01
@APickledWalrus APickledWalrus requested review from erenkarakal and removed request for a team June 27, 2025 20:01
@APickledWalrus APickledWalrus added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Jun 27, 2025
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label Jun 27, 2025
@APickledWalrus APickledWalrus moved this to In Review in 2.12 Releases Jun 27, 2025
@skriptlang-automation skriptlang-automation bot added feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. and removed needs reviews A PR that needs additional reviews labels Jun 28, 2025
@github-project-automation github-project-automation bot moved this from In Review to Awaiting Merge in 2.12 Releases Jun 28, 2025
@APickledWalrus APickledWalrus merged commit f599007 into SkriptLang:dev/feature Jun 30, 2025
5 checks passed
@skriptlang-automation skriptlang-automation bot added the completed The issue has been fully resolved and the change will be in the next Skript update. label Jun 30, 2025
@github-project-automation github-project-automation bot moved this from Awaiting Merge to Done in 2.12 Releases Jun 30, 2025
@APickledWalrus APickledWalrus deleted the patch/arithmetic-conversions branch June 30, 2025 01:47
@skriptlang-automation skriptlang-automation bot removed the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Jun 30, 2025
Burbulinis pushed a commit to Burbulinis/Skript that referenced this pull request Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. completed The issue has been fully resolved and the change will be in the next Skript update.

Projects

Status: Done - Released

Development

Successfully merging this pull request may close these issues.

4 participants