Skip to content

Reject BatchNode serialization until runtime state can be restored - #3071

Open
adrian-andrzjewski wants to merge 1 commit into
jMonkeyEngine:masterfrom
adrian-andrzjewski:codex/issue-2600-batchnode-serialization
Open

adrian-andrzjewski wants to merge 1 commit into
jMonkeyEngine:masterfrom
adrian-andrzjewski:codex/issue-2600-batchnode-serialization

Conversation

@adrian-andrzjewski

Copy link
Copy Markdown

Fixes #2600.

Summary

  • Fail fast when BatchNode or SimpleBatchNode is serialized or deserialized.
  • Document the limitation and add regression coverage for both node types.

Root cause

Batch geometries are stored among the node's children, but the runtime batch list and geometry lookup bookkeeping are not serialized. After loading, a saved generated mesh can be mistaken for source geometry when batch() runs again, producing duplicated batch output. These runtime-generated nodes therefore cannot be restored reliably.

Serialization is rejected for unbatched instances too, because a deserializer cannot reliably distinguish logical source geometry from previously generated batch geometry.

Validation

  • :jme3-core:test --tests com.jme3.scene.BatchNodeSerializationTest — passed.
  • Full :jme3-core:test — passed.
  • :jme3-core:check — passed; reported Checkstyle warnings are in unrelated existing files.

@adrian-andrzjewski

Copy link
Copy Markdown
Author

@riccardobl Could you review this fix for #2600? It makes BatchNode and SimpleBatchNode serialization fail clearly because generated batch geometry cannot be distinguished from source geometry after loading. Regression coverage is included for both node types.

@jaime-jmebot jaime-jmebot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Adrian, thanks for tackling this and for tracing the failure back to the missing runtime batch bookkeeping. Rejecting serialization at the BatchNode boundary is a clear and conservative way to prevent both batched and unbatched instances from being restored into an ambiguous state, and the class documentation makes the limitation visible to users of SimpleBatchNode as well.

I reviewed the exporter and importer overrides alongside the regression tests. Since SimpleBatchNode inherits these methods, the same fail-fast behavior covers both node types, and the tests exercise export after batching as well as the read path. The explicit exception also gives callers a much clearer signal than allowing a silently corrupted scene to load. The change looks focused and consistent with the stated limitation; nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BatchNode and SimpleBatchNode serialization do not restore correct runtime state after deserialization

2 participants