Skip to content

Absolute value operations on vector classes - #2962

Open
AfonsoDGBatista wants to merge 5 commits into
jMonkeyEngine:masterfrom
AfonsoDGBatista:master
Open

AfonsoDGBatista wants to merge 5 commits into
jMonkeyEngine:masterfrom
AfonsoDGBatista:master

Conversation

@AfonsoDGBatista

Copy link
Copy Markdown

Added abs and absLocal methods on Vector2f, Vector3f and Vector4f classes.

Add methods to compute absolute values of Vector2f components.
Added methods to compute absolute values of vector components.
Added methods to compute absolute values of vector components.
Comment thread jme3-core/src/main/java/com/jme3/math/Vector3f.java Outdated
Comment thread jme3-core/src/main/java/com/jme3/math/Vector4f.java Outdated
y = Math.abs(y);
z = Math.abs(z);
return this;
}

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.

The six new public methods have no test coverage. jme3-core/src/test/java/com/jme3/math/Vector3fTest.java exercises essentially every other public method of Vector3f (including maxLocal, minLocal, negate), so abs()/absLocal() should get matching tests there. Worth asserting the edge cases the existing suite cares about: negative components, -0.0f (→ +0.0f), and NaN propagation. Vector2f/Vector4f have no test classes, so covering the Vector3f variants is the minimum.

@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.

  • The added logic is correct: abs()/absLocal() follow the engine's ...Local naming, and Math.abs maps -0.0f to +0.0f and propagates NaN consistently with the rest of com.jme3.math.
  • Two formatting violations (missing space after a comma) in Vector3f.abs() and Vector4f.abs(); committable one-line fixes are posted inline.
  • No tests for the six new methods. Vector3fTest is the established home for per-method vector tests and should be extended to cover the abs variants, including -0.0f and NaN inputs.
  • Per CONTRIBUTING.md, non-trivial additions are expected to start with a forum thread; link the discussion in the PR description if one exists.

@JNightRider JNightRider 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.

It appears that java.lang.Math is being used. Since FastMath has this same method, shouldn't we take advantage of the FastMath class instead of the Math class?

AfonsoDGBatista and others added 2 commits September 19, 2026 23:20
Added missing space

Co-authored-by: Jaime Bot <jaime@rblb.it>
Added missing spaces

Co-authored-by: Jaime Bot <jaime@rblb.it>
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.

3 participants