Skip to content

Make event value registration check more lenient#8530

Merged
sovdeeth merged 7 commits intodev/featurefrom
feature/event-value-registration
Apr 12, 2026
Merged

Make event value registration check more lenient#8530
sovdeeth merged 7 commits intodev/featurefrom
feature/event-value-registration

Conversation

@UnderscoreTud
Copy link
Copy Markdown
Member

Problem

When registering an event value, the registry doesn't take into account any special validation when checking if the event value is already registered. This falsely claims two event values with different validation as the same

Solution

Include the input/event validation in the registration check

Testing Completed

Supporting Information


Completes: none
Related: none
AI assistance: none

@UnderscoreTud UnderscoreTud requested a review from a team as a code owner April 8, 2026 01:12
@UnderscoreTud UnderscoreTud requested review from Efnilite and Pesekjak and removed request for a team April 8, 2026 01:12
@UnderscoreTud UnderscoreTud added bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. 2.15 Targeting a 2.15.X version release. labels Apr 8, 2026
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label Apr 8, 2026
@UnderscoreTud UnderscoreTud moved this to In Review in 2.15 Releases Apr 8, 2026
@github-project-automation github-project-automation bot moved this from In Review to Awaiting Merge in 2.15 Releases Apr 8, 2026
@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 Apr 8, 2026
Copy link
Copy Markdown
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

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

Couple thoughts on the list changes (moving in a good direction so far).

Also, is there a reason for the pure contracts? As in, is there a reason to prevent impure implementations?

@github-project-automation github-project-automation bot moved this from Awaiting Merge to In Review in 2.15 Releases Apr 9, 2026
@UnderscoreTud
Copy link
Copy Markdown
Member Author

Also, is there a reason for the pure contracts? As in, is there a reason to prevent impure implementations?

Helps the IDE make accurate analysis on the code. And there is no reason to prevent impure implementations because it's a sealed interface so we have full control over its logic

Copy link
Copy Markdown
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

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

good work

@github-project-automation github-project-automation bot moved this from In Review to Awaiting Merge in 2.15 Releases Apr 10, 2026
@github-project-automation github-project-automation bot moved this from Awaiting Merge to In Review in 2.15 Releases Apr 10, 2026
@github-project-automation github-project-automation bot moved this from In Review to Awaiting Merge in 2.15 Releases Apr 12, 2026
@sovdeeth sovdeeth merged commit ce75dc9 into dev/feature Apr 12, 2026
14 checks passed
@github-project-automation github-project-automation bot moved this from Awaiting Merge to Done - Awaiting Release in 2.15 Releases Apr 12, 2026
@skriptlang-automation skriptlang-automation bot added completed The issue has been fully resolved and the change will be in the next Skript update. and removed feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. labels Apr 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.15 Targeting a 2.15.X version release. 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 - Awaiting Release

Development

Successfully merging this pull request may close these issues.

4 participants