feat-KotlinSerialization #360
Conversation
* Added dependencies.toml and version catalogue. * Added Serializers for easy data class serialization for non-kotlin classes, like Took 3 hours 42 minutes
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update migrates the entire codebase from Gson-based JSON serialization to Kotlinx Serialization, affecting both core and paper modules. It introduces a centralized version catalog for dependencies and plugins, updates Gradle configuration and wrapper scripts, and refactors numerous serializers to use concise, delegated Kotlinx Serialization implementations. Several previously custom serializers are removed or replaced by standard or generated serializers. Serialization annotations are added broadly to data classes and sealed interfaces. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Serializer
participant JsonFormat
App->>Serializer: Request serialization/deserialization
Serializer->>JsonFormat: Encode/Decode using KSerializer
JsonFormat-->>Serializer: Serialized/Deserialized data
Serializer-->>App: Return result
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🔭 Outside diff range comments (11)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/EntryReferenceSerializer.kt (1)
1-38: Remove unused imports and commented codeSince the
Refclass is now handled by@Serializableannotation (as indicated by the comment), this entire file appears to be obsolete. Either:
- Delete this file entirely if it's no longer needed, or
- Remove all the commented code and unused imports
All imports in this file are now unused after commenting out the implementation.
If keeping the file, apply this diff to clean it up:
-package com.typewritermc.engine.paper.loader.serializers - -import com.google.common.reflect.TypeToken -import com.google.gson.JsonDeserializationContext -import com.google.gson.JsonElement -import com.google.gson.JsonPrimitive -import com.google.gson.JsonSerializationContext -import com.typewritermc.core.entries.Entry -import com.typewritermc.core.entries.Ref -import com.typewritermc.core.serialization.DataSerializer -import java.lang.reflect.ParameterizedType -import java.lang.reflect.Type -import kotlin.reflect.KClass -// should be handled by @Serializable -//class EntryReferenceSerializer : DataSerializer<Ref<*>> { -// override val clazz: Type = Ref::class.java -// -// override fun serialize(src: Ref<*>?, typeOfSrc: Type?, context: JsonSerializationContext?): JsonElement { -// return JsonPrimitive(src?.id ?: "") -// } -// -// override fun deserialize( -// json: JsonElement?, -// typeOfT: Type?, -// context: JsonDeserializationContext? -// ): Ref<*> { -// val subType = (typeOfT as ParameterizedType).actualTypeArguments[0] -// val clazz = TypeToken.of(subType).rawType -// val klass = clazz.kotlin as KClass<Entry> -// -// if (json?.isJsonNull == true) { -// return Ref("", klass) -// } -// -// val id = json?.asString ?: "" -// return Ref(id, klass) -// } -//} +// This file is no longer needed as Ref is now handled by @Serializable annotationOr preferably, delete the entire file.
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/SoundIdSerializer.kt (1)
3-12: Remove unused importsAfter refactoring to use delegated serialization, the following imports are no longer used and should be removed:
- Lines 3-6: Gson-related imports
- Line 12:
java.lang.reflect.Type-import com.google.gson.JsonDeserializationContext -import com.google.gson.JsonElement -import com.google.gson.JsonObject -import com.google.gson.JsonSerializationContext import com.typewritermc.engine.paper.utils.DefaultSoundId import com.typewritermc.engine.paper.utils.EntrySoundId import com.typewritermc.engine.paper.utils.SoundId import com.typewritermc.core.serialization.DataSerializer import com.typewritermc.core.serialization.DataSerializer.Companion.toDataSerializer -import java.lang.reflect.Typeengine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/VectorSerializer.kt (1)
3-11: Remove unused importsAfter refactoring to use delegated serialization, the following imports are no longer used and should be removed:
- Lines 3-6: Gson-related imports
- Line 11:
java.lang.reflect.Type-import com.google.gson.JsonDeserializationContext -import com.google.gson.JsonElement -import com.google.gson.JsonObject -import com.google.gson.JsonSerializationContext import com.typewritermc.core.serialization.DataSerializer import com.typewritermc.core.serialization.serializer import kotlinx.serialization.builtins.serializer import org.bukkit.util.Vector -import java.lang.reflect.Typeengine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/WorldSerializer.kt (1)
3-13: Remove unused importsAfter refactoring to use delegated serialization, the following imports are no longer used and should be removed:
- Lines 3-6: Gson-related imports
- Lines 10-11:
logErrorIfNullandserverutilities- Line 12:
java.lang.reflect.Type- Line 13:
java.util.*-import com.google.gson.JsonDeserializationContext -import com.google.gson.JsonElement -import com.google.gson.JsonPrimitive -import com.google.gson.JsonSerializationContext import com.typewritermc.core.serialization.DataSerializer import com.typewritermc.core.serialization.DataSerializer.Companion.toDataSerializer import com.typewritermc.core.utils.point.World -import com.typewritermc.engine.paper.utils.logErrorIfNull -import com.typewritermc.engine.paper.utils.server -import java.lang.reflect.Type -import java.util.*engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/ColorSerializer.kt (1)
3-12: Remove unused importsAfter refactoring to use delegated serialization, the following imports are no longer used and should be removed:
- Lines 3-6: Gson-related imports
- Line 12:
java.lang.reflect.Type-import com.google.gson.JsonDeserializationContext -import com.google.gson.JsonElement -import com.google.gson.JsonPrimitive -import com.google.gson.JsonSerializationContext import com.typewritermc.engine.paper.utils.Color import com.typewritermc.core.serialization.DataSerializer import com.typewritermc.core.serialization.DataSerializer.Companion.toDataSerializer import com.typewritermc.core.serialization.xmap import kotlinx.serialization.builtins.serializer -import java.lang.reflect.Typeengine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/DurationSerializer.kt (1)
3-11: Remove unused Gson imports after migration.The Gson-related imports are no longer needed after migrating to Kotlinx Serialization.
-import com.google.gson.JsonDeserializationContext -import com.google.gson.JsonElement -import com.google.gson.JsonPrimitive -import com.google.gson.JsonSerializationContext import com.typewritermc.core.serialization.DataSerializer import com.typewritermc.core.serialization.DataSerializer.Companion.toDataSerializer import com.typewritermc.core.serialization.xmap import kotlinx.serialization.builtins.serializer -import java.lang.reflect.Type import java.time.Duration import kotlin.time.toJavaDuration import kotlin.time.toKotlinDurationengine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/SkinPropertySerializer.kt (1)
3-15: Remove all unused imports after migration.The delegation to
SkinProperty.serializer().toDataSerializer()is correct, but the file has many unused imports that should be removed.package com.typewritermc.engine.paper.loader.serializers -import com.google.gson.JsonDeserializationContext -import com.google.gson.JsonElement -import com.google.gson.JsonObject -import com.google.gson.JsonSerializationContext import com.typewritermc.engine.paper.entry.entity.SkinProperty import com.typewritermc.core.serialization.DataSerializer import com.typewritermc.core.serialization.DataSerializer.Companion.toDataSerializer -import com.typewritermc.core.serialization.serializer -import kotlinx.serialization.builtins.nullable -import kotlinx.serialization.builtins.serializer -import java.lang.reflect.Type class SkinPropertySerializer : DataSerializer<SkinProperty> by SkinProperty.serializer().toDataSerializer()engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entries/VariableEntry.kt (2)
69-75: Missing @serializable annotation on ConstVar class.The
ConstVarclass is missing the@Serializableannotation, which is inconsistent with otherVarimplementations and could cause serialization issues.+@Serializable class ConstVar<T : Any>(val value: T) : Var<T> {
109-119: Serialization Strategy for ComputeVar
Currently, any ComputeVar instance will cause VarSerializer to throw an IllegalStateException (“Could not serialize compute var”). You need to decide which of the following makes the intended behavior explicit:
- If ComputeVar should never be serialized:
- Document this in the class KDoc (e.g. “This implementation is not serializable.”).
- Optionally annotate the
computeproperty with@Transientor mark the class as non-serializable so downstream consumers know it’s excluded.- If you do need to persist ComputeVar logic in configs:
- Introduce a metadata representation for the computation (e.g. a serializable descriptor or enum of built-in functions).
- Implement a custom serializer that can read/write that descriptor and reconstruct the lambda at runtime.
Locations to address:
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/VarSerializer.kt (handles ComputeVar by throwing)
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entries/VariableEntry.kt (definition of ComputeVar)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/PotionEffectTypeSerializer.kt (1)
3-6: Remove unused Gson imports.These Gson-related imports are no longer used after the migration to Kotlinx Serialization.
-import com.google.gson.JsonDeserializationContext -import com.google.gson.JsonElement -import com.google.gson.JsonPrimitive -import com.google.gson.JsonSerializationContext import com.typewritermc.core.serialization.DataSerializer import com.typewritermc.core.serialization.DataSerializer.Companion.toDataSerializer import com.typewritermc.core.serialization.xmap import kotlinx.serialization.builtins.serializer import net.kyori.adventure.key.Key import org.bukkit.Registry import org.bukkit.potion.PotionEffectType -import java.lang.reflect.TypeAlso applies to: 14-14
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/ClosedRangeSerializer.kt (1)
3-6: Remove unused Gson imports and Java reflection types.These imports are no longer needed after migrating to Kotlinx Serialization.
-import com.google.gson.JsonDeserializationContext -import com.google.gson.JsonElement -import com.google.gson.JsonObject -import com.google.gson.JsonSerializationContext import com.typewritermc.core.serialization.DataSerializer import com.typewritermc.core.serialization.DataSerializer.Companion.toDataSerializer import com.typewritermc.engine.paper.plugin import kotlinx.serialization.builtins.serializer import kotlinx.serialization.descriptors.SerialDescriptor import kotlinx.serialization.descriptors.buildClassSerialDescriptor import kotlinx.serialization.encoding.Decoder import kotlinx.serialization.encoding.Encoder import kotlinx.serialization.json.Json import kotlinx.serialization.serializer import org.koin.core.component.get import org.koin.core.qualifier.named -import java.lang.reflect.ParameterizedType -import java.lang.reflect.Type import kotlin.reflect.KClassAlso applies to: 19-20
♻️ Duplicate comments (1)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/EntryInteractionContextKeySerializer.kt (1)
19-21: SameTypevsClass<*>mismatch as above
clazzshould be aClass<*>just like the rest of the fleet to avoidinstanceofcasts:- override val clazz: Type = EntryInteractionContextKey::class.java + override val clazz: Class<*> = EntryInteractionContextKey::class.java
🧹 Nitpick comments (11)
engine/engine-core/src/main/kotlin/com/typewritermc/core/entries/Entry.kt (1)
5-5: Unused import should be removed or applied.The
kotlinx.serialization.Serializableimport is added but not used in the visible code. If this is preparation for future serialization, consider adding a TODO comment. Otherwise, remove the unused import.-import kotlinx.serialization.Serializable +// TODO: Add @Serializable annotation when implementing serialization +import kotlinx.serialization.Serializableengine/engine-core/src/main/kotlin/com/typewritermc/core/entries/Page.kt (1)
4-6: Good explanatory comment, but could be more specific.The comment explaining why
@Serializableis not used is helpful. Consider expanding it to explain what specific aspects of error reporting would be affected.-// Could be @Serializable, but error reporting would be "worse" +// Could be @Serializable, but would degrade error reporting quality +// by losing detailed field-level validation messages during deserializationengine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/VarSerializer.kt (1)
15-16:clazzrename is fine, but the serializer is still Gson-bound – plan removal or bridgeProperty rename aligns with the new
DataSerializercontract, ✔️.
However, the whole class keeps hard references to Gson while the PR objective is “switch to Kotlin Serialization”. Carrying both stacks balloons the classpath and slows reflection scanning.Consider one of:
- import com.google.gson.* + // kotlinx.serialization.json.* and bridging adapter (if strictly needed)or implement a
VarSerializerviaKSerializer+DataSerializer.toDataSerializer()like other migrated types.engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/GenericSerializer.kt (1)
1-7: Remove unused imports after migration to Kotlinx Serialization.The migration to delegated serializer looks good. However, you can clean up the imports since only
DataSerializerandtoDataSerializerare needed now.package com.typewritermc.engine.paper.loader.serializers import com.typewritermc.core.serialization.DataSerializer import com.typewritermc.core.serialization.DataSerializer.Companion.toDataSerializer import com.typewritermc.core.utils.Generic class GenericSerializer : DataSerializer<Generic> by Generic.serializer().toDataSerializer()engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/CronExpressionSerializer.kt (1)
14-14: Consider expanding the comment for clarity.The comment
// CronExpression.createDynamicis quite minimal. Consider expanding it to explain the purpose or signature of the method.-// CronExpression.createDynamic +// CronExpression.createDynamic(expression: String): CronExpressionengine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/ClosedRangeSerializer.kt (1)
10-17: Remove unused Kotlinx Serialization imports.Several Kotlinx Serialization imports are not used in this delegated implementation.
import com.typewritermc.engine.paper.plugin -import kotlinx.serialization.builtins.serializer -import kotlinx.serialization.descriptors.SerialDescriptor -import kotlinx.serialization.descriptors.buildClassSerialDescriptor -import kotlinx.serialization.encoding.Decoder -import kotlinx.serialization.encoding.Encoder import kotlinx.serialization.json.Json import kotlinx.serialization.serializer import org.koin.core.component.get import org.koin.core.qualifier.namedengine/engine-core/src/main/kotlin/com/typewritermc/core/entries/Library.kt (1)
94-94: Consider enabling the @serializable annotation check.The commented-out require statement would ensure entry classes are properly annotated for Kotlinx Serialization. This check helps catch configuration errors early.
- // require(clazz.kotlin.hasAnnotation<Serializable>()) { "Entry class '${clazz.name}' is not annotated with @Serializable." } + require(clazz.kotlin.hasAnnotation<Serializable>()) { "Entry class '${clazz.name}' is not annotated with @Serializable." }engine/engine-core/src/main/kotlin/com/typewritermc/core/utils/Generic.kt (1)
58-58: Consider implementing recursive comparison for complex types.The TODO comment indicates that recursive comparison for JsonObject and JsonArray might be beneficial for proper ordering semantics.
Would you like me to implement recursive comparison logic for JsonObject and JsonArray to provide more meaningful ordering?
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/utils/BukkitDataParser.kt (1)
22-29: Consider implementing a centralized serialization module registry.The TODO comment indicates awareness of the issue. A centralized approach would improve maintainability and avoid scattered serializer registrations across the codebase.
Would you like me to suggest an implementation for a centralized serialization module management system?
engine/engine-core/src/main/kotlin/com/typewritermc/core/serialization/Serializers.kt (2)
1-1: Document the rationale for suppressed warnings.Consider adding a comment explaining why these suppressions are necessary, especially for future maintainers.
55-334: Consider code generation for repetitive serializer functions.While the current implementation is correct and type-safe, the repetitive nature of these functions could be addressed through code generation (e.g., using KSP or Kotlin compiler plugins) to reduce maintenance burden and support more than 10 properties if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
engine/gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (40)
engine/build.gradle.kts(3 hunks)engine/dependencies.toml(1 hunks)engine/engine-core/src/main/kotlin/com/typewritermc/core/books/pages/PageType.kt(1 hunks)engine/engine-core/src/main/kotlin/com/typewritermc/core/entries/Entry.kt(2 hunks)engine/engine-core/src/main/kotlin/com/typewritermc/core/entries/EntryReference.kt(2 hunks)engine/engine-core/src/main/kotlin/com/typewritermc/core/entries/Library.kt(4 hunks)engine/engine-core/src/main/kotlin/com/typewritermc/core/entries/Page.kt(1 hunks)engine/engine-core/src/main/kotlin/com/typewritermc/core/interaction/Interaction.kt(2 hunks)engine/engine-core/src/main/kotlin/com/typewritermc/core/serialization/AlgebraicSerialization.kt(0 hunks)engine/engine-core/src/main/kotlin/com/typewritermc/core/serialization/DataSerializer.kt(1 hunks)engine/engine-core/src/main/kotlin/com/typewritermc/core/serialization/Serializers.kt(1 hunks)engine/engine-core/src/main/kotlin/com/typewritermc/core/utils/Casting.kt(2 hunks)engine/engine-core/src/main/kotlin/com/typewritermc/core/utils/Extensions.kt(2 hunks)engine/engine-core/src/main/kotlin/com/typewritermc/core/utils/Generic.kt(1 hunks)engine/engine-core/src/main/kotlin/com/typewritermc/core/utils/RuntimeTypeAdapterFactory.kt(0 hunks)engine/engine-loader/src/main/kotlin/com/typewritermc/loader/ExtensionLoader.kt(9 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/TypewriterPaperPlugin.kt(3 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entries/VariableEntry.kt(5 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/ClosedRangeSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/ColorSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/CoordinateSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/CronExpressionSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/DurationSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/EntryInteractionContextKeySerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/EntryReferenceSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/GenericSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/OptionalSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/PositionSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/PotionEffectTypeSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/SkinPropertySerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/SoundIdSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/SoundSourceSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/VarSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/VectorSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/WorldSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/utils/BukkitDataParser.kt(2 hunks)engine/gradle/wrapper/gradle-wrapper.properties(1 hunks)engine/gradlew(4 hunks)engine/gradlew.bat(2 hunks)engine/settings.gradle.kts(2 hunks)
💤 Files with no reviewable changes (2)
- engine/engine-core/src/main/kotlin/com/typewritermc/core/serialization/AlgebraicSerialization.kt
- engine/engine-core/src/main/kotlin/com/typewritermc/core/utils/RuntimeTypeAdapterFactory.kt
🧰 Additional context used
🧬 Code Graph Analysis (6)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/ColorSerializer.kt (1)
engine/engine-core/src/main/kotlin/com/typewritermc/core/serialization/Serializers.kt (10)
serializer(55-72)serializer(75-94)serializer(96-117)serializer(119-142)serializer(144-169)serializer(171-198)serializer(200-229)serializer(231-262)serializer(264-297)serializer(299-334)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/PotionEffectTypeSerializer.kt (1)
engine/engine-core/src/main/kotlin/com/typewritermc/core/serialization/Serializers.kt (10)
serializer(55-72)serializer(75-94)serializer(96-117)serializer(119-142)serializer(144-169)serializer(171-198)serializer(200-229)serializer(231-262)serializer(264-297)serializer(299-334)
engine/engine-core/src/main/kotlin/com/typewritermc/core/utils/Casting.kt (1)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/GenericSerializer.kt (1)
Generic(7-7)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/utils/BukkitDataParser.kt (1)
engine/engine-core/src/main/kotlin/com/typewritermc/core/serialization/Serializers.kt (10)
serializer(55-72)serializer(75-94)serializer(96-117)serializer(119-142)serializer(144-169)serializer(171-198)serializer(200-229)serializer(231-262)serializer(264-297)serializer(299-334)
engine/engine-loader/src/main/kotlin/com/typewritermc/loader/ExtensionLoader.kt (1)
engine/engine-core/src/main/kotlin/com/typewritermc/core/serialization/DataSerializer.kt (1)
add(37-39)
engine/engine-core/src/main/kotlin/com/typewritermc/core/utils/Generic.kt (4)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/GenericSerializer.kt (1)
Generic(7-7)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entries/VariableEntry.kt (9)
get(22-25)get(24-24)get(54-57)get(56-56)get(60-67)get(70-70)get(83-86)get(98-100)get(112-114)extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/variables/RandomVariableEntry.kt (1)
get(25-37)extensions/BasicExtension/src/main/kotlin/com/typewritermc/basic/entries/variables/InteractionContextVariable.kt (1)
get(32-45)
⏰ 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 (40)
engine/gradlew.bat (3)
16-17: Good addition of SPDX license identifier.Adding the SPDX license identifier improves license compliance and makes the license information machine-readable.
73-73: CLASSPATH clearing is a good practice.Explicitly clearing the CLASSPATH environment variable before execution prevents potential conflicts from pre-existing classpath settings.
77-77: Java invocation method update looks correct.The change from classpath-based to jar-based invocation (
-jarflag) is a standard improvement that simplifies the Gradle wrapper startup process.engine/dependencies.toml (1)
27-28: Good use of bundles for related dependencies.Creating a bundle for Kotest libraries is a good practice that simplifies dependency management when multiple related libraries are used together.
engine/settings.gradle.kts (2)
1-7: Excellent implementation of version catalog management.The
dependencyResolutionManagementblock is properly configured to use the centralizeddependencies.tomlfile. This setup will provide consistent dependency management across the project.
24-24: New module inclusion looks appropriate.The addition of
engine-loadermodule is consistent with the project's modular structure.engine/gradle/wrapper/gradle-wrapper.properties (1)
3-3: Verify Gradle wrapper downgrade & distribution changeThe wrapper has been changed from 8.14.2-bin to 8.10-all (–4 minor versions, +≈60 MB archive). Please double-check:
• engine/gradle/wrapper/gradle-wrapper.properties – confirm this is the intended distribution
• build.gradle.kts – ensure all plugins (configuration-cache, Kotlin DSL 1.9+, KMP, etc.) compile under Gradle 8.10
• version catalog (e.g. gradle/libs.versions.toml) – verify that any cataloged plugins or dependency constraints remain compatible
• CI impact – validate download times and disk usage with the larger *-all bundleIf you don’t need the extra sources/tests in *-all, prefer the lighter ‑bin distro to keep CI download times and storage low.
engine/engine-core/src/main/kotlin/com/typewritermc/core/utils/Casting.kt (1)
82-83: LGTM! Clean migration from Gson to Kotlinx Serialization.The function correctly retrieves the
Jsoninstance from Koin and usesencodeToJsonElementto convert the value to aJsonElementfor theGenericconstructor.engine/engine-core/src/main/kotlin/com/typewritermc/core/utils/Extensions.kt (1)
9-24: LGTM! Solid camelCase to snake_case conversion.The implementation correctly handles the conversion logic for different character types. The function properly converts camelCase identifiers to snake_case format as needed for the serialization migration.
engine/engine-core/src/main/kotlin/com/typewritermc/core/interaction/Interaction.kt (1)
63-63: LGTM! Proper use of contextual serialization.The
@Contextualannotation is correctly applied to thekeyproperty, enabling the serialization framework to use registered contextual serializers for theEntryContextKeytype.engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/TypewriterPaperPlugin.kt (1)
128-129: LGTM! Proper migration to Kotlinx Serialization DI setup.The factory definitions correctly replace Gson with Kotlinx Serialization
Jsoninstances. ThedataSerializerfactory appropriately composes a serialization module from all registered serializers, and thebukkitDataParseruses the Bukkit-specific JSON format.engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/SoundIdSerializer.kt (1)
14-14: Clean migration to Kotlinx SerializationThe delegated implementation correctly uses the generated serializer from
@Serializableannotation onSoundId.engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/VectorSerializer.kt (1)
13-19: Well-structured serializer using property accessorsThe implementation correctly uses the generic serializer factory function with property accessors for each Vector component. This is a clean and maintainable approach.
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/ColorSerializer.kt (1)
14-15: Elegant use of xmap for type conversionThe implementation correctly uses
xmapto convert betweenIntandColortypes, providing a clean bidirectional mapping.engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/CoordinateSerializer.kt (1)
9-16: Well-structured declarative serializer implementation.The migration to Kotlinx Serialization is well done. Using nullable serializers with default values ensures backward compatibility and robustness when handling incomplete coordinate data.
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/SoundSourceSerializer.kt (1)
10-10: LGTM: Clean import addition for Kotlinx Serialization.The import of
toDataSerializerextension function supports the migration to delegated serialization.engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/CronExpressionSerializer.kt (2)
5-7: LGTM: Proper imports for Kotlinx Serialization utilities.The imports for
toDataSerializer,xmap, andserializerare correct for the delegated implementation pattern.
9-12: Approve CronExpressionSerializer ImplementationExcellent use of xmap for type transformation. The migration to use
String.serializer().xmap()is well-implemented:
{ expression }cleanly extracts the string representation fromCronExpression{ CronExpression.createDynamic(this) }correctly invokes the companion object factory- Declaring
CronExpressionSerializeras anobjectis appropriate since no state is maintainedThis pattern is much cleaner and more maintainable than manual Gson serialization.
engine/gradlew (5)
18-19: LGTM: Added SPDX license identifier.The addition of the SPDX license identifier improves license compliance and follows modern best practices.
89-89: LGTM: Improved APP_HOME resolution method.The change from
pwd -Ptocd -P "${APP_HOME:-./}" > /dev/null && printf '%s\n' "$PWD"provides better compatibility across different shell environments and handles edge cases more robustly.
117-117: LGTM: Cleared CLASSPATH variable.Setting CLASSPATH to an escaped empty string is appropriate when switching to
-jarinvocation method.
208-208: LGTM: Minor comment improvement.The comment refinement improves clarity.
216-216: LGTM: Switched to jar invocation method.The change from
-classpathwith main class to-jarwith the gradle-wrapper.jar is a standard Gradle wrapper update pattern and is correct.engine/build.gradle.kts (4)
3-3: LGTM: Migrated to version catalog for Kotlin JVM plugin.Using
libs.versions.kotlin.get()centralizes version management and improves maintainability.
6-7: LGTM: Proper version catalog usage for plugins.The use of
alias(libs.plugins.shadow)andalias(libs.plugins.kotlin.serialization)with version catalog references is correct and follows Gradle best practices.
34-35: LGTM: Consistent plugin application in subprojects.Using
rootProject.libs.plugins.*.get().pluginIdensures consistent plugin versions across all subprojects.
43-54: Version Catalog Verification: All dependencies presentI’ve confirmed that
engine/dependencies.tomldefines aliases for every dependency used inengine/build.gradle.kts(koin, kotlin-serialization, kotlinx-coroutines, mockk, and the kotest bundle). The migration to centralized version catalog references is complete—no further changes needed.engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entries/VariableEntry.kt (6)
11-13: LGTM: Proper imports for Kotlinx Serialization migration.The imports for
@Serializable,Json, andserializerare correct for the migration from Gson to Kotlinx Serialization.
33-33: LGTM: Dependency injection updated for Kotlinx Serialization.The change from
gson: Gsontojson: Jsonwith the named qualifier"dataSerializer"is consistent with the broader migration pattern.
36-36: LGTM: Proper API migration to Kotlinx Serialization.The change from
gson.fromJsontojson.decodeFromJsonElement(json.serializersModule.serializer(klass), data.data)is correct for Kotlinx Serialization. The use ofserializersModule.serializer(klass)dynamically obtains the appropriate serializer at runtime.
54-54: LGTM: Added @serializable to sealed interface.Adding
@Serializableto theVar<T>sealed interface enables automatic serialization for all implementations.
77-77: LGTM: Added @serializable to BackedVar class.The
@Serializableannotation onBackedVaris correct and necessary for the Kotlinx Serialization migration.
93-93: LGTM: Added @serializable to MappedVar class.The
@Serializableannotation onMappedVaris correct and necessary for the Kotlinx Serialization migration.engine/engine-core/src/main/kotlin/com/typewritermc/core/serialization/DataSerializer.kt (1)
1-53: Well-structured migration to Kotlinx Serialization!The refactored
DataSerializerinterface and utility functions provide a clean abstraction over Kotlinx Serialization. Thexmapfunction is particularly elegant for bidirectional type mapping.engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/utils/BukkitDataParser.kt (2)
35-38: LGTM!Clean implementation using Bukkit's built-in serialization methods with Base64 encoding.
40-44: LGTM!Proper UUID serialization using most and least significant bits.
engine/engine-core/src/main/kotlin/com/typewritermc/core/serialization/Serializers.kt (1)
12-53: Well-implemented core serialization logic.The function properly handles encoding/decoding with comprehensive null-safety checks for required fields. The unchecked cast is unavoidable due to type erasure and is handled safely.
engine/engine-loader/src/main/kotlin/com/typewritermc/loader/ExtensionLoader.kt (3)
115-122: LGTM!The migration to Kotlinx Serialization's
buildJsonArrayis implemented correctly.
195-319: Properly annotated for Kotlinx Serialization.All data classes and enums are correctly annotated with
@Serializableand@SerialNamefollowing Kotlinx Serialization conventions.
28-29: No functional issues with switching extensions to Set
- Extension is declared as a Kotlin data class, so equals()/hashCode() are correctly generated.
- No code relies on positional access (first(), elementAt(), indexOf(), etc.).
- Iteration-based lookups (e.g. firstNotNullOfOrNull) still work as expected: the map’s keySet preserves insertion order.
| kotlin = "2.0.21" | ||
|
|
||
| kotlinSerialization = "1.8.1" | ||
| coroutines = "1.10.2" | ||
| koin = "4.0.4" | ||
|
|
||
| kotest = "5.9.1" | ||
|
|
||
| shadow = "8.1.7" | ||
| mockk = "1.13.16" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify that dependency versions are current and secure.
The version catalog looks well-structured, but please verify that all specified versions are the latest stable releases and free from known security vulnerabilities.
🌐 Web query:
Are the following library versions current and secure: Kotlin 2.0.21, Kotlinx Serialization 1.8.1, Kotlinx Coroutines 1.10.2, Koin 4.0.4, Kotest 5.9.1, Shadow Plugin 8.1.7, and Mockk 1.13.16?
💡 Result:
Here is a review of the listed library versions as of July 11, 2025, addressing whether they are current and considered secure:
Kotlin 2.0.21
- Not current or supported. The most recent stable version is 2.2.0 (released June 23, 2025), and the 2.0 branch's support ended November 27, 2024. Using 2.0.21 means missing out on active security and bug fixes[4][7].
- Recommendation: Upgrade to at least 2.2.0 for support and security[4][7].
Kotlinx Serialization 1.8.1
- Not current. The library had a release with 1.8.x as of June 27, 2025, but it references 1.8.0-RC as a base; the latest stable version may be 1.8.1 or a minor increment, but check for a possible 1.8.2 or higher given recent activity[2].
- Recommendation: Use at least 1.8.x, but verify if a newer patch is available for the best support[2].
Kotlinx Coroutines 1.10.2
- Insufficient information in the search results to confirm if 1.10.2 is the latest. Coroutines frequently update, especially to track changes in Kotlin itself. Official documentation or the project's release page should be checked to confirm if a newer 1.10.x or 1.11.x is available.
Koin 4.0.4
- No coverage in search results for version recency. Historically, major Koin upgrades have followed Kotlin’s breaking changes. Koin 4.0.4 is likely outdated, as Koin 3.x was already available in 2023, with Koin 4 having appeared since then.
- Recommendation: Check the official Koin release page to confirm, but an upgrade is highly likely needed.
Kotest 5.9.1
- Current version. Kotest 5.9.1 is listed as the latest stable release as of June 5, 2024, with only alpha (6.0.0.Mx) versions being newer, so 5.9.1 is current and supported[6].
- Secure and recommended for production use.
Shadow Plugin 8.1.7
- No direct confirmation from search results. Shadow Plugin 8.x was the latest major version as of 2024. Check the plugin's release repository for any 8.1.x patch releases beyond 8.1.7.
- Recommendation: Ensure you are at least on the latest 8.1.x for bug and security fixes.
Mockk 1.13.16
- No release data in results. As of 2024, Mockk 1.13.x was current. Recommend confirming on the official repository, but 1.13.16 is likely recent and acceptable.
Summary Table
| Library | Your Version | Latest/Supported (as of July 2025) | Status | Recommendation |
|---|---|---|---|---|
| Kotlin | 2.0.21 | 2.2.0 | Outdated | Upgrade required |
| Kotlinx Serialization | 1.8.1 | 1.8.x (possibly newer) | Likely current | Confirm latest |
| Kotlinx Coroutines | 1.10.2 | Unknown | Uncertain | Check repository |
| Koin | 4.0.4 | Unknown, likely higher | Outdated | Upgrade likely |
| Kotest | 5.9.1 | 5.9.1 | Current | OK |
| Shadow Plugin | 8.1.7 | Possibly 8.1.7 or higher | Likely current | Confirm latest |
| Mockk | 1.13.16 | Possibly latest | Likely current | Confirm latest |
Security and Support Notes
- Kotlin language versions only get fixes on the latest major release[4], so running 2.0.21 leaves your project unsupported and potentially vulnerable.
- For all libraries, regularly check their repositories for new releases and changelogs, especially as they may be closely tied to Kotlin version compatibility.
Action Items
- Upgrade Kotlin to 2.2.0.
- Review and update other dependencies as needed, prioritizing Koin and any library with insufficient version information.
- For project security and maintainability, always align your Kotlin and major dependencies with supported, latest releases.
Citations:
- 1: https://kotlinlang.org/docs/whatsnew2120.html
- 2: https://github.com/Kotlin/kotlinx.serialization/releases
- 3: https://kotest.io/docs/changelog.html
- 4: https://endoflife.date/kotlin
- 5: https://github.com/Kotlin/kotlinx.serialization
- 6: https://github.com/kotest/kotest/releases
- 7: https://kotlinlang.org/docs/releases.html
- 8: https://kotlinlang.org/docs/serialization.html
- 9: https://micronaut.io/2022/12/27/micronaut-framework-3-8-0-released/
- 10: https://www.jetbrains.com/help/kotlin-multiplatform-dev/multiplatform-plugin-releases.html
Upgrade Outdated Dependencies in engine/dependencies.toml
A number of your catalog versions are outdated or unverified. Please update or confirm the latest stable releases:
• File: engine/dependencies.toml (lines 2–11)
• Action: bump versions or verify latest patches
-
Kotlin
- Current: 2.0.21
- Upgrade to ≥ 2.2.0 (2.0.x is unsupported since Nov 2024; 2.2.0 adds critical fixes)
- Diff example:
- kotlin = "2.0.21" + kotlin = "2.2.0"
-
Kotlinx Serialization
- Current: 1.8.1
- Verify if a newer 1.8.x patch exists; update to the highest stable 1.8.* release
-
Kotlinx Coroutines
- Current: 1.10.2
- Check the official repo for any 1.10.x or 1.11.x updates and bump accordingly
-
Koin
- Current: 4.0.4
- Likely outdated—upgrade to the latest 4.x release (consult Koin’s release page)
-
Kotest
- Current: 5.9.1
- ✅ Already on latest stable
-
Shadow Plugin
- Current: 8.1.7
- Confirm no newer 8.1.x patch; update if available
-
Mockk
- Current: 1.13.16
- Confirm latest 1.13.x patch and update if a newer release exists
Keeping these aligned with supported, secure versions will ensure you receive bug fixes and security patches.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| kotlin = "2.0.21" | |
| kotlinSerialization = "1.8.1" | |
| coroutines = "1.10.2" | |
| koin = "4.0.4" | |
| kotest = "5.9.1" | |
| shadow = "8.1.7" | |
| mockk = "1.13.16" | |
| kotlin = "2.2.0" | |
| kotlinSerialization = "1.8.1" | |
| coroutines = "1.10.2" | |
| koin = "4.0.4" | |
| kotest = "5.9.1" | |
| shadow = "8.1.7" | |
| mockk = "1.13.16" |
🤖 Prompt for AI Agents
In engine/dependencies.toml lines 2 to 11, several dependency versions are
outdated or unverified. Update Kotlin to version 2.2.0 or higher, verify and
bump Kotlinx Serialization to the latest 1.8.x patch, check and update Kotlinx
Coroutines to the latest 1.10.x or 1.11.x release, upgrade Koin to the latest
4.x version, confirm and update Shadow plugin to the newest 8.1.x patch if
available, and verify Mockk is at the latest 1.13.x patch, updating if
necessary. Leave Kotest at 5.9.1 as it is already the latest stable.
| class OptionalSerializer : DataSerializer<Optional<*>> { | ||
| override val type: Type = Optional::class.java | ||
| override val clazz: Type = Optional::class.java | ||
|
|
There was a problem hiding this comment.
Inconsistent use of Type vs Class<*> for clazz
Other serializers expose clazz: Class<*>; here it is Type.
This breaks the common reflection path (isAssignableFrom, java) and may fail Serializers.register() look-ups.
- override val clazz: Type = Optional::class.java
+ override val clazz: Class<*> = Optional::class.java📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class OptionalSerializer : DataSerializer<Optional<*>> { | |
| override val type: Type = Optional::class.java | |
| override val clazz: Type = Optional::class.java | |
| class OptionalSerializer : DataSerializer<Optional<*>> { | |
| override val clazz: Class<*> = Optional::class.java | |
| // … rest of implementation … | |
| } |
🤖 Prompt for AI Agents
In
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/OptionalSerializer.kt
around lines 12 to 14, the property clazz is declared as Type but should be
Class<*> to maintain consistency with other serializers. Change the type of
clazz from Type to Class<*> and assign it Optional::class.java to ensure
compatibility with reflection methods like isAssignableFrom and proper
functioning of Serializers.register() look-ups.
| import kotlinx.serialization.SerialName | ||
|
|
||
| enum class PageType(val id: String) { | ||
| @SerializedName("sequence") | ||
| @SerialName("sequence") | ||
| SEQUENCE("sequence"), | ||
|
|
||
| @SerializedName("static") | ||
| @SerialName("static") | ||
| STATIC("static"), | ||
|
|
||
| @SerializedName("cinematic") | ||
| @SerialName("cinematic") | ||
| CINEMATIC("cinematic"), | ||
|
|
||
| @SerializedName("manifest") | ||
| @SerialName("manifest") | ||
| MANIFEST("manifest"), | ||
| ; |
There was a problem hiding this comment.
Missing @Serializable – compilation will fail
@SerialName requires the enum itself to be annotated with @Serializable.
Without it, the Kotlinx compiler plugin reports:
SerialName annotation is meaningless on an enum without @Serializable
Fix:
-import kotlinx.serialization.SerialName
+import kotlinx.serialization.SerialName
+import kotlinx.serialization.Serializable
...
- enum class PageType(val id: String) {
+ @Serializable
+ enum class PageType(val id: String) {🤖 Prompt for AI Agents
In
engine/engine-core/src/main/kotlin/com/typewritermc/core/books/pages/PageType.kt
lines 3 to 17, the enum PageType uses @SerialName annotations but is missing the
@Serializable annotation on the enum itself. To fix this, add the @Serializable
annotation above the enum declaration to ensure proper serialization and avoid
compilation errors.
| @Serializable | ||
| class Ref<E : Entry>( | ||
| val id: String, | ||
| private val klass: KClass<E>, | ||
| private var cache: E? = null, | ||
| @Transient private var cache: E? = null, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify KClass serialization handling.
The @Serializable annotation looks correct, and marking cache as @Transient is appropriate since it's a runtime optimization. However, the klass property of type KClass<E> is not serializable by default in Kotlinx Serialization. This could cause runtime serialization failures.
Please verify that the KClass property is properly handled by running the following script to check if there's a custom serializer for this class:
🏁 Script executed:
#!/bin/bash
# Description: Check for custom serializers for Ref class and KClass handling
# Search for custom serializers for Ref class
echo "=== Searching for Ref serializers ==="
rg -A 10 -B 2 "class.*Ref.*Serializer"
# Search for KClass serialization handling
echo "=== Searching for KClass serialization ==="
rg -A 5 -B 2 "KClass.*serializer"
# Check if there are any contextual serializers for KClass
echo "=== Searching for contextual KClass serializers ==="
rg -A 5 -B 2 "@Contextual.*KClass|KClass.*@Contextual"Length of output: 2378
Add a serializer for the KClass property in Ref
The klass: KClass<E> constructor property isn’t serializable by default in Kotlinx Serialization—this will lead to runtime failures when (de)serializing Ref. You’ll need to either:
- Annotate
klasswith@Contextualand register theKClassserializer in yourSerializersModule, or - Provide a custom serializer via
@Serializable(with = YourKClassSerializer::class)
Suggested change in engine/engine-core/src/main/kotlin/com/typewritermc/core/entries/EntryReference.kt:
import kotlinx.serialization.Serializable
+import kotlinx.serialization.Contextual
@Serializable
class Ref<E : Entry>(
val id: String,
- private val klass: KClass<E>,
+ @Contextual
+ private val klass: KClass<E>,
@Transient private var cache: E? = null,
)• Don’t forget to add in your module setup:
SerializersModule {
contextual(KClass::class, KClassSerializer)
}🤖 Prompt for AI Agents
In
engine/engine-core/src/main/kotlin/com/typewritermc/core/entries/EntryReference.kt
around lines 17 to 21, the klass property of type KClass<E> is not serializable
by default, causing runtime serialization errors. Fix this by annotating klass
with @Contextual and ensure that the KClass serializer is registered in your
SerializersModule setup with contextual(KClass::class, KClassSerializer).
Alternatively, implement and use a custom serializer for KClass and annotate
klass with @Serializable(with = YourKClassSerializer::class).
| return World(bukkitWorld.uid.toString()) | ||
| } | ||
| } | ||
| class WorldSerializer : DataSerializer<World> by World.serializer().toDataSerializer() |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify World deserialization behavior
The previous implementation handled both UUID and world name lookups with fallback logic and error logging. Ensure that World.serializer() provides equivalent functionality for resolving world references.
🏁 Script executed:
#!/bin/bash
# Description: Check the World class implementation to verify serialization behavior
# Find the World class definition
fd -e kt "World.kt" | xargs rg -A 20 "^@Serializable.*class World"
# Check if there's a custom serializer defined for World
ast-grep --pattern '@Serializable($_)
class World {
$$$
}'Length of output: 129
Continuing verification by inspecting the serializer adapter and WorldSerializer implementation:
🏁 Script executed:
#!/usr/bin/env bash
set -e
# 1. Locate and display the WorldSerializer file
FILE=$(fd -e kt WorldSerializer.kt | head -n1)
echo "=== $FILE ==="
sed -n '1,200p' "$FILE"
# 2. Find the toDataSerializer extension that bridges KSerializer → DataSerializer
rg -n "fun .*toDataSerializer" -A5 -B5 || true
# 3. Locate the DataSerializer interface to see its read/write contract
rg -n "interface DataSerializer" -A5 -B5 || true
# 4. Search for any custom KSerializer<World> definitions
rg -n "KSerializer<World>" -A5 -B5 || true
# 5. Look for an extension or factory named serializer on World
rg -n "World\\.serializer" -A5 -B5 || trueLength of output: 5832
Continuing verification by inspecting the core World definition for custom serialization logic:
🏁 Script executed:
#!/usr/bin/env bash
set -e
# 1. Locate the World.kt file
FILE=$(fd -e kt World.kt | grep 'core/utils/point/World.kt' | head -n1)
echo "=== $FILE ==="
sed -n '1,200p' "$FILE"
# 2. Check for @Serializable on World
echo; echo "=== @Serializable lines ==="
rg -n '@Serializable' -A2 -B2 "$FILE" || true
# 3. Look for custom serializer implementations in World
echo; echo "=== Companion object & serializer hints ==="
rg -n 'companion object' -A5 -B5 "$FILE" || true
rg -n 'object Serializer' -A5 -B5 "$FILE" || trueLength of output: 1287
Restore custom World deserialization logic
The current WorldSerializer simply delegates to the default World.serializer(), which only reads/writes the identifier property and drops the previous UUID-and-name lookup plus error logging. You need to reintroduce a custom serializer that:
- Writes the world as a single string (
identifier). - On read, attempts
UUID.fromString(id)→server().getWorld(uuid), falls back toserver().getWorld(name), and logs errors vialogErrorIfNull.
Locations to update:
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/WorldSerializer.kt
Suggested diff:
- class WorldSerializer : DataSerializer<World> by World.serializer().toDataSerializer()
+ class WorldSerializer : DataSerializer<World> {
+ override val clazz = World::class
+ override val descriptor = PrimitiveSerialDescriptor("World", PrimitiveKind.STRING)
+
+ override fun serialize(encoder: Encoder, value: World) {
+ encoder.encodeString(value.identifier)
+ }
+
+ override fun deserialize(decoder: Decoder): World {
+ val idOrName = decoder.decodeString()
+ val world = runCatching { UUID.fromString(idOrName) }
+ .mapCatching { server().getWorld(it) }
+ .getOrNull()
+ ?: server().getWorld(idOrName)
+ return logErrorIfNull(world, "World not found: $idOrName")
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class WorldSerializer : DataSerializer<World> by World.serializer().toDataSerializer() | |
| class WorldSerializer : DataSerializer<World> { | |
| override val clazz = World::class | |
| override val descriptor = PrimitiveSerialDescriptor("World", PrimitiveKind.STRING) | |
| override fun serialize(encoder: Encoder, value: World) { | |
| encoder.encodeString(value.identifier) | |
| } | |
| override fun deserialize(decoder: Decoder): World { | |
| val idOrName = decoder.decodeString() | |
| val world = runCatching { UUID.fromString(idOrName) } | |
| .mapCatching { server().getWorld(it) } | |
| .getOrNull() | |
| ?: server().getWorld(idOrName) | |
| return logErrorIfNull(world, "World not found: $idOrName") | |
| } | |
| } |
🤖 Prompt for AI Agents
In
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/WorldSerializer.kt
at line 15, replace the current delegation to World.serializer() with a custom
serializer implementation. This serializer should write the world as a single
string using its identifier, and on reading, it should attempt to parse the
string as a UUID to retrieve the world via server().getWorld(uuid). If that
fails, it should fall back to server().getWorld(name) using the string as a
name. Ensure to log errors using logErrorIfNull when the world cannot be found.
This restores the previous custom deserialization logic that includes UUID and
name lookup with error logging.
| // Might work. | ||
| val serial = jsonFormat.serializersModule.serializer(clazz) | ||
| val entry = jsonFormat.decodeFromJsonElement(serial, obj) as Entry | ||
| entryValidation(entry, pageName, blueprintId) |
There was a problem hiding this comment.
Address uncertainty in serialization approach.
The comment "Might work" suggests uncertainty about the serialization approach. The unchecked cast to Entry could fail at runtime.
Consider adding explicit type checking:
- // Might work.
val serial = jsonFormat.serializersModule.serializer(clazz)
- val entry = jsonFormat.decodeFromJsonElement(serial, obj) as Entry
+ val decoded = jsonFormat.decodeFromJsonElement(serial, obj)
+ val entry = decoded as? Entry
+ ?: throw IllegalArgumentException("Decoded object is not an Entry: ${decoded::class.simpleName}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Might work. | |
| val serial = jsonFormat.serializersModule.serializer(clazz) | |
| val entry = jsonFormat.decodeFromJsonElement(serial, obj) as Entry | |
| entryValidation(entry, pageName, blueprintId) | |
| val serial = jsonFormat.serializersModule.serializer(clazz) | |
| val decoded = jsonFormat.decodeFromJsonElement(serial, obj) | |
| val entry = decoded as? Entry | |
| ?: throw IllegalArgumentException("Decoded object is not an Entry: ${decoded::class.simpleName}") | |
| entryValidation(entry, pageName, blueprintId) |
🤖 Prompt for AI Agents
In engine/engine-core/src/main/kotlin/com/typewritermc/core/entries/Library.kt
around lines 96 to 99, the current serialization approach uses an unchecked cast
to Entry which may cause runtime failures. Replace the unchecked cast by adding
explicit type checking after deserialization to ensure the object is indeed an
Entry instance before proceeding. If the type does not match, handle the error
gracefully, such as by throwing a clear exception or logging an error, to avoid
unexpected crashes.
| return jsonFormat.decodeFromJsonElement(jsonFormat.serializersModule.serializer(klass),data) as T? | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null-safe handling for deserialization.
The unchecked casts to T? could throw ClassCastException if the deserialization produces an incompatible type.
fun <T> get(klass: Class<T>): T? {
- return jsonFormat.decodeFromJsonElement(jsonFormat.serializersModule.serializer(klass),data) as T?
+ return try {
+ jsonFormat.decodeFromJsonElement(jsonFormat.serializersModule.serializer(klass), data) as? T
+ } catch (e: Exception) {
+ null
+ }
}
fun <T : Any> get(klass: KClass<T>): T? {
- return jsonFormat.decodeFromJsonElement(jsonFormat.serializersModule.serializer(klass.starProjectedType),data) as T?
+ return try {
+ jsonFormat.decodeFromJsonElement(jsonFormat.serializersModule.serializer(klass.starProjectedType), data) as? T
+ } catch (e: Exception) {
+ null
+ }
}Also applies to: 42-43
🤖 Prompt for AI Agents
In engine/engine-core/src/main/kotlin/com/typewritermc/core/utils/Generic.kt
around lines 38-39 and 42-43, the deserialization uses unchecked casts to T?
which can cause ClassCastException if the result type is incompatible. Modify
the code to safely handle nulls and type mismatches by checking the deserialized
object's type before casting or using safe cast operators, ensuring the method
returns null or a valid instance without throwing exceptions.
| fun createBukkitDataParser(): Gson = GsonBuilder() | ||
| .create() | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Function no longer provides Bukkit data parsing functionality.
The function name createBukkitDataParser() is misleading as it now returns a plain Gson instance without any Bukkit-specific serialization configuration. Consider either removing this function or updating its name/documentation to reflect its current behavior.
🤖 Prompt for AI Agents
In
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/utils/BukkitDataParser.kt
around lines 31 to 33, the function createBukkitDataParser() returns a plain
Gson instance without any Bukkit-specific configuration, making its name
misleading. To fix this, either rename the function to something like
createGson() to reflect its generic behavior or update the function to include
Bukkit-specific serialization setup if that functionality is still needed.
Alternatively, remove the function if it is no longer used or necessary.
There was a problem hiding this comment.
Potential IndexOutOfBoundsException in world fallback logic.
The fallback to Bukkit.getWorlds()[0] could throw an exception if no worlds are loaded (e.g., during testing or server shutdown).
Apply this diff to add safer fallback logic:
- if (worldObj == null) {
- logger.warning("Failed to find world '$world' while deserializing location '$this'")
- Location(Bukkit.getWorlds()[0], x, y, z, yaw ?: 0f, pitch ?: 0f)
- } else {
- Location(worldObj, x, y, z, yaw ?: 0f, pitch ?: 0f)
- }
+ if (worldObj == null) {
+ logger.warning("Failed to find world '$world' while deserializing location")
+ val fallbackWorld = Bukkit.getWorlds().firstOrNull()
+ if (fallbackWorld == null) {
+ throw IllegalStateException("No worlds available for location deserialization")
+ }
+ Location(fallbackWorld, x, y, z, yaw ?: 0f, pitch ?: 0f)
+ } else {
+ Location(worldObj, x, y, z, yaw ?: 0f, pitch ?: 0f)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| object LocationSerializer : KSerializer<Location> by serializer( | |
| "x", { x }, Double.serializer(), | |
| "y", { y }, Double.serializer(), | |
| "z", { z }, Double.serializer(), | |
| "yaw", { yaw }, Float.serializer().nullable, | |
| "pitch", { pitch }, Float.serializer().nullable, | |
| "world", { world.uuid }, UUIDSerializer, | |
| { x, y, z, yaw, pitch, world -> | |
| val worldObj = Bukkit.getWorld(world) | |
| if (worldObj == null) { | |
| logger.warning("Failed to find world '$world' while deserializing location '$this'") | |
| Location(Bukkit.getWorlds()[0], x, y, z, yaw ?: 0f, pitch ?: 0f) | |
| } else { | |
| Location(worldObj, x, y, z, yaw ?: 0f, pitch ?: 0f) | |
| } | |
| } | |
| } | |
| ) | |
| object LocationSerializer : KSerializer<Location> by serializer( | |
| "x", { x }, Double.serializer(), | |
| "y", { y }, Double.serializer(), | |
| "z", { z }, Double.serializer(), | |
| "yaw", { yaw }, Float.serializer().nullable, | |
| "pitch", { pitch }, Float.serializer().nullable, | |
| "world", { world.uuid }, UUIDSerializer, | |
| { x, y, z, yaw, pitch, world -> | |
| val worldObj = Bukkit.getWorld(world) | |
| if (worldObj == null) { | |
| logger.warning("Failed to find world '$world' while deserializing location") | |
| val fallbackWorld = Bukkit.getWorlds().firstOrNull() | |
| if (fallbackWorld == null) { | |
| throw IllegalStateException("No worlds available for location deserialization") | |
| } | |
| Location(fallbackWorld, x, y, z, yaw ?: 0f, pitch ?: 0f) | |
| } else { | |
| Location(worldObj, x, y, z, yaw ?: 0f, pitch ?: 0f) | |
| } | |
| } | |
| ) |
🤖 Prompt for AI Agents
In
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/utils/BukkitDataParser.kt
between lines 46 and 62, the fallback logic uses Bukkit.getWorlds()[0] which can
throw an IndexOutOfBoundsException if no worlds are loaded. To fix this, check
if Bukkit.getWorlds() is not empty before accessing the first element; if it is
empty, handle the case safely by either returning null, throwing a controlled
exception, or providing a sensible default to avoid the exception.
| @OptIn(ExperimentalSerializationApi::class) | ||
| fun readJsonTexts(json: List<String>): Map<Extension, String> = json.associateBy { extensionJson -> | ||
| try { | ||
| gson.fromJson(extensionJson, Extension::class.java) | ||
| } catch (e: JsonSyntaxException) { | ||
| jsonFormat.decodeFromString(jsonFormat.serializersModule.getContextual(Extension::class)!!, extensionJson) | ||
| } catch (e: SerializationException) { | ||
| logger.severe("Error while loading extension meta: ${e.message}") | ||
| null | ||
| } | ||
| }.filter { it.key != null } as Map<Extension, String> |
There was a problem hiding this comment.
Potential NullPointerException from non-null assertion.
The use of getContextual(Extension::class)!! will throw NPE if the Extension serializer isn't registered in the serializersModule.
Apply this diff to add proper error handling:
- jsonFormat.decodeFromString(jsonFormat.serializersModule.getContextual(Extension::class)!!, extensionJson)
+ val serializer = jsonFormat.serializersModule.getContextual(Extension::class)
+ ?: throw IllegalStateException("Extension serializer not found in serializersModule")
+ jsonFormat.decodeFromString(serializer, extensionJson)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @OptIn(ExperimentalSerializationApi::class) | |
| fun readJsonTexts(json: List<String>): Map<Extension, String> = json.associateBy { extensionJson -> | |
| try { | |
| gson.fromJson(extensionJson, Extension::class.java) | |
| } catch (e: JsonSyntaxException) { | |
| jsonFormat.decodeFromString(jsonFormat.serializersModule.getContextual(Extension::class)!!, extensionJson) | |
| } catch (e: SerializationException) { | |
| logger.severe("Error while loading extension meta: ${e.message}") | |
| null | |
| } | |
| }.filter { it.key != null } as Map<Extension, String> | |
| @OptIn(ExperimentalSerializationApi::class) | |
| fun readJsonTexts(json: List<String>): Map<Extension, String> = json.associateBy { extensionJson -> | |
| try { | |
| val serializer = jsonFormat.serializersModule.getContextual(Extension::class) | |
| ?: throw IllegalStateException("Extension serializer not found in serializersModule") | |
| jsonFormat.decodeFromString(serializer, extensionJson) | |
| } catch (e: SerializationException) { | |
| logger.severe("Error while loading extension meta: ${e.message}") | |
| null | |
| } | |
| }.filter { it.key != null } as Map<Extension, String> |
🤖 Prompt for AI Agents
In
engine/engine-loader/src/main/kotlin/com/typewritermc/loader/ExtensionLoader.kt
around lines 39 to 47, the code uses a non-null assertion (!!) on
getContextual(Extension::class), which can cause a NullPointerException if the
serializer is not registered. To fix this, first retrieve the serializer into a
variable and check if it is null. If null, log an error and skip decoding. Only
proceed with decoding if the serializer is present, thus avoiding the NPE and
adding proper error handling.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/DataSerializer.kt (2)
34-34: Address the TODO for serialization module management.The TODO comment indicates a need for better module organization. Consider creating a structured approach for managing serializers, possibly grouping them by domain or type category.
Would you like me to help design a better serialization module structure or open an issue to track this improvement?
83-88: Consider adding validation for cron expressions.The
CronExpression.createDynamic()method may throw exceptions for invalid cron expressions. Consider wrapping the creation in a try-catch to provide more descriptive error messages.String.serializer().xmap( { expression }, - { CronExpression.createDynamic(this) } + { + try { + CronExpression.createDynamic(this) + } catch (e: Exception) { + error("Invalid cron expression '$this': ${e.message}") + } + } ).toDataSerializer()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
engine/engine-core/src/main/kotlin/com/typewritermc/core/utils/point/Position.kt(1 hunks)engine/engine-core/src/main/kotlin/com/typewritermc/core/utils/point/World.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/SkinProperty.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/DataSerializer.kt(1 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/ClosedRangeSerializer.kt(0 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/ColorSerializer.kt(0 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/CoordinateSerializer.kt(0 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/CronExpressionSerializer.kt(0 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/DurationSerializer.kt(0 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/PositionSerializer.kt(0 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/PotionEffectTypeSerializer.kt(0 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/SkinPropertySerializer.kt(0 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/SoundIdSerializer.kt(0 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/SoundSourceSerializer.kt(0 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/VectorSerializer.kt(0 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/WorldSerializer.kt(0 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/utils/BukkitDataParser.kt(2 hunks)engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/utils/Sound.kt(4 hunks)
💤 Files with no reviewable changes (12)
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/DurationSerializer.kt
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/VectorSerializer.kt
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/CronExpressionSerializer.kt
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/ClosedRangeSerializer.kt
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/SkinPropertySerializer.kt
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/PotionEffectTypeSerializer.kt
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/WorldSerializer.kt
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/ColorSerializer.kt
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/SoundSourceSerializer.kt
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/PositionSerializer.kt
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/CoordinateSerializer.kt
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/serializers/SoundIdSerializer.kt
✅ Files skipped from review due to trivial changes (4)
- engine/engine-core/src/main/kotlin/com/typewritermc/core/utils/point/World.kt
- engine/engine-core/src/main/kotlin/com/typewritermc/core/utils/point/Position.kt
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/utils/Sound.kt
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/SkinProperty.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/utils/BukkitDataParser.kt
⏰ 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 (2)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/DataSerializer.kt (2)
65-72: LGTM!The vector serializer implementation is clean and properly maps the x, y, z fields.
73-82: Well-designed nullable field handling.The coordinate serializer effectively handles partial data by using nullable fields with sensible defaults. This provides flexibility for scenarios where not all coordinate components are specified.
| single<DataSerializer<*>>(named("potionEffectType")) { | ||
| String.serializer().xmap<String, PotionEffectType>( | ||
| { key.asString() }, | ||
| { Registry.EFFECT.get(Key.key(this)) ?: error("Unknown potion type '$this'") } | ||
| ).toDataSerializer() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add namespace validation for potion effect keys.
The current implementation assumes the input string is in the correct namespaced format. Consider adding validation or using NamespacedKey.fromString() for more robust parsing.
String.serializer().xmap<String, PotionEffectType>(
{ key.asString() },
- { Registry.EFFECT.get(Key.key(this)) ?: error("Unknown potion type '$this'") }
+ {
+ val key = try {
+ Key.key(this)
+ } catch (e: Exception) {
+ error("Invalid potion effect key format '$this'")
+ }
+ Registry.EFFECT.get(key) ?: error("Unknown potion type '$this'")
+ }
).toDataSerializer()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| single<DataSerializer<*>>(named("potionEffectType")) { | |
| String.serializer().xmap<String, PotionEffectType>( | |
| { key.asString() }, | |
| { Registry.EFFECT.get(Key.key(this)) ?: error("Unknown potion type '$this'") } | |
| ).toDataSerializer() | |
| } | |
| single<DataSerializer<*>>(named("potionEffectType")) { | |
| String.serializer().xmap<String, PotionEffectType>( | |
| { key.asString() }, | |
| { | |
| val key = try { | |
| Key.key(this) | |
| } catch (e: Exception) { | |
| error("Invalid potion effect key format '$this'") | |
| } | |
| Registry.EFFECT.get(key) ?: error("Unknown potion type '$this'") | |
| } | |
| ).toDataSerializer() | |
| } |
🤖 Prompt for AI Agents
In
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/loader/DataSerializer.kt
around lines 47 to 52, the code currently converts strings to PotionEffectType
without validating the namespace format of the keys. To fix this, update the
deserialization logic to parse the string keys using NamespacedKey.fromString()
which validates the namespace format, and handle any parsing errors
appropriately before retrieving the effect from the Registry. This ensures only
valid namespaced keys are accepted and improves robustness.
|
I have the impression that EntityCinematicArtifact still uses Gson |
2 participants
Implement Kotlin Serialization to replace google.Gson.
Allows much more flexibility for final data format, such as CBOR or Protobuf.
Summary by CodeRabbit
New Features
Refactor
Chores