Skip to content

update for 1.20(.1)#11

Merged
Fourmisain merged 7 commits into
mainfrom
1.20-wip
Mar 18, 2025
Merged

update for 1.20(.1)#11
Fourmisain merged 7 commits into
mainfrom
1.20-wip

Conversation

@oliwoli
Copy link
Copy Markdown
Contributor

@oliwoli oliwoli commented Mar 16, 2025

  • update loom to 1.10
  • libgui to 8.0.1+1.20
  • fabric api 0.83.0+1.20
  • fabric loader 0.16.10
  • gradle 8.13
  • small fixes for 1.20 (still broken but the game starts at least)

Minecraft 1.20.0 starts now but when you right click on the rulebook item, the game crashes:

Caused by: java.lang.UnsupportedOperationException: [LibGui] Screens cannot be opened in item use methods. Some alternatives include:
        - Using a packet together with LightweightGuiDescription
        - Using an ItemSyncedGuiDescription
Setting the screen in item use methods leads to threading issues and
other potential crashes on both the client and the server.
If you want to disable this check, set the system property libgui.allowItemUse to "true".
        at io.github.cottonmc.cotton.gui.impl.client.ItemUseChecker.checkSetScreen(ItemUseChecker.java:125) ~[libgui-8.0.1+1.20-53e2a69f3f0c7058.jar:?]

I'm not confident enough with java to try to change this, even though it's probably fairly straight forward. If anyone wants to have a go at it @Fourmisain @fiws @Da-Scher go for it :)

- update loom to 1.10
- libgui to 8.0.1+1.20
- fabric api 0.83.0+1.20
- fabric loader 0.16.10
- gradle 8.13
- small fixes for 1.20 (still broken but the game starts at least)
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Hello @oliwoli, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request updates several dependencies and makes small fixes for Minecraft version 1.20. Specifically, it updates Loom to 1.10, libgui to 8.0.1+1.20, Fabric API to 0.83.0+1.20, Fabric Loader to 0.16.10, and Gradle to 8.13. The changes also include minor fixes to get the game to start in 1.20, although there's a known crash when right-clicking the rulebook item due to an issue with LibGui screens being opened in item use methods. The author is seeking assistance from other developers to resolve this LibGui issue.

Highlights

  • Dependency Updates: Updates Loom, libgui, Fabric API, Fabric Loader, and Gradle to their latest versions for Minecraft 1.20 support.
  • Minecraft 1.20 Fixes: Includes small fixes to allow the game to start in Minecraft 1.20, but a crash related to LibGui remains when interacting with the rulebook item.
  • Workflow Updates: Updates the GitHub Actions workflows to use newer versions of actions/checkout and actions/setup-java and actions/upload-artifact.
  • Code Adjustments: Adjusts the TestUtils.java file to use player.getServerWorld() instead of player.world to get the server world instance.
  • GUI Updates: Updates WGradient.java and WSpriteButton.java to use DrawContext instead of MatrixStack for rendering.

Changelog

Click here to see the changelog
  • .github/workflows/build-and-test.yml
    • Updated actions/checkout to v4.
    • Updated actions/setup-java to v4.
    • Updated actions/cache to v4.
    • Updated actions/upload-artifact to v4.
  • .github/workflows/minepkg-publish.yml
    • Updated actions/checkout to v4.
    • Updated actions/setup-java to v4.
  • .minepkg-lock.toml
    • Updated minecraft version to 1.20.
    • Updated fabricLoader version to 0.16.10.
    • Updated mapping to 1.20+build.1.
    • Updated fabric dependency to 0.92.3+1.20.1.mpkg.1.
    • Updated minepkg-companion dependency to 0.8.0.
  • build.gradle
    • Updated fabric-loom version to 1.10-SNAPSHOT.
  • gradle.properties
    • Updated minecraft_version to 1.20.
    • Updated yarn_mappings to 1.20+build.1.
    • Updated loader_version to 0.16.10.
    • Updated fabric_version to 0.83.0+1.20.
    • Updated libgui_version to 8.0.1+1.20.
  • gradle/wrapper/gradle-wrapper.properties
    • Updated distributionUrl to gradle-8.13-bin.zip.
  • minepkg.toml
    • Updated minecraft requirement to ~1.20.0.
  • src/main/java/io/minepkg/testutils/TestUtils.java
    • Replaced player.world with player.getServerWorld() to get the correct server world instance on lines 74, 84, 99 and 140.
  • src/main/java/io/minepkg/testutils/gui/WGradient.java
    • Imported net.minecraft.client.gui.DrawContext.
    • Replaced MatrixStack with DrawContext in the paint method signature on line 26.
    • Used context instead of matrices in ScreenDrawing.coloredRect on line 30.
  • src/main/java/io/minepkg/testutils/gui/WSpriteButton.java
    • Removed import of io.minepkg.testutils.TestUtils and net.minecraft.client.util.math.MatrixStack.
    • Added import of net.minecraft.client.gui.DrawContext.
    • Replaced MatrixStack with DrawContext in the paint method signature on line 31.
    • Used context instead of matrices in ScreenDrawing.coloredRect on lines 38 and 39, and ScreenDrawing.texturedRect on line 45.
  • src/main/resources/fabric.mod.json
    • Updated minecraft dependency to ~1.20.0.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


