Repository navigation
Continue Bedrock compatibility improvements - #25
Merged
Merged
Conversation
Private replay cleanup removes installed mods, so hashing their former paths after a capture cannot establish which JARs were staged. Record filenames, byte counts, and SHA-256 hashes before launching clients and transport processes. Keep the receipts private and refuse replacement of existing evidence. Cover source replacement, dependency cleanup, missing inputs, and evidence overwrite refusal. Preserve normal dependency cleanup and capture manifest compatibility.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 49 minutes. View limit details
 |
| Layer / File(s) | Summary |
|---|---|
Gameplay readiness and capture timing src/replay/java/net/pistonmaster/stackanvil/replay/RecordingReadiness.java, src/replay/fabric/.../FabricRecorder.java, src/replay/java/.../RecordingPlugin.java, src/replay/cli.ts, src/replay/java/.../RecordingReadinessSelfTest.java, docs/capture-lab.md |
The shared readiness tracker marks gameplay ready after serverbound initialization followed by player input. The Fabric recorder and proxy plugin report recorded packets to the tracker. The CLI waits for readiness before timing Java captures. Self-tests cover packet ordering and readiness state. |
Artifact evidence and CLI receipts src/replay/artifact-evidence.ts, src/replay/cli.ts, test/replay-artifact-evidence.test.ts, docs/capture-lab.md |
The evidence writer records artifact filenames, byte counts, and SHA-256 hashes in schema-1 receipts. The CLI writes receipts for sorted client JARs and for the ViaProxy JAR and recorder plugin. Tests cover file handling and concurrent publication. Documentation describes receipt timing and verification. |
ViaProxy patch updates
| Layer / File(s) | Summary |
|---|---|
Native binding update and lifecycle checks patches/viaproxy/series.json, patches/viaproxy/upstreamable/0002-update-native-binding-to-avoid-exit-deadlocks.patch, patches/viaproxy/upstreamable/0002-update-native-binding-to-avoid-exit-deadlocks.pr.md, src/replay/java/net/pistonmaster/stackanvil/replay/NativeShutdownSelfTest.java, src/replay/cli.ts |
The ViaProxy patch updates libdatachannel dependency selection. A self-test exits a child JVM during an active native callback. The patch description reports selected Linux lifecycle checks and states that gameplay success was not established. |
ViaProxy patch metadata and dependencies patches/viaproxy/setup/0001-stackanvil-branding.patch, patches/viaproxy/upstreamable/0001-use-shared-http-signaling-for-bds-connections.patch |
The branding patch groups RakNet transport with Netty dependencies. The HTTP-signaling patch records updated commit and build metadata. |
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix
Merge Risk: 
44754
-
A Java replay capture that never reaches gameplay readiness can still be reported as successful after the 20-minute wait, even though the requested scene timer never started. The case is narrow and only affects capture tooling, but adding a rejection check after the wait loop is a small fix.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title accurately identifies the continued Bedrock compatibility work, which is a substantial part of the changes. It does not mention the replay timing and artifact evidence changes, but the title… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title accurately identifies the continued Bedrock compatibility work, which is a substantial part of the changes. It does not mention the replay timing and artifact evidence changes, but the title need not cover every detail.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 13 functions across 8 files. (6 skipped: 6 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
CodeRabbit Verify
- Run CodeRabbit Verify to test the changes in this pull request.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Proxy recordings previously counted resource loading toward the requested gameplay duration. Share the connection-owned readiness observer with the direct recorder and start both Java scene timers only after initialization and subsequent player input. Cover wrong-direction and premature input, repeated readiness, and separate connection ownership. Installed Lifeboat captures retain over twenty seconds of admitted gameplay on both routes and exit normally. The earlier native-library shutdown race remains unresolved.
Pin the Java facade and native bundle to the same verified October publication. Correct the transitive facade exclusion so the proxy cannot package older Java classes beside newer JNI binaries. Fold the capture dependency relocation into its setup patch so the native update applies independently. Add a forked-JVM loopback regression that exits during an active native callback. It reproduces the old bundle hang and passes with the matching update. Full and standalone builds, transport self-tests, and Linux JDK 17 and 25 controls pass. The installed join hit HTTP 503 before gameplay; cancellation stopped every owned process normally.
Write each private receipt in a staging directory and hard-link the completed file into place. A failed write cannot leave partial final evidence, and concurrent publishers cannot replace the winner. Remove staging files after publication or failure. Verify concurrent publication, existing receipt preservation, private modes, missing dependencies, and retained staged hashes. All 289 tooling tests and bun run check pass.
Reject Java setup timeouts when the scene clock never starts, after saving diagnostic logs and a screenshot. The existing finally block still stops owned processes and releases private dependencies. Derive gameplay acknowledgment from ordered serverbound initialization and subsequent movement, including transfer journals. Packet counts alone admitted input sent before initialization. Regression fixtures cover packet order, direction, both movement protocols, and incomplete sequences; recorded direct and proxy controls remain accepted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replay sessions retain private artifact receipts after dependency cleanup. Each receipt records staged JAR filenames, sizes, and SHA-256 hashes. Complete receipts publish exclusively, so interrupted writes cannot leave partial final evidence or overwrite another attempt.
Both Java routes start the requested scene duration after local-player initialization and the first subsequent input packet. Resource loading no longer consumes the requested gameplay duration. The shared observer rejects premature input, wrong-direction packets, and initialized state from another connection. Java captures fail if their scene timer never starts. Journal and transfer validation also require movement after initialization, rather than unordered packet counts.
ViaProxy now uses matching immutable Java and native libdatachannel artifacts from October 29. The previous bundle can deadlock during JVM exit with an active native callback. The updated upstream C API gives its registries process lifetime. Correct dependency exclusions prevent an older Java facade from accompanying the new JNI binaries.
Validation:
bun run checkpasses.Continue the existing compatibility goal across movement, inventory, native screens, cameras, and rendering, including skin parity and unresolved live reports.
Summary by CodeRabbit