Conversation
…sions-on-nserver feat: implement permission-based server argument for network commands
feat: add display name to SurfServer and update connection address ha…
Dev/18 hub command
…nterface and updating related components
…player information
… and adjust related components
…adjust related components
…cketAddress for address field
…odel to use SurfProxyServer
…ers with server updates
…mprove proxy server retrieval
…r-management Dev/24 restructure server management
…pment # Conflicts: # surf-core-core/surf-core-core-common/src/main/kotlin/dev/slne/surf/core/core/common/redis/SendPlayerToServerRequest.kt # surf-core-velocity/src/main/kotlin/dev/slne/surf/core/velocity/api/SurfCoreApiVelocityImpl.kt
…e server handling
There was a problem hiding this comment.
Pull request overview
This PR refactors the server model to distinguish backend servers vs proxy servers (SurfServer vs SurfProxyServer) behind a shared CommonSurfServer interface, and updates Velocity/Paper integrations and commands accordingly (including adding a /hub command and syncing max player changes).
Changes:
- Introduce
CommonSurfServer+SurfProxyServer, migrate APIs/services/serialization to handle proxies separately from backends. - Update Velocity/Paper core implementations and commands to use the new server types (including
/hub, improved send result messaging, and max-players sync). - Adjust Redis storage layout (new keys) and config fields (add
serverDisplayName; move proxy connection address to Velocity config).
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| surf-core-velocity/src/main/kotlin/dev/slne/surf/core/velocity/listener/VelocityServerListener.kt | Switch shutdown player redistribution to proxy-server list/type. |
| surf-core-velocity/src/main/kotlin/dev/slne/surf/core/velocity/listener/ConnectionListener.kt | Track current proxy using SurfProxyServer; update server lookup helper usage. |
| surf-core-velocity/src/main/kotlin/dev/slne/surf/core/velocity/config/VelocityCoreConfig.kt | Add proxy connectionAddress to Velocity config. |
| surf-core-velocity/src/main/kotlin/dev/slne/surf/core/velocity/api/SurfCoreApiVelocityImpl.kt | Update sendPlayer to accept CommonSurfServer and route by subtype. |
| surf-core-velocity/src/main/kotlin/dev/slne/surf/core/velocity/VelocityMain.kt | Register current instance as SurfProxyServer with address from Velocity config. |
| surf-core-paper/src/main/kotlin/dev/slne/surf/core/paper/task/SurfServerInformationSyncTask.kt | New periodic task to update backend maxPlayers in the server service. |
| surf-core-paper/src/main/kotlin/dev/slne/surf/core/paper/permission/PermissionRegistry.kt | Add permission for /hub. |
| surf-core-paper/src/main/kotlin/dev/slne/surf/core/paper/command/NetworkServerMaxPlayersCommand.kt | Update command argument type usage for new server abstractions. |
| surf-core-paper/src/main/kotlin/dev/slne/surf/core/paper/command/NetworkServerCommand.kt | Add awaiting-send behavior and per-server permissions argument. |
| surf-core-paper/src/main/kotlin/dev/slne/surf/core/paper/command/NetworkSendCommand.kt | Add awaiting-send behavior and consolidated multi-send handler. |
| surf-core-paper/src/main/kotlin/dev/slne/surf/core/paper/command/NetworkListCommand.kt | Improve network list output by grouping players by server/proxy. |
| surf-core-paper/src/main/kotlin/dev/slne/surf/core/paper/command/NetworkInformationCommand.kt | Adapt info output to the new server abstraction (no address on backend). |
| surf-core-paper/src/main/kotlin/dev/slne/surf/core/paper/command/HubCommand.kt | New /hub command sending to least-populated lobby backend. |
| surf-core-paper/src/main/kotlin/dev/slne/surf/core/paper/api/SurfCoreApiPaperImpl.kt | Update sendPlayer to accept CommonSurfServer and route by subtype. |
| surf-core-paper/src/main/kotlin/dev/slne/surf/core/paper/PaperMain.kt | Register /hub, start/stop maxPlayers sync task. |
| surf-core-paper/src/main/kotlin/dev/slne/surf/core/paper/PaperBootstrap.kt | Register backend server without a connection address; add display name. |
| surf-core-core/surf-core-core-common/src/main/kotlin/dev/slne/surf/core/core/common/server/SurfServerService.kt | Expand service API to include proxies and accept CommonSurfServer in mutators. |
| surf-core-core/surf-core-core-common/src/main/kotlin/dev/slne/surf/core/core/common/redis/event/SurfServerChangeMaxPlayersRedisEvent.kt | Widen event server type to CommonSurfServer. |
| surf-core-core/surf-core-core-common/src/main/kotlin/dev/slne/surf/core/core/common/redis/SendPlayerToServerRequest.kt | Widen request server type to CommonSurfServer. |
| surf-core-core/surf-core-core-common/src/main/kotlin/dev/slne/surf/core/core/common/config/SurfServerConfig.kt | Add serverDisplayName; remove connection address from common config. |
| surf-core-core/surf-core-core-common/src/main/kotlin/dev/slne/surf/core/core/common/SurfCoreApiImpl.kt | Add proxy getters/listing and “least players” helper. |
| surf-core-backend/src/main/kotlin/dev/slne/surf/core/fallback/service/SurfServerServiceImpl.kt | Split Redis maps for servers vs proxies; change Redis key names. |
| surf-core-backend/src/main/kotlin/dev/slne/surf/core/fallback/service/SurfPlayerServiceImpl.kt | Change Redis key name for cached players; minor access cleanup. |
| surf-core-api/surf-core-api-velocity/src/main/kotlin/dev/slne/surf/core/api/velocity/command/argument/SurfServerArgument.kt | Change argument type to CommonSurfServer. |
| surf-core-api/surf-core-api-velocity/src/main/kotlin/dev/slne/surf/core/api/velocity/command/argument/SurfProxyServerArgument.kt | Change proxy argument to return SurfProxyServer. |
| surf-core-api/surf-core-api-velocity/src/main/kotlin/dev/slne/surf/core/api/velocity/command/argument/SurfBackendServerArgument.kt | Change backend argument type to CommonSurfServer + backend filtering. |
| surf-core-api/surf-core-api-paper/src/main/kotlin/dev/slne/surf/core/api/paper/command/argument/SurfServerArgument.kt | Change argument type to CommonSurfServer. |
| surf-core-api/surf-core-api-paper/src/main/kotlin/dev/slne/surf/core/api/paper/command/argument/SurfProxyServerArgument.kt | Change proxy argument to return SurfProxyServer and suggest proxies. |
| surf-core-api/surf-core-api-paper/src/main/kotlin/dev/slne/surf/core/api/paper/command/argument/SurfBackendServerArgument.kt | Backend-only filtering via isBackend(). |
| surf-core-api/surf-core-api-paper/src/main/kotlin/dev/slne/surf/core/api/paper/command/argument/PermissionSurfServerArgument.kt | New argument enforcing per-server permission gates. |
| surf-core-api/surf-core-api-common/src/main/kotlin/dev/slne/surf/core/api/common/server/SurfServer.kt | Convert backend SurfServer to implement CommonSurfServer and remove proxy fields. |
| surf-core-api/surf-core-api-common/src/main/kotlin/dev/slne/surf/core/api/common/server/SurfProxyServer.kt | New serializable proxy server model with network address. |
| surf-core-api/surf-core-api-common/src/main/kotlin/dev/slne/surf/core/api/common/server/CommonSurfServer.kt | New shared server interface with common ops/helpers. |
| surf-core-api/surf-core-api-common/src/main/kotlin/dev/slne/surf/core/api/common/player/serializer/SurfPlayerSerializer.kt | Deserialize currentProxy via getProxyServerByName. |
| surf-core-api/surf-core-api-common/src/main/kotlin/dev/slne/surf/core/api/common/player/SurfPlayer.kt | Change send() to accept CommonSurfServer; proxy type update + username helper. |
| surf-core-api/surf-core-api-common/src/main/kotlin/dev/slne/surf/core/api/common/SurfCoreApi.kt | Add displayName getter and proxy APIs; widen sendPlayer signature. |
| gradle.properties | Bump project version. |
Comments suppressed due to low confidence (1)
surf-core-api/surf-core-api-paper/src/main/kotlin/dev/slne/surf/core/api/paper/command/argument/SurfServerArgument.kt:26
- This argument is typed as
CommonSurfServer, but it only parses/suggests backend servers (getServerByName/getServers()). Either change the type back toSurfServer(to match behavior) or resolve proxies too (e.g., fall back togetProxyServerByNameand includegetProxies()in suggestions). The current mismatch leads to unreachable proxy-handling code at call sites.
class SurfServerArgument(nodeName: String) :
CustomArgument<CommonSurfServer, String>(StringArgument(nodeName), { info ->
surfCoreApi.getServerByName(info.input)
?: throw CustomArgumentException.fromAdventureComponent(
buildText {
appendErrorPrefix()
error("Der Server wurde nicht gefunden.")
})
}) {
init {
this.replaceSuggestions(
ArgumentSuggestions.stringCollection {
surfCoreApi.getServers().map { it.name }
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| when (target) { | ||
| is SurfProxyServer -> { | ||
| players.forEach { it.send(target) } | ||
|
|
||
| executor.sendText { | ||
| appendSuccessPrefix() | ||
| variableValue(players.size) | ||
| success(" Spieler wurden zum Proxy ") | ||
| variableValue(target.name) |
There was a problem hiding this comment.
handleMultipleSend supports SurfProxyServer, but in this command target comes from surfServerArgument("targetServer"), which only suggests/parses backend servers today. This makes the proxy branch effectively dead and prevents sending groups to proxies. Consider using a unified argument that can return CommonSurfServer for both proxies and backends, or add a separate surfProxyServerArgument path/subcommand for proxy targets.
| permissionSurfServerArgument("server") { | ||
| playerExecutor { player, args -> | ||
| val server: SurfServer by args | ||
| val server: CommonSurfServer by args | ||
| val surfPlayer = player.toSurfPlayer() | ||
|
|
||
| player.sendText { | ||
| appendSuccessPrefix() | ||
| success("Du wirst zum Server ") | ||
| appendInfoPrefix() | ||
| info("Du wirst zum Server ") | ||
| variableValue(server.name) | ||
| success(" gesendet...") | ||
| info(" gesendet...") | ||
| } | ||
|
|
||
| surfPlayer.send(server) | ||
| when (val commonServer = server) { | ||
| is SurfProxyServer -> { | ||
| surfPlayer.send(commonServer) | ||
| } |
There was a problem hiding this comment.
permissionSurfServerArgument("server") only looks up backend servers via surfCoreApi.getServerByName, so the is SurfProxyServer branch here cannot be hit. Either broaden the argument to also resolve proxy servers (and update its suggestions accordingly) or remove the proxy branch/imports to avoid dead code.
| * Requests a change of the maximum player count for a server. | ||
| * Note: This event is only available for backend servers, not for proxy servers. | ||
| * | ||
| * @see SurfServer | ||
| * @see CommonSurfServer | ||
| */ | ||
| @Serializable | ||
| data class SurfServerChangeMaxPlayersRedisEvent( | ||
| val server: SurfServer, | ||
| val server: CommonSurfServer, | ||
| val maxPlayers: Int |
There was a problem hiding this comment.
The KDoc states this event is "only available for backend servers", but the type was widened to CommonSurfServer, which includes proxies. To keep the contract enforceable, consider changing server back to SurfServer (backend) or update the documentation and downstream handlers to explicitly support proxy servers too.
| import kotlinx.serialization.Serializable | ||
| import java.util.* | ||
|
|
||
| object SendPlayerToServerRequest { | ||
| @Serializable | ||
| data class Request(val player: SurfPlayer, val server: SurfServer, val requestId: @Contextual UUID) : RedisRequest() | ||
| data class Request(val player: SurfPlayer, val server: CommonSurfServer, val requestId: @Contextual UUID) : RedisRequest() | ||
|
|
||
| @Serializable | ||
| data object Acknowledged : RedisResponse() |
There was a problem hiding this comment.
SendPlayerToServerRequest now accepts CommonSurfServer, but sendPlayerAwaiting (and the documented behavior) appears to be backend-server-only. Unless the request/handler is intended to support proxy transfers as well, narrowing Request.server back to SurfServer would prevent accidental misuse and keep the API contract clearer.
| val globalServers = redisApi.createSyncMap<String, SurfServer>("surf-core:surf-servers") | ||
| val globalProxies = | ||
| redisApi.createSyncMap<String, SurfProxyServer>("surf-core:surf-server-proxies") |
There was a problem hiding this comment.
The Redis key for servers was changed from surf-core:servers to surf-core:surf-servers (and proxies use a new key). This is an operationally breaking change for existing deployments/rolling upgrades because all nodes must switch keys at once and existing data will be ignored. Consider adding a migration/backward-compat read (e.g., load from old key if new is empty) or explicitly documenting this as a breaking change tied to the version bump.
| val globalPlayers = redisApi.createSyncMap<UUID, SurfPlayer>("surf-core:surf-players") | ||
| override val players get() = globalPlayers.snapshot().values.toObjectSet() | ||
|
|
There was a problem hiding this comment.
The Redis key for players was changed from surf-core:players-new to surf-core:surf-players. This will drop the visible online-player cache during mixed-version rollouts and may leave stale data in the old key. Consider a migration/backward-compat strategy (read old key on startup, write-through for one release, or a documented required full restart) to avoid production inconsistencies.
| surfServerArgument("server") { | ||
| anyExecutor { executor, args -> | ||
| val player: SurfPlayer by args | ||
| val server: SurfServer by args | ||
|
|
||
| player.send(server) | ||
|
|
||
| executor.sendText { | ||
| appendSuccessPrefix() | ||
| success("Der Spieler ") | ||
| variableValue(player.lastKnownName ?: "Unbekannt") | ||
| success(" wurde zum Server ") | ||
| variableValue(server.name) | ||
| success(" gesendet.") | ||
| val server: CommonSurfServer by args | ||
|
|
||
| when (val commonServer = server) { | ||
| is SurfProxyServer -> { | ||
| player.send(commonServer) | ||
|
|
||
| executor.sendText { | ||
| appendSuccessPrefix() |
There was a problem hiding this comment.
surfServerArgument("server") currently resolves only backend servers (it calls surfCoreApi.getServerByName / getServers()), so the is SurfProxyServer branch here is unreachable and users can't target proxies with /nsend player …. Either update the argument to resolve both backends and proxies (e.g., try getServerByName then getProxyServerByName, and suggest from getServers()+getProxies()), or switch this command to use surfProxyServerArgument when proxies are intended.
| @@ -27,7 +27,7 @@ open class SurfServerArgument(nodeName: String) : | |||
| cmdCtx: CommandContext<Source>, | |||
| key: String, | |||
| previousArgs: CommandArguments, | |||
| ): SurfServer = surfCoreApi.getServerByName(StringArgumentType.getString(cmdCtx, key)) | |||
| ): CommonSurfServer = surfCoreApi.getServerByName(StringArgumentType.getString(cmdCtx, key)) | |||
| ?: throw SimpleCommandExceptionType( | |||
There was a problem hiding this comment.
This Velocity SurfServerArgument advertises CommonSurfServer but only resolves backend servers via surfCoreApi.getServerByName. Either make it truly resolve both backends and proxies (try getProxyServerByName when getServerByName is null, and provide suggestions accordingly) or keep it as SurfServer to avoid implying proxy support that isn't there.
No description provided.