In realms of code, a quest unfolds,
To mend the world, as version molds.
From Loom's embrace to Fabric's grace,
We chase the bugs, with steady pace.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates several dependencies and makes small fixes for Minecraft 1.20. The update includes changes to loom, libgui, fabric API, fabric loader, and gradle. The pull request also addresses a crash related to opening screens in item use methods, although a complete fix is not yet implemented. The code changes include updates to GitHub Actions workflows, dependency versions in various configuration files, and minor code adjustments in the Java source files.

Summary of Findings

  • Potential crash in RuleBookItem: The pull request description mentions a crash when right-clicking the rulebook item due to Screens cannot be opened in item use methods. While the dependency updates might mitigate this, the underlying issue in RuleBookItem.java needs to be addressed to ensure stability. The author acknowledges this and seeks assistance.
  • Inconsistent Minecraft version: The fabric.mod.json and minepkg.toml files specify different Minecraft versions (~1.20.0 in fabric.mod.json and ~1.20.0 in minepkg.toml). While both are targeting 1.20, it's best to ensure consistency across all configuration files.
  • Redundant Interface Casting: The code casts player.world to ServerWorld multiple times. This is redundant and can be simplified by using player.getServerWorld() directly.

Merge Readiness

The pull request updates dependencies and includes some code fixes. However, the primary issue of the game crashing when right-clicking the rulebook item is not fully resolved. Given the unresolved crash and the potential for threading issues, I recommend that the pull request not be merged until the crash is properly addressed. Additionally, the Minecraft version inconsistency should be resolved. I am unable to approve this pull request, and users should have others review and approve this code before merging.

Comment thread src/main/java/io/minepkg/testutils/TestUtils.java
Comment thread src/main/java/io/minepkg/testutils/TestUtils.java
Comment thread src/main/java/io/minepkg/testutils/TestUtils.java
@Fourmisain
Copy link
Copy Markdown
Contributor

Setting the screen in item use methods leads to threading issues and other potential crashes on both the client and the server.

Huh, can't say I understand the issue.
The wiki says

That's it. No ScreenHandlerType, no packets, just be absolutely certain to only do this on the client. You can't call MinecraftClient.setScreen directly from common methods like Block.onUse, Item.use or Item.useOnBlock - then you need packets or a SyncedGuiDescription.

So no further explanation either.
How do threading issues arise if setScreen is called from the Render thread only when called in Item.use()?

I guess this PSA is mostly meant for synced screens like inventories and such, as the server-side Item.use() could get blocked for some reason, leaving a client-side GUI open without the server having a ScreenHandler open, or vice-versa.

That said, our GUI is completely, 100% client-side and the syncing of server-side stuff either happens automatically (time, weather) or is done manually via packets (weather tick gamerule), so I don't think this error actually applies to us.

Not sure how to use ItemSyncedGuiDescription, but it's simple enough to just send a server->client packet and open the book that way.

