AbstractShadowRender.setRenderBackFacesShadows(): Why use Boolean class to wrap the input value?

[capdevon]

The variable renderBackFacesShadows is of type boolean.

Passing the value null to the AbstractShadowRender.setRenderBackFacesShadows() method will raise an exception. Is this intentional or is it worth changing the method signature by changing the type of the value from Boolean to boolean?

If so, the AbstractShadowFilter.setRenderBackFacesShadows(Boolean bool) method should also be modified.

https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-core/src/main/java/com/jme3/shadow/AbstractShadowRenderer.java#L809

    public void setRenderBackFacesShadows(Boolean renderBackFacesShadows) {
        this.renderBackFacesShadows = renderBackFacesShadows;
        if(renderBackFacesShadows) {
            getPreShadowForcedRenderState().setPolyOffset(5, 3);
            getPreShadowForcedRenderState().setFaceCullMode(RenderState.FaceCullMode.Back);
        }else{
            getPreShadowForcedRenderState().setPolyOffset(0, 0);
            getPreShadowForcedRenderState().setFaceCullMode(RenderState.FaceCullMode.Front);
        }
    }

@scenemax3d @stephengold

[scenemax3d]

public void setRenderBackFacesShadows(Boolean renderBackFacesShadows)

I think you are right and there should be alignment between setRenderBackFacesShadows and the protected renderBackFacesShadows variable. Personally I support breaking the interface and changing the method to accept boolean instead of Boolean but we can also change the implementation for example:

this.renderBackFacesShadows = renderBackFacesShadows == Null ? false : renderBackFacesShadows;

WDYT?

[capdevon]

Hi @scenemax3d , I think it is better not to persist in the error and change the input parameter from Boolean to boolean.

Anyway, even calling the method by passing Boolean.TRUE/FALSE as parameter works the same. So the change Boolean -> boolean is backward compatible with the legacy code.

Here is an example:

public class DirectionalLightShadow {

    private boolean renderBackFacesShadows;

    public boolean isRenderBackFacesShadows() {
        return renderBackFacesShadows;
    }

    public void setRenderBackFacesShadows(boolean renderBackFacesShadows) {
        this.renderBackFacesShadows = renderBackFacesShadows;
    }

    public static void main(String[] args) {
        DirectionalLightShadow s = new DirectionalLightShadow();
        s.setRenderBackFacesShadows(Boolean.FALSE); // it works the same.
    }
}

Another important reason to switch from Boolean -> boolean is the ability to use reflection to automate the generation of visual editors. This is important both for the SDK and for community members who want to write their own editors. I am writing one based on Lemur, and these details are critical for the future development of the engine.

Here is an example:

        BeanInfo beanInfo;
        try {
            beanInfo = Introspector.getBeanInfo(AbstractShadowRender.getClass(), Object.class);
        } catch (IntrospectionException e) {
            throw new RuntimeException(e);
        }
        for (PropertyDescriptor pd : beanInfo.getPropertyDescriptors()) {
            if (pd.getReadMethod() != null && pd.getWriteMethod() != null) {
                // It is important that getter/setter methods respect the type and name of the class variable
            }
        }

I will send you a PR shortly.