diff --git a/jme3-core/src/main/java/com/jme3/bounding/BoundingBox.java b/jme3-core/src/main/java/com/jme3/bounding/BoundingBox.java index 6b0e023b34..079d96bfe3 100644 --- a/jme3-core/src/main/java/com/jme3/bounding/BoundingBox.java +++ b/jme3-core/src/main/java/com/jme3/bounding/BoundingBox.java @@ -412,7 +412,11 @@ public Plane.Side whichSide(Plane plane) { * altered) * @return this box (with its components modified) or null if the second * volume is of some type other than AABB or Sphere + * @deprecated This method modifies the receiver in place, which is + * inconsistent with {@link BoundingSphere#merge}. Use + * {@link #mergeWith} instead. */ + @Deprecated @Override public BoundingVolume merge(BoundingVolume volume) { return mergeLocal(volume); diff --git a/jme3-core/src/main/java/com/jme3/bounding/BoundingSphere.java b/jme3-core/src/main/java/com/jme3/bounding/BoundingSphere.java index 67cf7263f8..e4dda2287d 100644 --- a/jme3-core/src/main/java/com/jme3/bounding/BoundingSphere.java +++ b/jme3-core/src/main/java/com/jme3/bounding/BoundingSphere.java @@ -465,7 +465,9 @@ public Plane.Side whichSide(Plane plane) { * @param volume * the sphere to combine with this sphere. * @return a new sphere + * @deprecated Use {@link #mergeWith} instead. */ + @Deprecated @Override public BoundingVolume merge(BoundingVolume volume) { if (volume == null) { diff --git a/jme3-core/src/main/java/com/jme3/bounding/BoundingVolume.java b/jme3-core/src/main/java/com/jme3/bounding/BoundingVolume.java index 3a80764910..c9d226c9e0 100644 --- a/jme3-core/src/main/java/com/jme3/bounding/BoundingVolume.java +++ b/jme3-core/src/main/java/com/jme3/bounding/BoundingVolume.java @@ -149,6 +149,18 @@ public final BoundingVolume transform(Transform trans) { */ public abstract void computeFromPoints(FloatBuffer points); + /** + * Combines this bounding volume with a second bounding volume so that the + * result contains both volumes. Returns a new bounding volume without + * modifying either input. + * + * @param volume the volume to combine with this one (may be null) + * @return a new bounding volume containing both volumes + */ + public BoundingVolume mergeWith(BoundingVolume volume) { + return clone(null).mergeLocal(volume); + } + /** * merge combines two bounding volumes into a single bounding * volume that contains both this bounding volume and the parameter volume. @@ -156,7 +168,12 @@ public final BoundingVolume transform(Transform trans) { * @param volume * the volume to combine. * @return the new merged bounding volume. + * @deprecated This method has inconsistent behavior across subclasses + * ({@link BoundingBox#merge} modifies the receiver, while + * {@link BoundingSphere#merge} returns a new instance). + * Use {@link #mergeWith} instead. */ + @Deprecated public abstract BoundingVolume merge(BoundingVolume volume); /** diff --git a/jme3-core/src/test/java/com/jme3/bounding/TestBoundingBox.java b/jme3-core/src/test/java/com/jme3/bounding/TestBoundingBox.java index 7a48e553b6..ed0c7c3408 100644 --- a/jme3-core/src/test/java/com/jme3/bounding/TestBoundingBox.java +++ b/jme3-core/src/test/java/com/jme3/bounding/TestBoundingBox.java @@ -71,6 +71,54 @@ public void testEquals() { Assert.assertEquals(bb5, bb6); // because check planes are ignored } + /** + * Verify that merge() still modifies the original BoundingBox in place + * (deprecated behavior preserved for backward compatibility). + */ + @Test + @SuppressWarnings("deprecation") + public void testMergeModifiesReceiver() { + BoundingBox bb1 = new BoundingBox(new Vector3f(0f, 0f, 0f), 1f, 1f, 1f); + BoundingBox bb2 = new BoundingBox(new Vector3f(4f, 0f, 0f), 1f, 1f, 1f); + + BoundingBox bb1Before = (BoundingBox) bb1.clone(); + + BoundingVolume result = bb1.merge(bb2); + + // merge() delegates to mergeLocal(), so bb1 IS modified. + Assert.assertNotEquals(bb1Before, bb1); + + // The result is the same object as bb1. + Assert.assertSame(bb1, result); + } + + /** + * Verify that mergeWith() does not modify the original bounding box, and + * that the returned box contains both inputs. + */ + @Test + public void testMergeWithDoesNotModifyReceiver() { + BoundingBox bb1 = new BoundingBox(new Vector3f(0f, 0f, 0f), 1f, 1f, 1f); + BoundingBox bb2 = new BoundingBox(new Vector3f(4f, 0f, 0f), 1f, 1f, 1f); + + // Record the original state of bb1 before merging. + BoundingBox bb1Before = (BoundingBox) bb1.clone(); + + BoundingVolume result = bb1.mergeWith(bb2); + + // bb1 must be unmodified. + Assert.assertEquals(bb1Before, bb1); + + // The result must be a different object than bb1. + Assert.assertNotSame(bb1, result); + + // The result must contain both inputs' centers. + Assert.assertTrue(result instanceof BoundingBox); + BoundingBox merged = (BoundingBox) result; + Assert.assertTrue(merged.contains(bb2.getCenter())); + Assert.assertTrue(merged.contains(bb1.getCenter())); + } + /** * Verify that isSimilar() behaves as expected. */