Btw, @oliwoli, use gradlew wrapper --gradle-version 8.13 && gradlew wrapper to update the wrapper scripts too.
(The first gradlew wrapper will only update the gradle version in gradle-wrapper.properties because it's still running the old gradle version. The second call will run the new gradle version and will thus also update the scripts.)

If you have gradle (8.13) installed directly, I think it's enough to just do gradle wrapper (note the missing w).

@Fourmisain
Copy link
Copy Markdown
Contributor

Fourmisain commented Mar 16, 2025

Oh, I wanted to push to this branch but I don't have the rights. Pushed it to my fork:
https://github.com/Fourmisain/test-utils/commits/1.20-wip/

Works in singleplayer and should work on a proper server + client setup too, but needs to be tested still.

@Fourmisain
Copy link
Copy Markdown
Contributor

Also I don't think I ever noticed but the skylight preview seems a little wrong:
1
2
3

@oliwoli
Copy link
Copy Markdown
Contributor Author

oliwoli commented Mar 16, 2025

Btw, @oliwoli, use gradlew wrapper --gradle-version 8.13 && gradlew wrapper to update the wrapper scripts too. (The first gradlew wrapper will only update the gradle version in gradle-wrapper.properties because it's still running the old gradle version. The second call will run the new gradle version and will thus also update the scripts.)

If you have gradle (8.13) installed directly, I think it's enough to just do gradle wrapper (note the missing w).

Oh, I didn't know, thanks! I actually just edited the url by hand to update the version haha :D

Oh, I wanted to push to this branch but I don't have the rights. Pushed it to my fork:

Can't give you access, you'll have to wait for fiws :)

@oliwoli
Copy link
Copy Markdown
Contributor Author

oliwoli commented Mar 16, 2025

Also I don't think I ever noticed but the skylight preview seems a little wrong: 1 2 3

You mean that setting the slider all the way to the right isn't 100% night? Yeah I noticed that too. I think we discussed it in the past but never bothered to fix it 🫣

@Fourmisain
Copy link
Copy Markdown
Contributor

You mean that setting the slider all the way to the right isn't 100% night? Yeah I noticed that too. I think we discussed it in the past but never bothered to fix it 🫣

Yesn't, it's more like when the slider is at the beginning and at the end the same sun position is the same (it's basically the same time), so it should have the same brightness. At highnoon, when the sun is at it's peak, it should be the brightest.

@Fourmisain
Copy link
Copy Markdown
Contributor

Fourmisain commented Mar 18, 2025

GitHub is weird, I didn't even see the organization invite until I opened my mail.
(And it again doesn't show the added commits here.)

@oliwoli
Copy link
Copy Markdown
Contributor Author

oliwoli commented Mar 18, 2025

(And it again doesn't show the added commits here.)

It does for me but in the reverse order they are usually sorted by.

@Fourmisain
Copy link
Copy Markdown
Contributor

Ohh, it's up there! Because the commits are 2 days old! That's totally not confusing at all!

ohh

@Fourmisain
Copy link
Copy Markdown
Contributor

Fourmisain commented Mar 18, 2025

Btw, are we sure this doesn't work on 1.21?
Also just noticed the gradle build needs an update, stuff like targetCompatibility, archivesBaseName doesn't exist anymore. (Can't just bump gradle without adjusting the build.)

@Fourmisain
Copy link
Copy Markdown
Contributor

Fourmisain commented Mar 18, 2025

Btw, are we sure this doesn't work on 1.21?

Ah, yeah, because it bundles LibGui it should only work on 1.20(.1).

Edit: Yep, crashes on 1.20.2

@oliwoli
Copy link
Copy Markdown
Contributor Author

oliwoli commented Mar 18, 2025

Btw, are we sure this doesn't work on 1.21?

Ah, yeah, because it bundles LibGui it should only work on 1.20(.1).

I'll test it with different LibGui versions later. Hopefully that's all we gotta do to make it run :)

@Fourmisain
Copy link
Copy Markdown
Contributor

Updated the version range to just 1.20(.1), tested it on a 1.20.1 server as well, works just fine.
Should be good to merge now!

@Fourmisain Fourmisain changed the title update stuff: update for 1.20(.1) Mar 18, 2025
@Fourmisain
Copy link
Copy Markdown
Contributor

Looking at the version history, I guess we should bump to 0.6.0?

@oliwoli
Copy link
Copy Markdown
Contributor Author

oliwoli commented Mar 18, 2025

Looking at the version history, I guess we should bump to 0.6.0?

Sure, if everything works on 1.20.0 and 1.20.1

@Fourmisain
Copy link
Copy Markdown
Contributor

I forgot the minepkg.toml, hold on.

@Fourmisain
Copy link
Copy Markdown
Contributor

Fourmisain commented Mar 18, 2025

Hm... I guess it's easier better to bump after merging this PR?
Because there'll be a merge commit and it's cleaner to have a tag for the specific version bump commit like you said last time.

@Fourmisain Fourmisain marked this pull request as ready for review March 18, 2025 16:31
@Fourmisain Fourmisain merged commit 1f60c5a into main Mar 18, 2025
@Fourmisain Fourmisain deleted the 1.20-wip branch March 18, 2025 16:34
@Fourmisain
Copy link
Copy Markdown
Contributor

OK, only needs to be published to minepkg now.

@oliwoli
Copy link
Copy Markdown
Contributor Author

oliwoli commented Mar 18, 2025

OK, only needs to be published to minepkg now.

seems to have worked automatically via the workflow/GitHub action☺️👍

@Fourmisain
Copy link
Copy Markdown
Contributor

seems to have worked automatically via the workflow/GitHub action☺️👍

Ah, so that's how that works, nice!

@Fourmisain
Copy link
Copy Markdown
Contributor

Ah, it was actually because of the release I (manually) created, the publish tasks looks for that.
I thought it would go by tag or need to be manually executed or something.

@oliwoli
Copy link
Copy Markdown
Contributor Author

oliwoli commented Mar 18, 2025

Ah, it was actually because of the release I (manually) created, the publish tasks looks for that. I thought it would go by tag or need to be manually executed or something.

Yep, I know :) Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants