Fix/sync initial players #138

twisti-dev · 2025-11-23T12:00:47Z

No description provided.

- Added new netty packets for syncing player data: start, chunk, and end packets for both hydration and large PDC synchronization. - Introduced caching for pending PDCs using Caffeine with a two-minute expiration. - Refactored player server and proxy management APIs to use nullable parameters for enhanced flexibility. - Updated synchronization protocols to include newly added codec streams. - Improved handling logic in packet listeners for hydration and PDC chunks with fail-safe checks. - Refactored existing player creation methods to consolidate logic and improve initialization consistency.

…nchronizeUserTask

- Replaced synchronized map-based queue with `ConcurrentLinkedQueue` for improved concurrency. - Refactored `_connection` setter logic to delegate queue draining to a dedicated method. - Introduced `QueuedPacket` data class for cleaner queue operation. - Simplified `fireAndForget` and `fire` methods to enhance readability and maintainability.

- Renamed `BeforeStartTaskScope` to `SynchronizeTasksScope` for clarity. - Replaced `Dispatchers.IO` with `Dispatchers.Default` in task scope implementation for better task scheduling. - Updated synchronization task executions to use `fireAndForget` for reduced overhead. - Added new synchronization tasks: `SetVelocitySecretTask` and `SynchronizeServersTask` for modularity and reusability.

- Added `SynchronizePlayerPunishmentManagers` task for synchronizing player mute data. - Introduced `ClientboundSynchronizePlayerMutes` packet with supporting `STREAM_CODEC`. - Updated synchronization protocols to include new packet type. - Expanded punishment manager with methods to manage raw cached mutes. - Enhanced server-client packet listeners to handle and process mute synchronization.

… ConcurrentLinkedQueue

Copilot

Pull request overview

This PR refactors the initial player synchronization system to properly sync existing players when a new server connects to the standalone server. The key changes include modifying player manager APIs to use nullable proxy/server names, implementing a new chunked player hydration protocol with support for large persistent data containers, and synchronizing player punishment data (mutes).

Refactored createPlayer API to accept nullable proxyName and serverName parameters instead of a boolean flag
Implemented new player hydration protocol with chunking for large data transfers
Added synchronization tasks for servers, players, and punishment data during initial connection

Reviewed changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
surf-cloud-velocity/.../VelocityCloudPlayerManagerImpl.kt	Updated `createPlayer` signature to use nullable proxy/server names
surf-cloud-bukkit/.../BukkitCloudPlayerManagerImpl.kt	Updated `createPlayer` signature to use nullable proxy/server names
surf-cloud-standalone/.../StandaloneCloudPlayerManagerImpl.kt	Refactored `createPlayer` with improved null handling and removed `serverName` parameter from `removeProxyServer`/`removeServer`
surf-cloud-standalone/.../SynchronizeUserTask.kt	New task implementing chunked player hydration with large PDC support
surf-cloud-standalone/.../SynchronizeServersTask.kt	New task to sync server list during initial connection
surf-cloud-standalone/.../SynchronizeRegistriesTask.kt	Updated to use `fireAndForget` instead of direct connection send
surf-cloud-standalone/.../SynchronizePlayerPunishmentManagers.kt	New task to sync cached player mutes
surf-cloud-standalone/.../SetVelocitySecretTask.kt	Extracted Velocity secret setting into separate task
surf-cloud-standalone/.../ServerSynchronizingPacketListenerImpl.kt	Updated to execute new synchronization tasks in refactored scope
surf-cloud-standalone/.../ByteCodeModifyingURLClassloader.kt	Replaced custom synchronized map with ConcurrentHashMap
surf-cloud-standalone/.../CoroutineManagerImpl.kt	Replaced custom synchronized map with ConcurrentHashMap and used `compute` for atomic operations
surf-cloud-core/.../CloudSynchronizeTaskManager.kt	Renamed scope from BeforeStartTaskScope to SynchronizeTasksScope
surf-cloud-core/.../CloudPlayerPunishmentManagerImpl.kt	Added methods for raw mute access and updating mutes from sync
surf-cloud-core/.../CommonCloudPlayerImpl.kt	Added utility methods to estimate and serialize PDC size
surf-cloud-core/.../CloudPlayerManagerImpl.kt	Updated API signatures and added `createExistingPlayer` helper for hydration
surf-cloud-core/.../SynchronizingProtocols.kt	Registered new player hydration and mute sync packets
surf-cloud-core/.../ClientboundSynchronizePlayerMutes.kt	New packet for syncing player mutes
surf-cloud-core/.../ClientboundSyncPlayerHydration*.kt	New packets for chunked player hydration protocol
surf-cloud-core/.../ClientboundSyncLargePlayerPersistentDataContainer*.kt	New packets for large PDC transfer
surf-cloud-core/.../ClientSynchronizingPacketListener.kt	Added handler methods for new synchronization packets
surf-cloud-core/.../TickablePacketListener.kt	Replaced synchronized list with ConcurrentLinkedQueue and added error handling
surf-cloud-core/.../RespondingPacketSendHandler.kt	Removed synchronization from respondingPackets map
surf-cloud-core/.../ConnectionImpl.kt	Removed extraneous blank line
surf-cloud-core/.../CommonNettyClientImpl.kt	Refactored packet queue to use ConcurrentLinkedQueue with double-check pattern
surf-cloud-core/.../scopes.kt	Renamed BeforeStartTaskScope to SynchronizeTasksScope and changed dispatcher to Default
surf-cloud-core/.../CommonClientCloudPlayerManagerImpl.kt	Updated `removeProxyServer` and `removeServer` to not require `serverName` parameter
surf-cloud-core/.../ClientSynchronizingPacketListenerImpl.kt	Implemented handlers for new player hydration and mute sync packets

