Custom class version checks

Over the past two weeks, we’ve had 49 bug reports from a popular module which has custom classes which were compiled without ensuring that they were Java 5 compatible. This is frustrating for users and wastes a lot of our time. I notified the maintainer of this module and found that he was unaware that he needed to compile his class files to be Java 5 compatible. (Because he was compiling with Java 7, his class files were failing to load for everyone with Java 5 and Java 6.)

I’d like to add a warning dialog which appears when incompatibly compiled classes are loaded. This will be annoying if you’re using a module which has them—which is the whole point of it. The text I have for the dialog is:

Title: Custom Code Should Be Recompiled

Heading: The Custom Code In This Module Should Be Recompiled

Text: This module contains custom Java code () which was not compiled to be Java 5 compatible. As a result, this module will not run on all versions of Java which VASSAL itself supports. Please contact the maintainer of this module and request that it be fixed.

The idea is that for modules which have the problem now (which I believe to be very few), this will cause the maintainers to get some emails from users asking for the problem to be fixed, and for modules yet to be created, the creators will see this themselves and fix the problem.

What do you guys think?

It’s a good idea but I think you should put the annoyance more on the developer - not the user. In a lot of cases users dont even know who the module developer is or even how to contact them, especially if they dont provide contact info. As such to be sure I would be reaching the right people this pertains to I would propose this:

Whenever the developer goes to bring in their custom class through the “add imported class” selection. Where the Input dialog box pops up change the text from “Enter fully-qualified name of Java class to import” to something like “Enter fully qualified name of Java 5 compiled class to import” or before the input box pops up have a class compile warning dialog box pop up first that they have to hit ok to that will open up the input box.

Furthermore if the developer prefers to go in through the backdoor via the buildfile, in almost everycase they have to do some sort of editing around the <VASSAL.build.module.BasicCommandEncoder/> line.
If it is possible maybe you can build in a comment around this line saying “Make sure your stuff is compiled in Java 5”. That way they’ll always see it

One potential problem I see is that a lot of players don’t actually update their modules.

The message should also suggest they look for an updated version.

Thus spake mkiefte:

One potential problem I see is that a lot of players don’t actually
update their modules.

The message should also suggest they look for an updated version.

Modified text:

This module contains custom Java code () which was not compiled to
be Java 5 compatible. As a result, this module will not run on all
versions of Java which VASSAL itself supports.

Please check whether there is an updated version of this module. If not,
please contact the maintainer of this module and request that it be fixed.

How’s that?


J.

Thus spake Tim M:

[This message has been edited.]

It’s a good idea but I think you should put the annoyance more on the
developer - not the user. In a lot of cases users dont even know who the
module developer is or even how to contact them, especially if they dont
provide contact info. As such to be sure I would be reaching the right
people this pertains to I would propose this:

I’m conflicted about this. One one hand, I see your point—some users
may have no recourse. My reason for asking the user to notify the module
maintainer is that it encourages the problem to be self-correcting for
modules with enough users that we get a significant number of bug
reports. It’s a way to turn an email from us to the module maintainer—
which would be easy to ignore—into some pressure from users.

Whenever the developer goes to bring in their custom class through the
“add imported class” selection. Where the Input dialog box pops up
change the text from “Enter fully-qualified name of Java class to
import” to something like “Enter fully qualified name of Java 5 compiled
class to import” or before the input box pops up have a class compile
warning dialog box pop up first that they have to hit ok to that will
open up the input box.

Hmm. What would you think about refusing to import incompatible classes
in the editor, instead of giving a warning but doing it anyway?

Furthermore if the developer prefers to go in through the backdoor via
the buildfile, in almost everycase they have to do some sort of editing
around the <VASSAL.build.module.BasicCommandEncoder/> line.
If it is possible maybe you can build in a comment around this line
saying “Make sure your stuff is compiled in Java 5”. That way they’ll
always see it

Adding a comment would be easy, I think.


J.

I’ve looked into adding a class version check which happens when at class is imported in the Editor. Unfortunately, doing this is quite awkward because the place where class importation happens, ConfigureTree.importConfigurable(), is relying on the DataArchive’s implementation of ClassLoader. The only place you have the bytes which comprise the class file is inside one of the ClassLoader methods, and you have to have the bytes at offsets 5-8 in the class file to get the version information. The Class class doesn’t provide any mechanism for getting them. So, we never have access to the bytes comprising the class file inside importConfigurable(), nor do we even know where to look for them from there—it’s all hidden by the way that ClassLoaders work.

I think this means that a check at import time is not feasible without some invasive restructuring.

What do you guys think in light of that—introduce the check I originallly proposed, or not?

It was bother you most. I haven’t looked at what kinds of exceptions are being generated but I hadn’t heard anything from the TS module.

Thus spake mkiefte:

It was bother you most.

Eh? Sorry, English is not my strong suit…

I haven’t looked at what kinds of exceptions are being generated but I
hadn’t heard anything from the TS module.

There haven’t been any class version exceptions from the TS module
recently. (IIRC, you classes are compiled with Java 6, so would only
give trouble to the small number of people running Java 5 still.)


J.

Go with the original. Seeing as we can’t check the files on import we could still target the Designers albeit without any verification - simply change the Right click/Dialog text the designer sees from “import custom class” to “import java5 custom class” - better than nothing, maybe they see it, maybe they don’t, might help a little

I think the message to the users would be good. The import custom class warning in the editor does not catch people that place the classes manually in the zip and edit the build file with a text editor.

The module which prompted my original question—a popular one which we don’t host—is fixed now.

I wrote a script to check the versions of class files in the modules we host. I found that the current versions of the following modules have class files compiled for Java 6:

A Victory Denied
Karelia '44
Malifaux
Red Star Rising
Star Fleet Battle Force
Star Trek: Expeditions
Star Trek: Fleet Captains
The Devil’s Cauldron
1914: Twilight in the East
Waterloo (AH)
OCS Baltic Gap
OCS The Blitzkrieg Legend
OCS Case Blue/Guderian’s Blitzkrieg II
OCS DAK2
OCS Korea
OCS Tunisia
OCS Reluctant Enemies

This is few enough that we could contact the maintainer of each module and ask that a new version be released with compatible class files, giving them a bit of time to do it before we do a release which adds the class check. This way should minimize the impact of the change on everyone involved.

So, my plan is, today:

  • Release 3.2.13 without the class check.
  • Notify the maintainers of these modules to fix them.

Then, sometime in the future:

  • Release 3.2.14 with the class check.

Thanks, everyone, for commenting on this. It’s important that we get right things which could affect so many users.

I’ve emailed the maintainers of all of these modules except Karelia '44 (for which the module maintainer left no email address) and 1914: Twilight in the East (which still has George Hayward, who died last year, listed as the maintainer).

Email to the maintainer of OCS Reluctant Enemies bounced.