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

Update KQL to Minecraft 1.21 and Kotlin 2.0.0 #98

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Pyrofab
Copy link
Member

@Pyrofab Pyrofab commented Jul 29, 2024

Main highlights :

  • item tooltip API now takes a DataComponentMap argument instead of an NbtCompound
  • some Brigadier DSL methods now take an additional context parameter
    • future refactors of the DSL could hide this parameter away inside a receiver object
  • some Brigadier classes are still unmapped

Fixes #96 and #97

@quinn-semele
Copy link
Contributor

Looks good to me but the gradle checksum needs to be updated in gradle-wrapper.properties file, honestly not really sure why qkl verifies the gradle wrapper though I guess it is good practice. You can find the value needed here: https://gradle.org/release-checksums/#v8.8

Copy link

@770grappenmaker 770grappenmaker left a comment

Choose a reason for hiding this comment

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

Please run ./gradlew wrapper --gradle-version latest after applying my changes, in order to update gradle-wrapper.jar, gradlew and gradlew.bat.

An additional suggestion I'd have is using the toolchains feature and the new compilerOptions block in favor of the kotlinOptions.

diff --git a/build.gradle.kts b/build.gradle.kts
index 03b64a2..aaaf23e 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -5,7 +5,7 @@
 import org.jetbrains.dokka.base.DokkaBaseConfiguration
 import org.jetbrains.dokka.gradle.AbstractDokkaLeafTask
 import org.jetbrains.dokka.gradle.AbstractDokkaTask
-import org.jetbrains.kotlin.gradle.tasks.KotlinCompile
+import org.jetbrains.kotlin.gradle.dsl.KotlinVersion
 import java.time.Year

 @Suppress(
@@ -39,7 +39,7 @@
 version = "${project.version}+kt.${project.libs.versions.kotlin.orNull}+flk.$flkVersion"
 val projectVersion = project.version as String + if (System.getenv("SNAPSHOTS_URL") != null && System.getenv("MAVEN_URL") == null) "-SNAPSHOT" else ""

-val javaVersion = 17 // The current version of Java used by Minecraft
+val javaVersion = 21 // The current version of Java used by Minecraft

 repositories {
     mavenCentral()
@@ -78,6 +78,12 @@ fun DependencyHandlerScope.includeApi(dependency: Any) {
     kotlin {
         // Enable explicit API mode, as this is a library
         explicitApi()
+        jvmToolchain(javaVersion)
+        compilerOptions {
+            languageVersion = KotlinVersion.fromVersion(
+                rootProject.libs.plugins.kotlin.get().version.requiredVersion.substringBeforeLast(".")
+            )
+        }
     }

     tasks {
@@ -91,14 +97,6 @@ fun DependencyHandlerScope.includeApi(dependency: Any) {
             }
         }

-        withType<KotlinCompile> {
-            kotlinOptions {
-                jvmTarget = javaVersion.toString()
-                languageVersion =
-                    rootProject.libs.plugins.kotlin.get().version.requiredVersion.substringBeforeLast(".")
-            }
-        }
-
         // Every dokka task
         withType<AbstractDokkaTask> {
             pluginConfiguration<DokkaBase, DokkaBaseConfiguration> {
diff --git a/settings.gradle.kts b/settings.gradle.kts
index 370c2da..c563533 100644
--- a/settings.gradle.kts
+++ b/settings.gradle.kts
@@ -12,6 +12,11 @@
         gradlePluginPortal()
     }
 }
+
+plugins {
+    id("org.gradle.toolchains.foojay-resolver-convention") version "0.8.0"
+}
+
 rootProject.name = "quilt-kotlin-libraries"

 include(":core")

Comment on lines 1 to 7
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionSha256Sum=9631d53cf3e74bfa726893aee1f8994fee4e060c401335946dba2156f440f24c
distributionUrl=https\://services.gradle.org/distributions/gradle-8.6-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.8-bin.zip
networkTimeout=10000
validateDistributionUrl=true
zipStoreBase=GRADLE_USER_HOME

Choose a reason for hiding this comment

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

Suggested change
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionSha256Sum=9631d53cf3e74bfa726893aee1f8994fee4e060c401335946dba2156f440f24c
distributionUrl=https\://services.gradle.org/distributions/gradle-8.6-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.8-bin.zip
networkTimeout=10000
validateDistributionUrl=true
zipStoreBase=GRADLE_USER_HOME
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionSha256Sum=d725d707bfabd4dfdc958c624003b3c80accc03f7037b5122c4b1d0ef15cecab
distributionUrl=https\://services.gradle.org/distributions/gradle-8.9-bin.zip
networkTimeout=10000
validateDistributionUrl=true
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists

@quinn-semele
Copy link
Contributor

Would I be able to take over this PR as I'm interested in maintaining QKL going forward, though this PR doesn't seem to need a lot of changes if you'd like to finish it off and I'll do a basic 1.21.1 port afterwards (most likely just updated dependencies)

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.

Update for Kotlin 2.0.0
3 participants