Skip to content

Conversation

@riccardobl
Copy link
Member

All the old methods are kept but deprecated.

@riccardobl riccardobl merged commit 1b2fc23 into jMonkeyEngine:master Oct 9, 2020
@stephengold stephengold added this to the v3.4.0 milestone Mar 13, 2021
@stephengold
Copy link
Member

There are many places in the codebase where the deprecated methods are still used.

For instance, setDepthBuffer() is used in jme3-core, jme3-desktop, jme3-examples, and jme3-effects. But setDepthTarget() (its replacement) has no javadoc and is only used in a single example (TestMultiRenderTarget).

The shortage of examples (and javadoc) makes it difficult to understand how the replacement methods are meant to be used.

@MeFisto94
Copy link
Member

It's difficult for me to write this paragraph, given that I didn't complain a year ago, but I've been using this API recently and had a few things I'd propose for potential changes.

  1. Remove the FrameBuffer prefix from inner classes, preventing things like FrameBuffer.FrameBufferTextureTarget adding verbosity.
  2. get/set/add: If you can get specific buffers and set used to overwrite the old one, can we maybe add an api to overwrite specific targets (if that is even supported / makes sense?)
  3. FrameBufferTarget.newTarget and the private constructors. When I search for RenderBuffers and find e.g. the FrameBuffer.TextureTarget, then I probably expect to construct it via new TextureTarget, or at the very least with a TextureTargetFactory or -Builder. In this case however, it's not justified to require some complex factory, given that we only look at one simple parameter each. I understand that someone may want a convenience method to pick the right class based on the parameter, so one does not need to call new on the class, but I don't understand preventing regular use.

Now the problem is that this is already part of 3.4, but what do you think of it in general?

@stephengold
Copy link
Member

Is the final question addressed to me, to @riccardobl, or to both of us?

@MeFisto94
Copy link
Member

to everyone of jme. Especially depending on if it's just me or not

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants