-
Notifications
You must be signed in to change notification settings - Fork 0
feat: introduce persistent player data tracking and binary tag handling #113
Conversation
- Added `TrackingPlayerPersistentDataContainerImpl` for player data tracking with patch-based updates. - Implemented binary tag codecs (`BINARY_TAG_CODEC`, `BINARY_TAG_CODEC_COMPRESSED`) for NBT serialization. - Integrated persistent data container updates with `ClientboundUpdatePlayerPersistentDataContainerPacket`. - Enhanced persistent data handling in `StandaloneCloudPlayerImpl` with tracking and broadcast logic. - Updated `PersistentPlayerDataContainerImpl` with utility methods for patch operations (`applyOps`, `getPatchOps`). - Added error handling for failed data fetches in `onNetworkConnect`. - Optimized collection codec constructors in `ByteBufCodecs` for size constraints and performance.
…ctional updates - Removed `TrackingPlayerPersistentDataContainerImpl` from `standalone` module and consolidated logic in `core`. - Replaced `withPersistentData` with `editPdc` for non-suspending data modification and deprecated the former. - Introduced snapshots in `PersistentPlayerDataContainer` for safe concurrent edits. - Added bidirectional `UpdatePlayerPersistentDataContainerPacket` and streamlined packet handling. - Improved player data synchronization with patch-based updates and lock mechanisms.
Qodana for JVM513 new problems were found
☁️ View the detailed Qodana report Contact Qodana teamContact us at qodana-support@jetbrains.com
|
- Updated `RespondingNettyPacket` to utilize weak references for response connections, enhancing memory management. - Introduced `overwritePpdc` in `ClientCloudPlayerImpl` for efficient persistent player data updates. - Enhanced `PlayerConnectedToServerPacket` and `PlayerConnectToServerPacket` with `StreamCodec` for seamless serialization. - Refined `PrePlayerJoinTask.Result` to support clean serialization and type distinction. - Updated netty protocol to version `2` and adjusted related codec registrations for consistency.
- Updated `ppdcReentrantLock` initialization in `CommonCloudPlayerImpl` to use fair locking, improving thread coordination.
- Replaced `ByteBufInputStream` and `ByteBufOutputStream` with `FastBufferedInputStream` and `FastBufferedOutputStream` for improved performance. - Added GZIP compression support for `BINARY_TAG_CODEC_COMPRESSED`.
- Introduced `BinaryTagTypeProxy` to dynamically handle `BinaryTagType` mappings, removing static `TAG_TYPES`. - Replaced static tag type resolution with `ByIdMap.continuous` using the proxy for better flexibility and error handling. - Enhanced `ByIdMap` with additional out-of-bounds strategies, including `DECODE_ERROR` and custom error factories.
- Removed `ServerboundPlayerPersistentDataContainerUpdatePacket` and its associated logic for simplified player data updates. - Reorganized test plugin structure: - Added `TestSpringApplication` in `test-core` module for Spring-based test applications. - Introduced `TestPpdcCommand` to enhance PPDC testing capabilities within `standalone` and `paper` modules. - Updated `RunningProtocols` to streamline persistent data request handling and improve codec consistency. - Enhanced shutdown handling with improved player disconnect management during network termination.
- Added a non-recursive `deepCopy` method to safely handle deeply nested `CompoundBinaryTag` structures. - Updated `snapshot` to utilize the new deep copy logic for better reliability and stack safety.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a comprehensive refactoring of the Persistent Player Data Container (PPDC) system to support real-time synchronization across the network. The key changes include:
- Replaced the request/response pattern for PPDC updates with a patch-based bidirectional synchronization system
- Changed from
KClassto Java'sClassfor reflection APIs throughout the codebase - Made primitive setters in PPDC accept nullable values for convenience
- Added new test plugin modules to validate PPDC functionality
- Incremented protocol version from 1 to 2 due to breaking network changes
Reviewed Changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| TestPpdcCommand.kt (standalone) | New console command to test PPDC operations on standalone server |
| TestPpdcCommand.kt (paper) | New Paper command to test PPDC operations |
| TestStandaloneBootstrap.kt | Refactored to use shared TestSpringApplication |
| TestSpringApplication.kt | New shared Spring application context for test plugins |
| PpdcTestExecutor.kt | Executor for PPDC test operations (read/write test data) |
| PpdcTest.kt | Core PPDC test implementation with test data model |
| PdcPatch.kt | New patch representation for PPDC synchronization |
| PdcOp.kt | Operations (Put/Remove) for PPDC patches |
| TrackingPlayerPersistentDataContainerImpl.kt | New tracking container that generates patches for changes |
| PersistentPlayerDataContainerImpl.kt | Refactored to support patch application and nullable setters |
| UpdatePlayerPersistentDataContainerPacket.kt | New bidirectional packet for PPDC updates |
| StandaloneCloudPlayerImpl.kt | Implements new PPDC synchronization with patches |
| ClientCloudPlayerImpl.kt | Client-side PPDC patch application |
| CommonCloudPlayerImpl.kt | Base implementation for PPDC with tracking support |
| PluginSpringConfig.kt | Registers console command processor bean for plugins |
| settings.gradle.kts | Includes new test-core and test-paper modules |
.../main/kotlin/dev/slne/surf/cloud/core/common/player/ppdc/PersistentPlayerDataTypeRegistry.kt
Show resolved
Hide resolved
.../kotlin/dev/slne/surf/cloud/core/common/player/ppdc/PersistentPlayerDataContainerViewImpl.kt
Show resolved
Hide resolved
...ne/src/main/kotlin/dev/slne/surf/cloud/standalone/player/StandaloneCloudPlayerManagerImpl.kt
Show resolved
Hide resolved
...-common/src/main/kotlin/dev/slne/surf/cloud/api/common/netty/packet/RespondingNettyPacket.kt
Show resolved
Hide resolved
...main/kotlin/dev/slne/surf/cloud/core/common/player/ppdc/PersistentPlayerDataContainerImpl.kt
Outdated
Show resolved
Hide resolved
...ev/slne/surf/cloud/core/common/netty/network/protocol/running/PlayerConnectToServerPacket.kt
Show resolved
Hide resolved
...tandalone/src/main/kotlin/dev/slne/surf/cloud/standalone/player/StandaloneCloudPlayerImpl.kt
Show resolved
Hide resolved
...i/surf-cloud-api-common/src/main/kotlin/dev/slne/surf/cloud/api/common/player/CloudPlayer.kt
Outdated
Show resolved
Hide resolved
- Set linter to `jetbrains/qodana-jvm:2024.3`. - Added `exclude` paths for specific directories. - Introduced `IncorrectFormatting` entry in `include`.
Co-authored-by: twisti-dev <76837088+twisti-dev@users.noreply.github.com>
Co-authored-by: twisti-dev <76837088+twisti-dev@users.noreply.github.com>
Co-authored-by: twisti-dev <76837088+twisti-dev@users.noreply.github.com>
- Replaced `tag.fast()` with `snapshotTag().fast()` in snapshot methods of player data containers.
…her-one Fix race condition in PersistentPlayerDataContainerView.snapshot()
Replace Stack with ArrayDeque in pruneEmptyAncestors
…ainerView - Moved `MAX_NESTING_DEPTH` and related logic to `PersistentPlayerDataContainerView` for consistent enforcement across components. - Updated `PdcOp` and data iteration logic to utilize the centralized validation method.
Add ArrayDeque rationale and nesting depth limit to deep copy
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 51 out of 51 changed files in this pull request and generated 3 comments.
.../main/kotlin/dev/slne/surf/cloud/api/common/player/ppdc/PersistentPlayerDataContainerView.kt
Show resolved
Hide resolved
.../kotlin/dev/slne/surf/cloud/core/common/player/ppdc/PersistentPlayerDataContainerViewImpl.kt
Show resolved
Hide resolved
...-common/src/main/kotlin/dev/slne/surf/cloud/api/common/netty/packet/RespondingNettyPacket.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
...core-common/src/main/kotlin/dev/slne/surf/cloud/core/common/player/CloudPlayerManagerImpl.kt
Show resolved
Hide resolved
|
@twisti-dev I've opened a new pull request, #125, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@twisti-dev I've opened a new pull request, #126, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: twisti-dev <76837088+twisti-dev@users.noreply.github.com>
- Modified `response` to use lazy initialization with `CompletableDeferred` for proper memory handling. - Ensure exception completion if response is accessed after connection garbage collection.
docs: add KDoc to ensureValidNestingDepth function
TrackingPlayerPersistentDataContainerImplfor player data tracking with patch-based updates.BINARY_TAG_CODEC,BINARY_TAG_CODEC_COMPRESSED) for NBT serialization.ClientboundUpdatePlayerPersistentDataContainerPacket.StandaloneCloudPlayerImplwith tracking and broadcast logic.PersistentPlayerDataContainerImplwith utility methods for patch operations (applyOps,getPatchOps).onNetworkConnect.ByteBufCodecsfor size constraints and performance.