Dgt 2.71 and mc 26.1#605
Conversation
Job Summary for GradleBuild :: build
|
There was a problem hiding this comment.
Pull request overview
This PR updates the DeftuGradleTools (DGT) version to 2.72.0 and adds support for Minecraft 26.1-fabric. It also updates various dependencies and adjusts version handling logic to accommodate the new drop version format (26.1).
Changes:
- Updated Kotlin to 2.3.0, DGT to 2.72.0, Gradle wrapper to 9.2.1, and other dependencies (textile, omnicore)
- Added Minecraft 26.1-fabric support with new version handling for "drop" versions
- Updated preprocessor conditionals to exclude YACL, RConfig, and ModMenu compatibility code for MC >= 26.1
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| settings.gradle.kts | Updated Kotlin and DGT versions, added 26.1-fabric to version list |
| gradle/libs.versions.toml | Updated DGT, textile, and omnicore versions |
| gradle.properties | Incremented Minecraft revision from 4 to 5 |
| gradle/wrapper/* | Updated Gradle wrapper to 9.2.1 |
| minecraft/versions/26.1-fabric/gradle.properties | New gradle properties for 26.1 Fabric version |
| minecraft/root.gradle.kts | Added 26.1-fabric to preprocessing hierarchy |
| modules/relocator/src/main/kotlin/utils.kt | Enhanced version conversion logic to handle drop versions |
| minecraft/build.gradle.kts | Updated version handling for drop versions, changed manifest logic, added empty function |
| buildSrc/src/main/kotlin/org/polyfrost/gradle/addDependencies.kt | Updated dependency logic for version 26.1 and drop version support |
| minecraft/src/main/resources/fabric.mod.json | Changed minecraft version variable reference |
| modules/dependencies/legacy/build.gradle.kts | Changed Loom plugin reference |
| Multiple compat files | Added preprocessor conditionals to exclude compatibility code for MC >= 26.1 |
| bootstrap/build.gradle.kts | Added debug println statement |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
minecraft/build.gradle.kts
Outdated
|
|
||
| fun Manifest.applyManifest() { | ||
|
|
||
| } |
There was a problem hiding this comment.
This function is defined but empty and never used. Consider removing it to keep the codebase clean.
| fun Manifest.applyManifest() { | |
| } |
| * @param loader The mod loader being used. | ||
| */ | ||
| fun Project.provideIncludedDependencies(version: Triple<Int, Int, Int>?, loader: String?): List<OCDependency> { // Either a String or ExternalModuleDependency | ||
| fun Project.provideIncludedDependencies(version: Triple<Int, Int, Int>?, loader: String?, shenanigans: String? = version?.toMCVer()): List<OCDependency> { // Either a String or ExternalModuleDependency |
There was a problem hiding this comment.
The parameter name "shenanigans" is unprofessional and unclear. Consider renaming it to something more descriptive like "mcVersionString" or "fabricMcVersion" to better indicate its purpose as a Minecraft version string override.
| val version = if (version.substringBefore('.') != "1") { | ||
| version.substringBefore("-") | ||
| } else version | ||
|
|
||
| return version.let { if (it.count { char -> char == '.' } == 1) "$it.0" else it }.split(".") |
There was a problem hiding this comment.
Variable shadowing: The local variable "version" on line 15 shadows the function parameter "version" from line 14. Consider using a different name like "normalizedVersion" or "cleanedVersion" to avoid confusion.
| val version = if (version.substringBefore('.') != "1") { | |
| version.substringBefore("-") | |
| } else version | |
| return version.let { if (it.count { char -> char == '.' } == 1) "$it.0" else it }.split(".") | |
| val normalizedVersion = if (version.substringBefore('.') != "1") { | |
| version.substringBefore("-") | |
| } else version | |
| return normalizedVersion.let { if (it.count { char -> char == '.' } == 1) "$it.0" else it }.split(".") |
bootstrap/build.gradle.kts
Outdated
| println("[${mcData.version::class.java}] ${mcData.version}") | ||
|
|
There was a problem hiding this comment.
Debug println statement should be removed before merging. This appears to be development/debugging code that was left in.
| println("[${mcData.version::class.java}] ${mcData.version}") |
settings.gradle.kts
Outdated
| kotlin("jvm") version("2.2.10") | ||
| id("dev.deftu.gradle.multiversion-root") version("2.69.0") // Update in libs.versions.toml too!!! | ||
| kotlin("jvm") version("2.3.0") | ||
| id("dev.deftu.gradle.multiversion-root") version("2.72.0") // Update in libs.versions.toml too!!! |
There was a problem hiding this comment.
The PR title "Dgt 2.71 and mc 26.1" mentions DGT version 2.71, but the actual version being updated to is 2.72.0 (as seen in this file and gradle/libs.versions.toml). The title should be corrected to reflect the actual version.
cdafb2b
into
state-v2-and-actually-use-glrenderer
Job Summary for GradleBuild :: build
|
ough