Fix BoundingBox.merge() inconsistency — deprecate merge() and introduce mergeWith() (#2661)

* Initial plan

* Fix BoundingBox.merge() to not modify receiver in place

Agent-Logs-Url: https://github.com/jMonkeyEngine/jmonkeyengine/sessions/e27f3192-530d-4525-aa2a-ad3e611ae4ed

Co-authored-by: riccardobl <4943530+riccardobl@users.noreply.github.com>

* Deprecate merge(), restore original BoundingBox.merge() behavior, add mergeWith()

Agent-Logs-Url: https://github.com/jMonkeyEngine/jmonkeyengine/sessions/223ff945-a733-4b7b-8b8b-2e90418270de

Co-authored-by: riccardobl <4943530+riccardobl@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: riccardobl <4943530+riccardobl@users.noreply.github.com>

Author: Copilot

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);

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) {

jme3-core/src/main/java/com/jme3/bounding/BoundingVolume.java

@@ -149,14 +149,31 @@ 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);
+    }
+
     /**
      * <code>merge</code> combines two bounding volumes into a single bounding
      * volume that contains both this bounding volume and the parameter volume.
      *
      * @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);
 
     /**

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.
      */