PR for Plane

[pdid]

I’ve read the contributors guide to submiting a PR and I found I should post it hear first. I propose an additional constructor to Plane in the package com.jme3.math that would allow construction of a plane from another common mathematical definition of a plane. The plane is defined by a normal vector and a single point on the plane.

The relevant parts are included here.

@@ -1,5 +1,5 @@
/*

• Copyright © 2009-2012 jMonkeyEngine
• Copyright © 2009-2018 jMonkeyEngine
• All rights reserved.
• Redistribution and use in source and binary forms, with or without
  @ -33,6 +33,7 @@ package com.jme3.math;

import com.jme3.export.*;
import java.io.IOException;
import java.util.Objects;
import java.util.logging.Logger;

/**
@ -92,6 +93,16 @@ public class Plane implements Savable, Cloneable, java.io.Serializable {
this.constant = constant;
}

/**
 * Constructor instantiates a new <code>Plane</code> object.
 *
 * @param normal The normal of the plane.
 * @param displacment A vector representing a point on the plane.
 */
public Plane(Vector3f normal, Vector3f displacment) {
    this(normal, Objects.requireNonNull(displacment).dot(Objects.requireNonNull(normal)));
}

/**
 * <code>setNormal</code> sets the normal of the plane.
 * 

Side Note: Should I @author myself in the java source code?

[nehon]

As a rule of thumb when i’s a simple PR like this one you can directly submit it on github. When it’s a big change with a big feature implemented it’s best to talk about it here first (not posting the code, but discussing the design etc…)

So you can submit your PR on github.

@pdid

Should I @author myself in the java source code?

It’s not necessary, but if you want to go ahead.

Also as a side note, there is a proper way to post code on this forum.

How to type code blocks

Markdown code block : Embed your code in 3 backticks, like this ```<optional language> //your code here ``` If you don’t know how to type a backtick, here are some related links : How to type a backtick on windows For mac users : On qwerty keyboards it’s the key right to the left shift (along with the " key). On azerty keyboards it’s the key over the right shift (along with the £ key). This uses highlight.js, but uses a subset of the supported langages (following code block uses java)…

[Darkchaos]

That’s a really nice addition, much better than having to rotate the plane always by myself!

One thing to note though is that Objects.requireNonNull is not used in the Engine currently, even though it is good style.
Here it is unneccessary though since both would already throw a NullPointerException already (but that’s a detail and depends on the style we want to persue in the engine)

[nehon]

tbh I didn’t even know about the requireNonNull. We could use it though now that the base is java 7.

[javasabr]

I think an engine is a place where we shouldn’t spend CPU time for o lot of null checks.

[danielp]

@javasabr

I think an engine is a place where we shouldn’t spend CPU time for o lot of null checks.

I agree with you. I guess it could be handy for some cases since you can pass a custom error message, but in uses like this it seems a bit overkill - if you get a NPE out of a constructor for a simple object it’s a pretty safe guess that a parameter was invalid, and this is a trivial case to debug.