search

NearInfinityBrowser / NearInfinity

An Infinity Engine Browser and Editor
https://github.com/NearInfinityBrowser/NearInfinity/wiki
GNU Lesser General Public License v2.1
87 stars 37 forks source link

Update source code to Java 6 standard #29

Closed Argent77 closed 11 years ago

Argent77 commented 11 years ago

Many lines of deprecated code can be updated without breaking compatibility with Java 6 (or even Java 5). This would increase type safety (and possibly performance).

FredSRichardson commented 11 years ago

I found that there were errors in the code when using 1.6 - I had to move to 1.7 (Java 7).

Argent77 commented 11 years ago

If you are hinting at the errors in ResourceStructure.java. They are already fixed in the devel branch.

FredrikLindgren commented 11 years ago

Don't be too concerned with Java 5 compatibility. NI already uses a class introduced in Java 6 (java.awt.Desktop, in BrowserMenuBar). We should keep track of NI's dependencies [1], and it's nice if we don't break compatibility with Java 5 needlessly, but it's two versions obsolete and is starting to show its age.

  1. Perhaps we should set up a wiki page for documentation of dependencies?
FredSRichardson commented 11 years ago

Actually, what I saw was Java 6 incompatibility. It doesn't bother me too much, but we probably should decide on what minimum version we're compatible with.

On Sun, Sep 1, 2013 at 9:08 AM, Fredrik Lindgren aka Wisp < notifications@github.com> wrote:

Don't be too concerned with Java 5 compatibility. NI already uses a class introduced in Java 6 (java.awt.Desktop, in BrowserMenuBar). We should keep track of NI's dependencies [1], and it's nice if we don't break compatibility with Java 5 needlessly, but it's two versions obsolete and is starting to show its age.

  1. Perhaps we should set up a wiki page for documentation of dependencies?

— Reply to this email directly or view it on GitHubhttps://github.com/NearInfinityBrowser/NearInfinity/issues/29#issuecomment-23624558 .

FredSRichardson commented 11 years ago

Oh, there is a wiki on GitHub we could link back to from wherever.

On Sun, Sep 1, 2013 at 9:33 AM, Richardson, Fred frichard@ann-fred.orgwrote:

Actually, what I saw was Java 6 incompatibility. It doesn't bother me too much, but we probably should decide on what minimum version we're compatible with.

On Sun, Sep 1, 2013 at 9:08 AM, Fredrik Lindgren aka Wisp < notifications@github.com> wrote:

Don't be too concerned with Java 5 compatibility. NI already uses a class introduced in Java 6 (java.awt.Desktop, in BrowserMenuBar). We should keep track of NI's dependencies [1], and it's nice if we don't break compatibility with Java 5 needlessly, but it's two versions obsolete and is starting to show its age.

  1. Perhaps we should set up a wiki page for documentation of dependencies?

— Reply to this email directly or view it on GitHubhttps://github.com/NearInfinityBrowser/NearInfinity/issues/29#issuecomment-23624558 .

FredrikLindgren commented 11 years ago

but we probably should decide on what minimum version we're compatible with.

Currently that is effectively Java 6 and I'd like to preserve that for the time being.

I'll also set up an instance of Java 6 for regression testing and build production.

FredSRichardson commented 11 years ago

Okay, well the code compiles under Java 7 (1.7), but there are several failures with Java 7 (1.7) in "infinity.util.MassExporter". These have to do with JList not being a generic under 1.6

On Sun, Sep 1, 2013 at 10:03 AM, Fredrik Lindgren aka Wisp < notifications@github.com> wrote:

but we probably should decide on what minimum version we're compatible with.

Currently that is effectively Java 6 and I'd like to preserve that for the time being.

— Reply to this email directly or view it on GitHubhttps://github.com/NearInfinityBrowser/NearInfinity/issues/29#issuecomment-23625416 .

Argent77 commented 11 years ago

Hmm, it looks like NI needs to be compiled with a Java 7 compiler in compiler compliance setting 1.6. However, I can execute the resulting JAR just fine in my test VM with JRE 1.6u21 installed.

I'm not really sure what's the root of the problem, eclipse or my compiler setup. Wisp, do you have additional insights?

FredSRichardson commented 11 years ago

I think it's reasonable for us to pick and dictate a development environment including the java version, and then determine what compatibility level the jar has.

Standardizing on 1.7 for development seems reasonable to me.

I assume (or hope) that the compiler warns when it can't create 1.6 compatible byte code. Generics are probably an easy one since it's almost a preprocessing hack.

On Sun, Sep 1, 2013 at 12:20 PM, Argent77 notifications@github.com wrote:

Hmm, it looks like NI needs to be compiled with a Java 7 compiler in compiler compliance setting 1.6. However, I can execute the resulting JAR just fine in my test VM with JRE 1.6u21 installed.

I'm not really sure what's the root of the problem, eclipse or my compiler set up. Wisp, do you have additional insights?

— Reply to this email directly or view it on GitHubhttps://github.com/NearInfinityBrowser/NearInfinity/issues/29#issuecomment-23627930 .

FredrikLindgren commented 11 years ago

Well, NI currently does not build on Java 6. Additionally, you apparently can't specify a -target version that lower than the -source version. Unless anyone has a good idea, it seems like we will need to remove the 8 cases where the code depends on Java 7.

Output from javac as attachment, because github eats the angle brackets: link

Edit: or, obviously, abandon Java 6 as a supported platform.

FredSRichardson commented 11 years ago

Right, that makes more sense. Only three files are effected. It's not hard to avoid this in the future, just set the java compiler in Eclipse under Windows > Preferences > Java > Installed JREs (you guys probably know that already).

On Sun, Sep 1, 2013 at 1:53 PM, Fredrik Lindgren aka Wisp < notifications@github.com> wrote:

Well, NI currently does not build on Java 6. Additionally, you apparently can't specify a -target version that lower than the -source version. Unless anyone has a good idea, it seems like we will need to remove the 8 cases where the code depends on Java 7.

Output from javac as attachment, because github eats the angle brackets: link https://dl.dropboxusercontent.com/u/78949477/javac16.txt

— Reply to this email directly or view it on GitHubhttps://github.com/NearInfinityBrowser/NearInfinity/issues/29#issuecomment-23629564 .

Argent77 commented 11 years ago

I've fixed the issue. There are still lots of (over 200) compiler warnings about raw types and other stuff, but fixing those would probably take a lot of time.

Commit: https://github.com/Argent77/NearInfinity/commit/2d9d17eca743740ab9b84758d3fa66f32b42e33c

Argent77 commented 11 years ago

Updated many lines of code to the Java 6 standard. This should add a bit of type safety and remove the majority of warning messages. Most of the NWN/KOTOR stuff has been left out since it's been deprecated anyway. The compiler still generates many warning messages, since arrays of generics aren't properly supported by Java yet. I'd suggest to change the respective types from arrays into Collections, but that's a separate issue.

I have also updated the proprietary IntegerHashMap and LongIntegerHashMap. The classes are now based on the official java.util.HashMap class.

Commit: https://github.com/Argent77/NearInfinity/commit/25ff749ee79fa6d49c05024e67a5bea5e6a1b914

FredrikLindgren commented 11 years ago

Good stuff. Perhaps we should split the remaining stuff up into suitably sized chunks, so we can close them off as they get done, rather than having this issue haunt us all the while?