Preserve buffer positions during binary export - #2821
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates BinaryOutputCapsule to preserve the positions of NIO buffers (FloatBuffer, IntBuffer, ByteBuffer, and ShortBuffer) during serialization by wrapping the write operations in a try-finally block that restores the original position. It also adds a comprehensive unit test suite to verify this behavior. The reviewer suggests a cleaner approach: using absolute get(int index) instead of relative get(), which avoids mutating the buffer's position entirely and eliminates the need for the try-finally blocks.
|
Could a maintainer please approve/run the gated GitHub Actions workflow for this fork PR? If the screenshot comment workflow fails, it appears to be the same infrastructure issue: |
There was a problem hiding this comment.
Pull request overview
This PR fixes BinaryExporter buffer serialization so writing NIO buffers no longer mutates their current positions, aligning binary export behavior with the stated regression case for issue #2312.
Changes:
- Replaces relative buffer reads after
rewind()with absolute indexed reads inBinaryOutputCapsule. - Adds a regression test covering
ByteBuffer,FloatBuffer,IntBuffer, andShortBufferposition preservation and round-trip contents.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
jme3-core/src/plugins/java/com/jme3/export/binary/BinaryOutputCapsule.java |
Preserves caller buffer positions while serializing buffer contents. |
jme3-core/src/test/java/com/jme3/export/binary/BinaryOutputCapsuleBufferPositionTest.java |
Adds regression coverage for buffer position preservation during binary export. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…er-buffer-position
|
Thank you! Nice find and patch 💯 |
* Preserve buffer positions during binary export * Use absolute buffer reads during binary export --------- Co-authored-by: Riccardo Balbo <[email protected]>
Summary
Fixes #2312.
Testing