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 InventoryItemMoveEvent Implementations (#5462) #6233

Conversation

NotSoDelayed
Copy link
Contributor

@NotSoDelayed NotSoDelayed commented Dec 14, 2023

Description

This PR fixes #5462 where event values were unknowingly removed before the merge, with additions to event-block support and JUnit test.


Target Minecraft Versions: any
Requirements: none
Related Issues: #5462, #6092

@Pikachu920
Copy link
Member

tests 😍

Copy link
Member

@Moderocky Moderocky left a comment

Choose a reason for hiding this comment

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

Good, but some parsing is failing in the test.

@Pikachu920 Pikachu920 added the 2.8 Targeting a 2.8.X version release label Dec 19, 2023
@NotSoDelayed
Copy link
Contributor Author

NotSoDelayed commented Dec 28, 2023

Tests for this PR cannot be proceeded until #6261 and other issues related to delayed tests to be fixed in Skript's JUnit testing system.

@sovdeeth
Copy link
Member

Tests for this PR cannot be proceeded until #6261 and other issues related to delayed tests to be fixed in Skript's JUnit testing system.

If the junit changes can't be made in time for 2.8, consider testing by just creating and calling a InventoryMoveEvent yourself.

@sovdeeth
Copy link
Member

I'd like to have this in 2.8-pre1, @DelayedGaming, so would you be willing to either do (or let me do) the changes to the tests I suggested above as a stopgap until those JUnit fixes are done?

@sovdeeth sovdeeth added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Dec 30, 2023
@sovdeeth sovdeeth added the patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version. label Jan 12, 2024
@sovdeeth sovdeeth changed the base branch from dev/feature to dev/patch January 29, 2024 11:18
@Fusezion Fusezion mentioned this pull request Feb 22, 2024
1 task
@NotSoDelayed
Copy link
Contributor Author

As #6451 points a pattern improvement, I decided to apply it into this PR as both are on the same page.

Copy link
Contributor

@Fusezion Fusezion left a comment

Choose a reason for hiding this comment

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

double check the formatting, hard to really tell when using github

src/main/java/ch/njol/skript/events/EvtItem.java Outdated Show resolved Hide resolved
@APickledWalrus APickledWalrus merged commit 91059e0 into SkriptLang:dev/patch Mar 1, 2024
4 checks passed
@NotSoDelayed NotSoDelayed deleted the enhance/event-initiator-inventory branch March 2, 2024 05:21
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. 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.

None yet

6 participants