Skip to content

Conversation

@APickledWalrus
Copy link
Member

Problem

Working with type properties for #8322, I encountered issues when setting the name of something stored in a variable (unknown/object type). Specifically, PropertyBaseCondition determines that the valid change types are String and Component (some names are strings, others are components). At runtime, with the type of the stored value known, it is able to perform the change. However, if the runtime accepted change types differ from those provided at parse time, there is a silent failure. Additionally, the existing logic for verifying correct types is incorrect. It assumes, for arrays, that delta will be correctly typed. However, unlike getArray or getAll, there are no guarantees about the type of delta arrays.

Another issue exists with the implementation of PropCondContains, which fails to correctly convert inputs at runtime.

Solution

I have reworked the change logic in PropertyBaseExpression to verify each individual object within delta against a flattened list of allowed change types. If any values are unable to be verified, there is an additional stage that now converts delta values. If any values fail to convert, the change operation is abandoned.

PropCondContains already has logic for converting the needles expression. It works as expected during parse time. However, it was forgotten to update the field to instead store the converted needles. Thus, there were silent failures at runtime.

Testing Completed

I have confirmed that the failing tests in #8322 now pass. Additionally, I have added a regression test for the contains issue.

Supporting Information

n/a


Completes: none
Related: none
AI assistance: none

@APickledWalrus APickledWalrus requested review from a team as code owners January 6, 2026 16:26
@APickledWalrus APickledWalrus added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Jan 6, 2026
@APickledWalrus APickledWalrus requested review from Absolutionism and removed request for a team January 6, 2026 16:26
@APickledWalrus APickledWalrus added the 2.14 Targeting a 2.14.X version release. label Jan 6, 2026
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label Jan 6, 2026
@APickledWalrus APickledWalrus moved this to In Review in 2.14 Releases Jan 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.14 Targeting a 2.14.X version release. bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. needs reviews A PR that needs additional reviews

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

3 participants