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

Fix the REMOVE changer of variables #7163

Merged
merged 6 commits into from
Dec 15, 2024
Merged

Conversation

UnderscoreTud
Copy link
Member

@UnderscoreTud UnderscoreTud commented Oct 24, 2024

Description

Currently, removing elements from a list will only remove the first instance found in that list.

set {_list::*} to 1, 2, 1, 3
remove 1 and 1 from {_list::*}
broadcast {_list::*} # 2, 1, 3

This PR aims to fix that issue


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

@UnderscoreTud UnderscoreTud added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Oct 24, 2024
@Efnilite

This comment has been minimized.

Copy link
Contributor

@Asleeepp Asleeepp left a comment

Choose a reason for hiding this comment

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

heres my very important approval

@UnderscoreTud UnderscoreTud added the patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version. label Oct 28, 2024
@UnderscoreTud UnderscoreTud added the 2.10 Targeting a 2.10.X version release label Nov 9, 2024
@sovdeeth sovdeeth merged commit aa71764 into dev/patch Dec 15, 2024
7 checks passed
Moderocky added a commit that referenced this pull request Dec 30, 2024
* Fix the REMOVE changer of variables (#7163)

* Fix the REMOVE changer of variables

* Fix wording

* Fix tests

* Update Variable.java

---------

Co-authored-by: sovdee <[email protected]>

* Fix Improperly Typed Array in ExprXOf (#7268)

* Add boat data check to prevent error. (#7301)

* Fix BlockIgniteEvent  (#7252)

Update BukkitEventValues.java

Co-authored-by: Moderocky <[email protected]>

* Fix expression conversion (#7165)

* Fix expression conversion

* Extract duplicate code into a separate helper method

* improve conversion strategy

* Add .sk to test file

* Simplify conversion usage

We need to use conversion whenever there are multiple return types. If the expression does not accept our supertype, then we can attempt to convert it, which will already handle safety checks for multiple return types

* SimpleExpression: fix possible return type conversion

This fixes SimpleExpression not converting  possible return types that are not contained in the desired types array. For example, if an expression can return a Number or a String, and we want an Expression that is a Number or an World, it will now include converters for String->Number and String->World

* Use safety checks of ConvertedExpression

* Remove incorrect converter remake

* Move logic from SimpleExpression to ConvertedExpression

---------

Co-authored-by: APickledWalrus <[email protected]>
Co-authored-by: Efnilite <[email protected]>
Co-authored-by: Moderocky <[email protected]>

---------

Co-authored-by: _tud <[email protected]>
Co-authored-by: sovdee <[email protected]>
Co-authored-by: Patrick Miller <[email protected]>
Co-authored-by: kyoshuske <[email protected]>
Co-authored-by: Efnilite <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.10 Targeting a 2.10.X version release bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants