Cleans up warnings in jme3-desktop #2244
Conversation
| @@ -107,7 +109,7 @@ private JmeCursor loadCursor(InputStream inStream) throws IOException { | |||
| int steps = 0; | |||
| int width = 0; | |||
| int height = 0; | |||
| int flag = 0; // we don't use that. | |||
| //int flag = 0; // we don't use that. | |||
There was a problem hiding this comment.
Why comment out rather than delete?
There was a problem hiding this comment.
Good Question. I have no strong opinion on keeping the comment :)
There was a problem hiding this comment.
Since everything's under source control, code should be commented out only if there's a plausible reason to un-comment it later. And that rationale only works if it's obvious to someone reading the code what effects un-commenting would have.
| * @param main | ||
| * true if this processor is main. | ||
| */ | ||
| public void bind(final Component destination, final Application application, ViewPort viewPort, |
There was a problem hiding this comment.
If these redundant finals were removed it might fit on a single line?
| * @return the new frame transfer. | ||
| */ | ||
| protected AWTComponentRenderer reshapeInThread(final int width, final int height, final boolean fixAspect) { |
There was a problem hiding this comment.
This line was only 112 characters. Surely that can't be too long?
| @@ -158,10 +158,10 @@ private JmeContext newContextLwjgl(AppSettings settings, JmeContext.Type type) { | |||
| return null; | |||
| } | |||
|
|
|||
| @SuppressWarnings("unchecked") | |||
| //@SuppressWarnings("unchecked") | |||
There was a problem hiding this comment.
Should be removed not commented out
| } | ||
| /* | ||
| * public DecodeParams(int bpp, int rm, int rs, int im, int is){ this(bpp, rm, rs, im, is, false); } | ||
| */ |
There was a problem hiding this comment.
Should be removed rather than commented out
| @@ -104,13 +104,21 @@ public DecodeParams(int bpp, int rm, int rs, int im, int is){ | |||
| final int mxxxx = 0xffffffff; | |||
| final int sxxxx = 0; | |||
|
|
|||
| @SuppressWarnings("unused") | |||
There was a problem hiding this comment.
A comment (just in front of the first one) about why these really are used would be good for future understanding
| @@ -87,7 +87,7 @@ public static void generateMipMaps(Image image){ | |||
| BufferedImage original = ImageToAwt.convert(image, false, true, 0); | |||
| int width = original.getWidth(); | |||
| int height = original.getHeight(); | |||
| int level = 0; | |||
| // int level = 0; | |||
| * @return | ||
| */ | ||
| @SuppressWarnings("deprecation") | ||
| public static FrameBuffer getFrameBuffer(int width, int height, int samples) { |
There was a problem hiding this comment.
Should this have @SuppressWarnings("deprecation")? The warnings are valid. More generally; why extract this out into a seperate method (if we want to discourage using these deprecated things we don't want to make reuse easier)
|
Thank you for doing this, a page full of warnings is always a pain |
Fixes buffer creation Auto format
| /* | ||
| * public DecodeParams(int bpp, int rm, int rs, int im, int is){ this(bpp, rm, rs, im, is, false); } | ||
| */ | ||
| @SuppressWarnings("unused") |
There was a problem hiding this comment.
Why is this getting a @SuppressWarnings("unused")? There might be legitimate reasons for this suppression but a comment to explain why the warnings are being supressed would be nice.
(I'm surprised a constructor has unused methods. Sometimes it makes sense for a method if it is overridden and the child class needs them)
| } | ||
|
|
||
| static { | ||
| /* |
There was a problem hiding this comment.
Nice! Thank you for adding this comment
As written in https://hub.jmonkeyengine.org/t/2079-warnings/47608/3 I tried to remove some warnings.
For the raw type warnings, I replaced it with a simple wildcard; maybe I should change some of them to a upper-bound wildcard. Any thoughts on that?
I think this doesn't work while I am writing this.
Ressource Warning in /cursors/plugins/CursorLoader.java was fixed by closing DataInput
the int flag wasn't and what the comment implies shoundn't be used so I removed it and nextInt into "nirvana".
/system/AWTComponentRenderer.java and system/AWTFrameProcessor.java had a deprecated FrameBuffer creation. I added AWTUtils with the function getFrameBuffer(...) and suppressed the warning. Is there a better way to create the buffer?
jme3tools/converters/ImageToAwt.java had some unused "constants" I suppressed the warning. I also removed an unused constructor.