Copilot · 2025-11-23T12:05:14Z

...src/main/kotlin/dev/slne/surf/cloud/core/common/netty/network/RespondingPacketSendHandler.kt

    private val log = logger()
    private val respondingPackets =
-        mutableObject2ObjectMapOf<UUID, CompletableDeferred<ResponseNettyPacket>>().synchronize()
+        mutableObject2ObjectMapOf<UUID, CompletableDeferred<ResponseNettyPacket>>()


The respondingPackets map is accessed from multiple methods (handleRead on line 41 and handleWrite on line 65) that can be called concurrently from Netty's event loop threads. The map should be thread-safe. Consider using ConcurrentHashMap instead of the unsynchronized mutableObject2ObjectMapOf.

Suggested change

mutableObject2ObjectMapOf<UUID, CompletableDeferred<ResponseNettyPacket>>()

ConcurrentHashMap<UUID, CompletableDeferred<ResponseNettyPacket>>()

Copilot · 2025-11-23T12:05:14Z

...locity/src/main/kotlin/dev/slne/surf/cloud/velocity/player/VelocityCloudPlayerManagerImpl.kt

+            it.proxyServerName = serverName
+            it.serverName = serverName


The proxy and server names are being swapped. Line 27 should set it.proxyServerName = proxyName and line 28 should set it.serverName = serverName. Currently both are incorrectly set to serverName.

Copilot · 2025-11-23T12:05:14Z

...oud-bukkit/src/main/kotlin/dev/slne/surf/cloud/bukkit/player/BukkitCloudPlayerManagerImpl.kt

-        } else {
-            it.serverName = serverName
-        }
+        it.proxyServerName = serverName


The proxy and server names are being swapped. Line 27 should set it.proxyServerName = proxyName and line 28 should set it.serverName = serverName. Currently both are incorrectly set to serverName.

Suggested change

it.proxyServerName = serverName

it.proxyServerName = proxyName

Copilot · 2025-11-23T12:05:15Z

...ne/src/main/kotlin/dev/slne/surf/cloud/standalone/player/StandaloneCloudPlayerManagerImpl.kt

+                val server = proxyName.toServer() ?: log.atWarning()
+                    .log("Could not find proxy server with id $proxyName")
+                check(server is StandaloneProxyCloudServerImpl) { "Server with id $proxyName is not a proxy server but specified as proxy" }
+                it.proxyServer = server
+            }
+
+            if (serverName != null) {
+                val server = serverName.toServer() ?: log.atWarning()
+                    .log("Could not find server with id $serverName")


The toServer() call returns either a server object or null, but this code uses the elvis operator (?:) with a log statement that returns Unit. This means server will be Unit (not null) when the server is not found, causing the check on line 106 to fail with the wrong type error instead of properly handling the missing server case. The log call should be moved after the null check. Same issue exists on line 111.

Suggested change

val server = proxyName.toServer() ?: log.atWarning()

.log("Could not find proxy server with id $proxyName")

check(server is StandaloneProxyCloudServerImpl) { "Server with id $proxyName is not a proxy server but specified as proxy" }

it.proxyServer = server

}

if (serverName != null) {

val server = serverName.toServer() ?: log.atWarning()

.log("Could not find server with id $serverName")

val server = proxyName.toServer()

if (server == null) {

log.atWarning().log("Could not find proxy server with id $proxyName")

}

check(server is StandaloneProxyCloudServerImpl) { "Server with id $proxyName is not a proxy server but specified as proxy" }

it.proxyServer = server

}

