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

1.21 Support #6798

Merged
merged 7 commits into from
Jul 1, 2024
Merged

1.21 Support #6798

merged 7 commits into from
Jul 1, 2024

Conversation

APickledWalrus
Copy link
Member

Description

This PR intends to add support for Minecraft 1.21.

I have added a new testing environment for Paper 1.21. Skript compiled without issue (at the time of writing this).

However, without this PR, Skript fails to load on Paper 1.21 (and presumably any further new releases) due to ItemStack changes. ItemStacks can no longer represent block-only materials (that is, materials such that !Material#isItem). This is only more of a push for us to make strong changes regarding the ItemType/ItemStack/BlockData trio, but an ideal fix would likely require a significant time investment to completely rework the alias/ItemType system.

Fortunately, I have managed to get Skript loading and tests passing without too many changes. One significant change is that ItemStack#getRandom is now nullable. There is no way to avoid this. There are now likely instances of unsafe getRandom usage that need fixed (I have not taken the time to resolve this yet). Internally, it is now the case that an ItemData may not have an ItemStack. This item stack was likely unused in many cases as these datas represented blocks. With all of these changes, this PR needs extensive testing to determine how


Target Minecraft Versions: 1.21+, though these ItemType/ItemData changes apply to all versions (which we may want to discuss)
Requirements: none
Related Issues:

@APickledWalrus APickledWalrus added bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. needs testing Needs testing to determine current status or issue validity, or for WIP feature pulls. labels Jun 18, 2024
Copy link
Contributor

@ShaneBeee ShaneBeee left a comment

Choose a reason for hiding this comment

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

I know my approval doesn't count, but I'm throwing my 2 cents in.
I've been using this PR since you put it up, and I've yet to see any issues.

While my testing may not have been extensive, I have done a fair bit of fiddling with items/blocks and I haven't noticed anything weird nor have I had any errors.

@PetyXbron
Copy link

Yeah! This works perfectly for me. Great work! Big up

This was referenced Jun 22, 2024
@csd4ni3l
Copy link

Works perfectly for me aswell, please merge it, so auto update works instead of using a custom version

@Moderocky
Copy link
Member

Works perfectly for me aswell, please merge it, so auto update works instead of using a custom version

The next public beta is scheduled for the 1st of July (in 3 days) where you'll be able to download it from the GitHub. There'll be about 15 days for people to review and give feedback before the next stable version is released after that. You can find more information in our release document and readme :)

@sovdeeth sovdeeth added the 2.9 Targeting a 2.9.X version release label Jun 28, 2024
@sovdeeth
Copy link
Member

related? #6830

@APickledWalrus
Copy link
Member Author

APickledWalrus commented Jun 28, 2024

Likely. There are probably mainly places that do not properly check ItemMeta/Stack nullability

@itfoldy
Copy link

itfoldy commented Jun 28, 2024

Can any of you build this version and link it as a .jar, so we don't have to do it ourselves, thank you

@APickledWalrus
Copy link
Member Author

Can any of you build this version and link it as a .jar, so we don't have to do it ourselves, thank you

This is automatically done 🙂 You can download the nightly build artifact from here: https://github.com/SkriptLang/Skript/actions/runs/9702144977

@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 Jun 30, 2024
@sovdeeth
Copy link
Member

sovdeeth commented Jul 1, 2024

The exp orb merge regression test is failing in a novel way, which seems unrelated to the changes here. Perhaps Paper 1.21 doesn't merge xp as aggressively?

@sovdeeth sovdeeth merged commit 585e8d1 into dev/patch Jul 1, 2024
5 checks passed
@APickledWalrus APickledWalrus deleted the patch/1.21-support branch July 1, 2024 16:19
@TheBentoBox
Copy link
Member

FYI @sovdeeth regarding this:

The exp orb merge regression test is failing in a novel way, which seems unrelated to the changes here. Perhaps Paper 1.21 doesn't merge xp as aggressively?

The defaults were reduced in a recent Paper commit here.

I've used this branch in my 1.21 testing with no issues so far -- thanks/great work team!

@sovdeeth
Copy link
Member

sovdeeth commented Jul 1, 2024

FYI @sovdeeth regarding this:

The exp orb merge regression test is failing in a novel way, which seems unrelated to the changes here. Perhaps Paper 1.21 doesn't merge xp as aggressively?

The defaults were reduced in a recent Paper commit here.

I've used this branch in my 1.21 testing with no issues so far -- thanks/great work team!

yeah, we (I) couldn't figure out how to edit the configs in the test servers so i just disabled it for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.9 Targeting a 2.9.X version release bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. needs testing Needs testing to determine current status or issue validity, or for WIP feature pulls. 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.

8 participants