Register each batch only once when adding geometry

[toaster0123]

When geometry is added to an existing BatchNode batch, the existing Batch object is reused but also added to the internal list again. Repeated additions therefore leave many entries pointing to the same batch. SimpleBatchNode inherits this behavior.

This change registers a batch only when it is newly created. Reusing its geometry no longer adds another list entry.

In a controlled scene with 128 quads and one material, incremental construction previously left 128 entries for one actual batch. A later batch() call repeated the same physics-ignore metadata write 128 times. Building the same scene in one pass needed one entry and one write. With this fix, both paths use one entry and one write, with the same generated geometry, triangle count and bounds. These are operation counts, not a timing or frame-rate claim.

Six tests cover both node types, identical and equal materials, separate materials, batching with no new geometry, and rebuilding after removal. Four fail before the fix; all six pass afterward.

Validation: core build (including Javadoc), plus desktop, effects and plugins checks passed: 563 tests, no failures, two existing skips. The unchanged baseline test-data JAR was reused locally. The patch was independently reviewed, and all eight focused tests also pass with the tangent-copy fix from #2996 applied.

[jaime-jmebot]

Small nit on the helper — nothing blocking. This builds a brand-new DesktopAssetManager for every material created in the test, which is more setup than these tests need. A plain new Material() (no definition) is enough to get two distinct materials for the "separate materials" case, and it drops the MaterialDef/VarType plumbing entirely.

Minor either way — the current version works and lives in jme3-core (I checked DesktopAssetManager is under com.jme3.asset in core, not jme3-desktop), so there's no layering issue. Just flagging it as a possible simplification.

[jaime-jmebot]

Nice, tightly scoped fix. Two things, one optional:

• Root cause and fix line up. Reusing a Batch via findBatchByMaterial while still unconditionally calling batches.add(batch) is exactly what produces N entries pointing at one batch, and gating that insertion on newBatch removes it at the source. I confirmed SimpleBatchNode.batch() just delegates to super.doBatch(), so it's covered by the same change as described. I also checked that DesktopAssetManager lives in jme3-core (com.jme3.asset), not jme3-desktop, so the test helper isn't reaching into a downstream module.

• One thing worth confirming: does any path set newBatch = false and then drop the batch (e.g. the batch is nulled because its geometry type doesn't match the incoming one) before constructBatch builds a fresh one? If so, that fresh batch would now be constructed but never registered, since the flag is already false. I couldn't read the middle of doBatch() to rule it out. If the reuse branch can't fall through to the construct branch, we're all good — and it may be worth a comment on the invariant, since it's the kind of thing a future edit could quietly break.

• Optional: the test's material() helper builds a fresh DesktopAssetManager per material, which isn't needed for these cases. new Material() would drop the MaterialDef/VarType plumbing.

Tests covering both node types, identical vs. equal vs. separate materials, no-new-geometry batching, and rebuild-after-removal sound like the right spread.

[toaster0123]

Checked the whole doBatch() method at fc7395f. The flag becomes false only after findBatchByMaterial() returns an existing Batch. That branch removes and replaces its geometry, but keeps the same Batch object registered. Both new Batch() calls are in the other branches, so neither can be reached after newBatch becomes false. There is no constructBatch() call or later reassignment of batch in this version.

I kept the material helper because these tests distinguish object identity from equal material contents. Two plain new Material() instances have equal contents here and would be combined. The Value parameter lets the separate-material case use the same definition with different contents, so it tests that boundary too.