13143 is a blocker for the next VASL release. 13140 and 13136 are confusing for users. 13126 is a severe problem for anyone using a Mac laptop with the trackpad. These represent all the outstanding new problems over 3.2.17 for which we’ve gotten a significant number of reports. 13104 isn’t new, but we seem to be getting more reports of it now. Releasing 3.3.2 with these changes will give us a good release to recommend users return to if the more invasive fixes in master now prove fraught.
I’ve already worked on 13126 and produced a test build addressing one of the problems, but have yet to hear back from anyone who has tested it:
Not sure whether this can go into 3.3.3 or not, but my plans are:
polish the maven story, the next step that converts to a multi-module project is already in a PR, further steps would be to research what exactly is needed to upload the Vassal jar to maven central. This would mean some additional work for you Joel, you would have to create an account at this sonatype nexus server, a set of gpg keys to sign the releases with, possibly more.
look into using jpackage and how we could get Travis configured to build on the three different OSes, there is a very interesting project here github.com/dlemmermann/JPackage … agefx-main that builds jpackage’d releases on different OSes, and Travis offers what they call “matrix builds” that build once on each of the three OSes. This could potentially allow us to build final releases on the CI server.
this here did sound pretty interesting https://forum.vassalengine.org/t/cpu-profiling/10728/1, Joel you think you can dig out what you did there? Just send me the patch if you’re too busy and I’ll take care of packaging it up in a nice PR. Otherwise I’ll just see about implementing the changes that Brent proposed.
work on removing the last calls to closeQuietly(), if it’s too tricky I’ll just write a new closeVerbosely() that tries to close and writes possible exceptions into log.error()
Concur on betas. I’ve also added 13146 - I’ve just identified (and found the specific location of, but not the solution for) a Vassal bug that is creating duplicate buildFile entries for custom classes, where those classes are children of another class (whether the parent is custom or not), whenever the parent’s properties are edited and then the Cancel (!) button pressed. It is probably a good one to check out for 3.3.3. I will make a separate post in the forum with details.
And of course I’d love to get the Chatter in for 3.3.? one of these days.
Off topic. As you are poking around, do you think Vassal has any protocol for handling errors? The reason I mention it is that I see way, way too many (in my opinion) error reports accompanied by traceback dumps, which I think would scare the hell out of new users. I often wonder if it’s possible to enclose those spots in try-catch blocks with a standardized presentation of messages (perhaps in a dialog box), maybe with the traceback dump in a log file. Any thoughts?
Well, it is more or less like a minefield. If you’re lucky, you get a standard dialog pop up and say, “There was an exception, click this button to send a bug report”. Nice for the users. If you’re not so lucky, the error ends up properly in the logfile, with the stack dump and all. Not nice for the users, but still valuable to the developers. And if you’re completely out of luck, the error happens but gets silently ignored, and no one will ever find out what happened.
So yes there is a protocol, this standard dialog is good, but it is not used consistently. Also forking the JVM and starting a whole new subprocess for the Editor or Player is not helpful with collecting exceptions in a single place, presenting a standard dialog and offering to send the error log to the developers.
Off topic. As you are poking around, do you think Vassal has any
protocol for handling errors?
We’ve had that for more than a decade.
(Alternativley, if you think we’re not talking about the same thing,
provide a concrete example.)
The reason I mention it is that I see way,
way too many (in my opinion) error reports accompanied by traceback
dumps, which I think would scare the hell out of new users. I often
wonder if it’s possible to enclose those spots in try-catch blocks with
a standardized presentation of messages (perhaps in a dialog box), maybe
with the traceback dump in a log file. Any thoughts?
That already exists. The users are getting those tracebacks from a dialog
which displays them.
That list is also missing the active PR for 13137 (Drag Threshold - fixed but where drag threshold was effectively one pixel)
and then I filed bug 13146 (Editor duplicating custom classes) today & put a “one-liner” PR up for it.
They’re both “3.3.3” candidates (bugs from 3.2.17 or before, so less urgent and can go in the “beta”)
I don’t currently have any bugs “burning a hole in my brain”, but I LOVE cleaning these things up while we have a critical mass of activity going on.
13117 (mouse wheel scroll rate) is really ready to go right away:
(1) Now uses height of default Jlabel font to calculate scroll rate
(2) Is less complex than the HTMLWindowHelper it inspired, that is already merged.
Can I help isolate where my bug report PR 13154 first got introduced ? I suspect it was one of the early developments that didn’t quite make it into v3.3.1.
If there is a set of builds containing each of the individual components of the release branch (or some accumulation of them), I could work my way through the different builds until I find the 1st one in which I get the problem.
Working through this with my latest version of the C&C Napoleonic module, I haven’t been able to reproduce the fault. However, when I go back to the version that I shared in the bug report, I find that the bug appears in VASSAL-3.3.1-152-g5913c01f-macosx.dmg (dated 2020-06-26 01:01) but not in the previous version, VASSAL-3.3.0-3-g428ea1d1-macosx.dmg, datted 2020-06-22 16:31.
The one change that seems possibly linked (though I can’t see how), seems to me to be this one:
12519: GlobalKeyCommand with Matching Properties does not apply ‘Fixed number of pieces’ to a Deck correctly
I would be interested to test a build with all changes except for this one.
Does the process that is failing use Decks at all as part of the particular automation that is failing? Bug fix 12519 only applies to pieces being moved out of a Deck and the effect of it is to potentially move more pieces out of the Deck, not less.
In your bug report you also say
Was this change made at the same time? Can you check if reverting the change to use the X/Y location again resolves the problem against later builds?
Gave 3.3.2-test1 a rip on Windows 10 tonight, and so far it works beautifully. The latest even fixed some weird “expression evaluation errors” (usually of “old vassal” expressions) that I’d been noticing occasionally in the post 3.3.0 landscape and was thinking I was going to have to go tune up, but apparently now do not. Wife is on some vast multi-hour computer thing tonight, I’ll eventually try to get hold of the Macbook Pro.
I had to prepare the release branch by cherry-picking commits from master. I probably missed a commit. I’ll check later today.
(The major structural changes we did to the repository are between the 3.3.1 tag and the changes which will go into 3.3.2, and there are also changes for 3.3.3 mixed in, which makes fishing out the changes for 3.3.2 a bit of a chore. I hadn’t expected three to be this many changes to master before 3.3.2 was released.)