Version/1.21.10 #1
Conversation
…omponents with maintenance-specific implementations
…ts and add logging
…for maintenance mode
There was a problem hiding this comment.
Pull Request Overview
This pull request migrates the project from a template structure (surf-cloud-plugin-template) to a functional maintenance plugin (surf-maintenance), implementing maintenance mode functionality across Velocity proxy and Paper/server platforms, updating the version to 1.21.10, and upgrading the Gradle wrapper to 8.14.3.
- Comprehensive renaming of all modules, packages, and references from template to maintenance
- Implementation of maintenance mode features including server/group toggling, status checking, and player kick functionality
- Upgrade of Gradle wrapper from 8.12 to 8.14.3 and project version to 1.21.10-1.0.0-SNAPSHOT
Reviewed Changes
Copilot reviewed 40 out of 53 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| settings.gradle.kts | Updated root project name and module structure to reflect maintenance plugin (contains duplicate includes) |
| surf-maintenance-velocity/* | New Velocity plugin implementation with maintenance service, listeners, and commands |
| surf-maintenance-server/* | New server-side plugin with configuration management and packet handling |
| surf-maintenance-paper/* | New Paper plugin bootstrap with maintenance mode tracking |
| surf-maintenance-core/* | Core maintenance logic including Spring application, context holder, and network packets |
| surf-maintenance-api/* | API layer defining internal maintenance interfaces and annotations |
| gradle.properties | Updated version to 1.21.10-1.0.0-SNAPSHOT |
| gradle/wrapper/* | Upgraded Gradle wrapper to 8.14.3 |
| build.gradle.kts | Updated surf-api-gradle-plugin dependency to 1.21.10+ |
| README.md | Updated title (description needs update) |
| .gitignore | Simplified to modern IntelliJ IDEA structure |
| .idea/* | Added IDE configuration files for Gradle modules, VCS, and Kotlin |
| surf-cloud-plugin-template-* | Removed all legacy template files |
Files not reviewed (3)
- .idea/gradle.xml: Language not supported
- .idea/kotlinc.xml: Language not supported
- .idea/vcs.xml: Language not supported
Comments suppressed due to low confidence (1)
surf-maintenance-core/src/main/kotlin/dev/slne/surf/maintenance/MaintenanceApplication.kt:6
- The class
MaintenanceApplicationis missing its body braces. While Kotlin allows empty classes to omit the body, this is inconsistent with the previous implementation which had explicit braces. Add{}after the class name for consistency and clarity.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ce-velocity/src/main/kotlin/dev/slne/surf/maintenance/velocity/listener/ProxyPingListener.kt
Outdated
Show resolved
Hide resolved
...city/src/main/kotlin/dev/slne/surf/maintenance/velocity/listener/ProxyConnectionsListener.kt
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 40 out of 53 changed files in this pull request and generated 1 comment.
Files not reviewed (3)
- .idea/gradle.xml: Language not supported
- .idea/kotlinc.xml: Language not supported
- .idea/vcs.xml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var maintenanceMotd: String = "<#6fa8dc> CASTCRAFTER.DE </#6fa8dc><gray>- </gray><red><b>WARTUNGSARBEITEN</red><gray> - </gray><white>[</white><light_purple>1.21.10</light_purple><white>]\n" + | ||
| "</white><#5ACFF5> Weitere Informationen findest du im Discord.", |
There was a problem hiding this comment.
The default maintenance MOTD contains a hardcoded server-specific name 'CASTCRAFTER.DE'. This should be a generic placeholder or template variable to make the configuration reusable across different server deployments without requiring code changes.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 45 changed files in this pull request and generated 8 comments.
Files not reviewed (3)
- .idea/gradle.xml: Language not supported
- .idea/kotlinc.xml: Language not supported
- .idea/vcs.xml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val pluginContainer: PluginContainer, | ||
| val suspendingPluginContainer: SuspendingPluginContainer | ||
| ) { | ||
| var enabled: Boolean = true |
There was a problem hiding this comment.
The enabled property is accessed and modified from multiple places (command handlers, event listeners) without synchronization. In a multi-threaded Velocity environment, this could lead to race conditions where the state changes between check and use. Consider using an AtomicBoolean or proper synchronization for thread-safe access.
| val pluginContainer: PluginContainer, | ||
| val suspendingPluginContainer: SuspendingPluginContainer | ||
| ) { | ||
| var enabled: Boolean = true |
There was a problem hiding this comment.
The maintenance mode state is initialized as true by default, which means the server will start in maintenance mode. This could prevent legitimate access if the configuration file doesn't exist yet or fails to load. Consider initializing to false for safety, or ensure the config loads before this initialization.
| }) | ||
| }) | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
There's trailing whitespace at the end of this line. This should be removed to maintain code cleanliness and avoid unnecessary whitespace in version control.
| fun onProxyPing(event: ProxyPingEvent) { | ||
| event.ping = event.ping.asBuilder().apply { | ||
| description( | ||
| MiniMessage.miniMessage().deserialize(configuration.config.maintenanceMotd) | ||
| ) | ||
|
|
||
| version( | ||
| ServerPing.Version( | ||
| 1, | ||
| LegacyComponentSerializer.legacySection().serialize( | ||
| MiniMessage.miniMessage() | ||
| .deserialize(configuration.config.versionMessage) | ||
| ) | ||
| ) | ||
| ) | ||
| }.build() | ||
| } |
There was a problem hiding this comment.
The ProxyPingListener unconditionally modifies all ping responses regardless of whether maintenance mode is enabled. It should check plugin.enabled before applying the maintenance MOTD and version message, otherwise the maintenance message will always be shown even when maintenance is disabled.
...ce-velocity/src/main/kotlin/dev/slne/surf/maintenance/velocity/command/MaintenanceCommand.kt
Show resolved
Hide resolved
| <option name="linkedExternalProjectsSettings"> | ||
| <GradleProjectSettings> | ||
| <option name="externalProjectPath" value="$PROJECT_DIR$" /> | ||
| <option name="gradleHome" value="C:\Gradle\gradle-8.13" /> |
There was a problem hiding this comment.
The gradleHome path is hardcoded to a Windows-specific absolute path "C:\Gradle\gradle-8.13". This IDE configuration file should not contain user-specific or platform-specific paths as it will not work for other developers or on different operating systems. Consider removing this line or ensuring it's in .gitignore.
| <option name="gradleHome" value="C:\Gradle\gradle-8.13" /> |
|
|
||
| val plugin get() = VelocityMain.instance | ||
| val proxy get() = plugin.proxy | ||
| val configuration = MaintenanceConfiguration() No newline at end of file |
There was a problem hiding this comment.
The global configuration is initialized before the plugin instance is set. Since configuration uses plugin.dataPath in its initialization (line 13 in MaintenanceConfiguration.kt), this may cause a lateinit access error because the assignment instance = this happens after configuration is initialized at the top level. The initialization order should be verified to ensure plugin is available when configuration is created.
| fun onPreConnect(event: LoginEvent) { | ||
| val player = event.player | ||
| if (player.hasPermission(MaintenancePermissions.MAINTENANCE_BYPASS)) { | ||
| return | ||
| } | ||
|
|
||
| event.result = ResultedEvent.ComponentResult.denied(buildText { | ||
| appendDisconnectMessage("DER SERVER BEFINDET SICH IM WARTUNGSMODUS", { | ||
| variableValue("Zurzeit werden Wartungen am Server durchgeführt.") | ||
| appendNewline() | ||
| spacer("Weitere Informationen findest du in unserem Discord.") | ||
| }, { | ||
| append(CommonComponents.RETRY_LATER_FOOTER) | ||
| }) | ||
| }) | ||
| } |
There was a problem hiding this comment.
The ProxyConnectionsListener unconditionally denies all connections for players without bypass permission, regardless of whether maintenance mode is enabled. It should check plugin.enabled before denying connections, otherwise all players without bypass permission will be kicked even when maintenance mode is disabled.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 45 changed files in this pull request and generated 5 comments.
Files not reviewed (3)
- .idea/gradle.xml: Language not supported
- .idea/kotlinc.xml: Language not supported
- .idea/vcs.xml: Language not supported
Comments suppressed due to low confidence (1)
surf-maintenance-velocity/src/main/kotlin/dev/slne/surf/maintenance/velocity/VelocityMain.kt:65
- The configuration is initialized as a top-level property before the plugin instance is fully initialized. This creates a potential initialization order issue where 'configuration.config.enabled' is accessed in loadFromConfig() before the MaintenanceConfiguration constructor may have completed. Consider moving the configuration initialization into the VelocityMain constructor or a proper initialization method.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ce-velocity/src/main/kotlin/dev/slne/surf/maintenance/velocity/command/MaintenanceCommand.kt
Show resolved
Hide resolved
| <option name="gradleHome" value="C:\Gradle\gradle-8.13" /> | ||
| <option name="gradleJvm" value="21" /> | ||
| <option name="modules"> | ||
| <set> |
There was a problem hiding this comment.
The gradleHome path is hardcoded to a Windows-specific path "C:\Gradle\gradle-8.13". This should be removed or changed to use the Gradle wrapper to ensure cross-platform compatibility. Additionally, this references Gradle 8.13 while the wrapper uses 8.14.3, creating a version mismatch.
| <option name="gradleHome" value="C:\Gradle\gradle-8.13" /> | |
| <option name="gradleJvm" value="21" /> | |
| <option name="modules"> | |
| <set> | |
| <option name="gradleJvm" value="21" /> | |
| <option name="modules"> | |
| <set> | |
| <set> |
| }) | ||
| }) | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
There are trailing whitespace characters at the end of this line. Remove the trailing spaces to maintain code cleanliness.
| var enabled: Boolean = false, | ||
| var maintenanceMotd: String = "<#6fa8dc> CASTCRAFTER.DE </#6fa8dc><gray>- </gray><red><b>WARTUNGSARBEITEN</b></red><gray> - </gray><white>[</white><light_purple>1.21.10</light_purple><white>]\n" + | ||
| "</white><#5ACFF5> Weitere Informationen findest du im Discord.", | ||
| var versionMessage: String = "ᴡᴀʀᴛᴜɴɢѕᴀʀʙᴇɪᴛᴇɴ" |
There was a problem hiding this comment.
The word "Wartungѕarbeiten" contains a Cyrillic character 'ѕ' (U+0455) instead of the Latin 's'. This should be corrected to use the standard Latin alphabet: "Wartungsarbeiten".
| appendDisconnectMessage("DER SERVER BEFINDET SICH IM WARTUNGSMODUS", { | ||
| variableValue("Zurzeit werden Wartungen am Server durchgeführt.") | ||
| appendNewline() | ||
| spacer("Weitere Informationen findest du in unserem Discord.") |
There was a problem hiding this comment.
The disconnect message contains hardcoded German text that should be moved to the configuration file for better maintainability and internationalization support. Consider adding these messages to MaintenanceConfig alongside the existing maintenanceMotd and versionMessage fields.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 50 changed files in this pull request and generated 3 comments.
Files not reviewed (3)
- .idea/gradle.xml: Language not supported
- .idea/kotlinc.xml: Language not supported
- .idea/vcs.xml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val eventManager: EventManager, | ||
| suspendingPluginContainer: SuspendingPluginContainer | ||
| ) { | ||
| var enabled: Boolean = true |
There was a problem hiding this comment.
The property enabled is initialized to true by default, but the intended behavior based on the configuration loading logic in loadFromConfig() is to load this value from the config file. This creates an inconsistency where maintenance mode would be active by default before the configuration is loaded during initialization. Consider initializing this to false or to the default value from the configuration class to ensure consistent behavior.
| event.result = ResultedEvent.ComponentResult.denied(buildText { | ||
| appendDisconnectMessage("DER SERVER BEFINDET SICH IM WARTUNGSMODUS", { | ||
| variableValue("Zurzeit werden Wartungen am Server durchgeführt.") | ||
| appendNewline() | ||
| spacer("Weitere Informationen findest du in unserem Discord.") | ||
| }, { | ||
| append(CommonComponents.RETRY_LATER_FOOTER) | ||
| }) |
There was a problem hiding this comment.
The disconnect and kick messages contain hardcoded German text that appears to be server-specific (mentions "unserem Discord"). For a reusable maintenance plugin, these messages should be configurable to allow different servers to customize them according to their needs.
| it.disconnect(buildText { | ||
| appendDisconnectMessage("DER SERVER BEFINDET SICH IM WARTUNGSMODUS", { | ||
| variableValue("Es werden nun Wartungen am Server durchgeführt.") | ||
| appendNewline() | ||
| spacer("Weitere Informationen findest du in unserem Discord.") | ||
| }, { | ||
| append(CommonComponents.RETRY_LATER_FOOTER) | ||
| }) |
There was a problem hiding this comment.
The disconnect message in the kick command contains hardcoded German text that should be configurable, similar to other user-facing messages in the plugin.
This pull request updates the project to use the new
surf-maintenancenaming and structure, removes legacy template code, and upgrades build tooling. The most important changes include renaming the project and modules, cleaning up obsolete files, and updating build configurations.Project renaming and restructuring:
surf-cloud-plugin-templatetosurf-maintenanceinsettings.gradle.kts,README.md, and related include statements. [1] [2].idea/gradle.xmlto reflect the newsurf-maintenancemodules.Cleanup of legacy template files:
build.gradle.ktsfiles and Kotlin source files for the deprecated template modules. [1] [2] [3] [4] [5] [6]Build tooling and configuration updates:
gradle/wrapper/gradle-wrapper.properties.1.21.10-1.0.0-SNAPSHOTingradle.properties..idea/kotlinc.xmland.idea/vcs.xml. [1] [2]