-
Notifications
You must be signed in to change notification settings - Fork 2
1.21.11 #322
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.11 #322
Conversation
Bumps [com.gradleup.shadow](https://github.com/GradleUp/shadow) from 9.2.2 to 9.3.0. - [Release notes](https://github.com/GradleUp/shadow/releases) - [Commits](GradleUp/shadow@9.2.2...9.3.0) --- updated-dependencies: - dependency-name: com.gradleup.shadow dependency-version: 9.3.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [io.lettuce:lettuce-core](https://github.com/redis/lettuce) from 6.8.1.RELEASE to 7.2.1.RELEASE. - [Release notes](https://github.com/redis/lettuce/releases) - [Changelog](https://github.com/redis/lettuce/blob/7.2.1.RELEASE/RELEASE-NOTES.md) - [Commits](redis/lettuce@6.8.1.RELEASE...7.2.1.RELEASE) --- updated-dependencies: - dependency-name: io.lettuce:lettuce-core dependency-version: 7.2.1.RELEASE dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [kapt](https://github.com/JetBrains/kotlin) from 2.2.20 to 2.3.0. - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/master/ChangeLog.md) - [Commits](JetBrains/kotlin@v2.2.20...v2.3.0) --- updated-dependencies: - dependency-name: kapt dependency-version: 2.3.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [jvm](https://github.com/JetBrains/kotlin) from 2.2.20 to 2.3.0. - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/master/ChangeLog.md) - [Commits](JetBrains/kotlin@v2.2.20...v2.3.0) --- updated-dependencies: - dependency-name: jvm dependency-version: 2.3.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
…hadow-9.3.0' into 1.21.11
…tuce-core-7.2.1.RELEASE' into 1.21.11
Summary of ChangesHello @jimchen5209, 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! This pull request primarily focuses on bringing the project up to date with Minecraft 1.21.11. This involves a significant overhaul of the underlying technology stack, including upgrading the Java runtime to version 25, updating the Gradle wrapper, and bumping various Kotlin and third-party library versions. Additionally, it addresses a critical bug in resource pack hash comparison and includes a minor code cleanup for improved robustness. Highlights
Ignored Files
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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. Footnotes
|
There was a problem hiding this 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 primarily updates dependencies and the build environment, including upgrading the Dockerfile's base JRE from Java 21 to Java 25, updating Kotlin and Gradle plugin versions, and bumping several library dependencies like lettuce-core and galaxy-lib. It also updates the Velocity proxy server and the Minecraft protocol version from 1.21.9 to 1.21.11 across ProxyConfig.kt and proxy.json. Minor code adjustments include removing a null-assertion in GalaxyPacket.kt and improving resource pack hash comparison in ResourcePackManager.kt. Review comments raise concerns about the upgrade to Java 25, noting it's an early-access build and recommending stable GA or LTS versions for production, and suggest renaming the ProtocolVersion property in ProxyConfig.kt to protocolVersion to adhere to Kotlin coding conventions, requiring corresponding JSON serialization adjustments.
|
|
||
| data class ProxyConfig( | ||
| val ProtocolVersion: Int = 773 // default Minecraft 1.21.9 | ||
| val ProtocolVersion: Int = 774 // default Minecraft 1.21.11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the Kotlin Coding Conventions, property names should be in camelCase starting with a lowercase letter. ProtocolVersion should be renamed to protocolVersion. Note that this will likely require updating the JSON serialization/deserialization logic (e.g., using an annotation like @JsonProperty("ProtocolVersion")) to match the field name in proxy.json.
| val ProtocolVersion: Int = 774 // default Minecraft 1.21.11 | |
| val protocolVersion: Int = 774 // default Minecraft 1.21.11 |
References
- Kotlin coding conventions state that names of properties and local variables should start with a lower case letter and use camel case. (link)
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughDependency and build-tool upgrades, a Velocity artifact update, a ProtocolVersion default bump (773 → 774), and small null-safety/content-based equality fixes in packet and resource-pack handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/kotlin/one/oktw/galaxy/proxy/resourcepack/ResourcePackManager.kt (1)
15-22: Edge case: empty lists may cause IndexOutOfBoundsException.When both
oldandneware empty lists,updateIndexremains atold.lastIndex(-1), causingold.subList(-1, 0)on Line 25 to throw anIndexOutOfBoundsException. Consider adding a guard condition:if (old.isEmpty() && new.isEmpty()) returnNote: This appears to be a pre-existing issue not introduced by this PR, but worth addressing.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (8)
.github/workflows/build_and_push.yml(1 hunks)Dockerfile(2 hunks)build.gradle.kts(2 hunks)gradle/wrapper/gradle-wrapper.properties(1 hunks)src/main/kotlin/one/oktw/galaxy/proxy/config/model/ProxyConfig.kt(1 hunks)src/main/kotlin/one/oktw/galaxy/proxy/event/GalaxyPacket.kt(1 hunks)src/main/kotlin/one/oktw/galaxy/proxy/resourcepack/ResourcePackManager.kt(1 hunks)src/main/resources/config/proxy.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (11)
Dockerfile (2)
2-2: LGTM! Base image updated to Java 25.The base image update aligns with the JVM target changes in the build configuration.
11-11: Velocity version and checksum verified.The Velocity 3.4.0-SNAPSHOT Build #558 with checksum ef1a852bfae7397e84907837925e7ad21c6312066290edaae401b77f6f423ac3 is the official PaperMC artifact.
build.gradle.kts (3)
5-7: LGTM! Build plugin versions updated.The Kotlin and Shadow plugin versions have been updated appropriately.
37-37: LGTM! JVM target updated to Java 25.The JVM target update aligns with the Dockerfile base image and CI configuration changes.
30-30: No breaking API changes detected for this upgrade.The codebase uses only modern Lettuce APIs that remain compatible across versions 6.8.1 and 7.2.1. The code properly uses
RedisClient.create(),client.connect(),client.connectPubSub(), and the modern sync/async/coroutines command APIs. None of the APIs removed in Lettuce 7.0 (dnsResolver option, reset() method, or deprecated connection methods) are utilized. This upgrade is safe to proceed with.src/main/resources/config/proxy.json (1)
2-2: LGTM! Protocol version updated to 774.The protocol version has been correctly updated to match Minecraft 1.21.11, consistent with the default value change in ProxyConfig.kt.
.github/workflows/build_and_push.yml (1)
27-27: LGTM! CI Java version updated to 25.The CI environment now matches the JVM target in build.gradle.kts and the Dockerfile base image.
src/main/kotlin/one/oktw/galaxy/proxy/resourcepack/ResourcePackManager.kt (1)
18-18: Excellent fix! Array comparison now uses content equality.The change from reference equality (
!=) to content equality (contentEquals) is critical for correct hash comparison. The old code would incorrectly treat byte arrays with identical content as different.src/main/kotlin/one/oktw/galaxy/proxy/event/GalaxyPacket.kt (1)
94-94: LGTM! Removed unnecessary non-null assertion.The non-null assertion (
!!) was unnecessary becausegalaxyis guaranteed to be non-null at this point. Both branches of the preceding if-else block (lines 47-90) ensure thatgalaxyis assigned a non-null value before reaching line 94.src/main/kotlin/one/oktw/galaxy/proxy/config/model/ProxyConfig.kt (1)
4-4: LGTM! Default protocol version updated to 774.The default value now matches the configuration in proxy.json and aligns with Minecraft 1.21.11.
gradle/wrapper/gradle-wrapper.properties (1)
3-3: Gradle version 9.2.1 is valid and stable.Gradle recommends upgrading to version 9.2.1, which was released on November 17, 2025 and is the current Gradle release. No further action needed on the version update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Dockerfile (1)
2-2: Inconsistency with PR objectives: Java version not updated.The PR description states "Java: 21 → 25", but the Dockerfile still uses the Java 21 base image. If the intention is to update to Java 25, this line should be changed to
eclipse-temurin:25-jre-alpine.🔎 Proposed fix if Java 25 update is intended
-FROM eclipse-temurin:21-jre-alpine +FROM eclipse-temurin:25-jre-alpinebuild.gradle.kts (1)
37-37: Inconsistency with PR objectives: JVM target not updated.The PR description states "Java: 21 → 25", but the JVM target remains set to
JVM_21. If the intention is to update to Java 25, this should be changed toJvmTarget.JVM_25. This is consistent with the issue found in the Dockerfile.🔎 Proposed fix if Java 25 update is intended
- jvmTarget = JvmTarget.JVM_21 + jvmTarget = JvmTarget.JVM_25
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Dockerfile(1 hunks)build.gradle.kts(2 hunks)
🔇 Additional comments (3)
Dockerfile (1)
11-11: LGTM!The Velocity artifact update includes proper SHA256 checksum verification, which is a good security practice.
build.gradle.kts (2)
5-7: Test Gradle 9.2.1 compatibility for Kotlin 2.3.0.Kotlin 2.3.0 is fully compatible with Gradle 7.6.3 through 9.0.0. Gradle 9.2.1 exceeds the officially tested range, so test the build as newer versions may result in deprecation warnings and some new Gradle features might not work. Shadow 9.3.0 requires Gradle 9.0 or later, so it is compatible with Gradle 9.2.1.
30-30: Verify timeout configuration and deprecated API usage for lettuce-core 7.x compatibility.Lettuce-core has been upgraded from 6.8.1 to 7.2.1 with several breaking changes. Timeout methods accepting TimeUnit have been removed in favor of methods accepting Duration, and Spring support has been removed (use Spring Data Redis instead). Additionally, deprecated dnsResolver option and reset() method have been removed. Verify that the codebase does not rely on these deprecated APIs, particularly any TimeUnit-based timeout configurations or Spring framework integration.
bill96012
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test passed
Changes
9.2.2->9.3.06.8.1->6.8.22.2.20->2.3.09.1.0->9.2.1f4e1b25->2235da5