if (serverName != null) {

val server = serverName.toServer()

if (server == null) {

log.atWarning().log("Could not find server with id $serverName")

}

Copilot · 2025-11-23T12:05:15Z

...ne/src/main/kotlin/dev/slne/surf/cloud/standalone/player/StandaloneCloudPlayerManagerImpl.kt

+                val server = proxyName.toServer() ?: log.atWarning()
+                    .log("Could not find proxy server with id $proxyName")
+                check(server is StandaloneProxyCloudServerImpl) { "Server with id $proxyName is not a proxy server but specified as proxy" }
+                it.proxyServer = server
+            }
+
+            if (serverName != null) {
+                val server = serverName.toServer() ?: log.atWarning()
+                    .log("Could not find server with id $serverName")


The toServer() call returns either a server object or null, but this code uses the elvis operator (?:) with a log statement that returns Unit. This means server will be Unit (not null) when the server is not found, causing the check on line 113 to fail with the wrong type error instead of properly handling the missing server case. The log call should be moved after the null check.

Suggested change

val server = proxyName.toServer() ?: log.atWarning()

.log("Could not find proxy server with id $proxyName")

check(server is StandaloneProxyCloudServerImpl) { "Server with id $proxyName is not a proxy server but specified as proxy" }

it.proxyServer = server

}

if (serverName != null) {

val server = serverName.toServer() ?: log.atWarning()

.log("Could not find server with id $serverName")

val server = proxyName.toServer()

if (server == null) {

log.atWarning().log("Could not find proxy server with id $proxyName")

}

check(server is StandaloneProxyCloudServerImpl) { "Server with id $proxyName is not a proxy server but specified as proxy" }

it.proxyServer = server

}

if (serverName != null) {

val server = serverName.toServer()

if (server == null) {

log.atWarning().log("Could not find server with id $serverName")

}

github-actions · 2025-11-23T12:12:36Z

Qodana for JVM

540 new problems were found

Inspection name	Severity	Problems
`Unused symbol`	🔶 Warning	425
`Unused import directive`	🔶 Warning	28
`Unresolved reference in KDoc`	🔶 Warning	27
`Redundant nullable return type`	🔶 Warning	6
`Leaking 'this' in constructor`	🔶 Warning	4
`Redundant 'suspend' modifier`	🔶 Warning	4
`Constant conditions`	🔶 Warning	3
`Control flow with empty body`	🔶 Warning	2
`Possibly blocking call in non-blocking context`	🔶 Warning	1
`Incorrect formatting`	◽️ Notice	21
`Function or property has platform type`	◽️ Notice	7
`Leaking 'this' in constructor`	◽️ Notice	5
`@Deprecated annotation without 'replaceWith' argument`	◽️ Notice	4
`Condition of 'if' expression is constant`	◽️ Notice	1
`Convert 'object' to 'data object'`	◽️ Notice	1
`Redundant empty initializer block`	◽️ Notice	1

☁️ View the detailed Qodana report

Contact Qodana team

Contact us at qodana-support@jetbrains.com

Or via our issue tracker: https://jb.gg/qodana-issue
Or share your feedback: https://jb.gg/qodana-discussions

twisti-dev added 6 commits November 20, 2025 19:42

refactor: remove unnecessary warning log for untransferred PDCs in Sy…

2de6906

…nchronizeUserTask

refactor: replace synchronized collections with ConcurrentHashMap and…

6b982e9

… ConcurrentLinkedQueue

github-project-automation bot added this to surf-cloud Nov 23, 2025

github-project-automation bot moved this to Backlog in surf-cloud Nov 23, 2025

twisti-dev marked this pull request as ready for review November 23, 2025 12:00

twisti-dev requested review from a team and Copilot November 23, 2025 12:00

twisti-dev temporarily deployed to production November 23, 2025 12:00 — with GitHub Actions Inactive

twisti-dev merged commit 2585f77 into master Nov 23, 2025
1 check passed

twisti-dev deleted the fix/sync-initial-players branch November 23, 2025 12:00

github-project-automation bot moved this from Backlog to Done in surf-cloud Nov 23, 2025

Copilot started reviewing on behalf of twisti-dev November 23, 2025 12:01 View session

Copilot finished reviewing on behalf of twisti-dev November 23, 2025 12:04

Copilot AI reviewed Nov 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix/sync initial players #138

Fix/sync initial players #138

Uh oh!

twisti-dev commented Nov 23, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

github-actions bot commented Nov 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	mutableObject2ObjectMapOf<UUID, CompletableDeferred<ResponseNettyPacket>>()
	ConcurrentHashMap<UUID, CompletableDeferred<ResponseNettyPacket>>()

	it.proxyServerName = serverName
	it.proxyServerName = proxyName

Fix/sync initial players #138

Fix/sync initial players #138

Uh oh!

Conversation

twisti-dev commented Nov 23, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 23, 2025

Qodana for JVM

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants