FYI - problem saving modules in recent branches

Development
1

I was just working on a branch that I branched off of the 13197 bugfix branch, and I noticed that every time I try to save a module, even a “new module”, I get a “couldn’t write the file” error and it just leaves the tmp1234123413.zip file.

And I thought “that’s odd, doesn’t seem related to anything I’ve just done”.

So I pulled down “master” itself and tried creating a “new module” and Save-As-ing it – FAIL.

Also the message it puts up is weird, it acts like it can’t write the TMP file even though it looks like it’s failing to rename it to vmod.

So anyway there you have it.

Brian

2

Let’s revert this guy to my original proposal, PR #63 github.com/vassalengine/vassal/pull/63, 1st commit, just put a check around FileUtils.forceDelete(archiveFile).

Everything else worked fine, no need to change the whole thing, I/O code is too difficult to test for us, too many OSes, too many users using network drives, and we don’t have a single unit test for this.

3

Thus spake Flint1b:

[This message has been edited.]

Let’s revert this guy to my original proposal, PR #63
github.com/vassalengine/vassal/pull/63[1], 1st commit, just put
a check around FileUtils.forceDelete(archiveFile).

That’s what I have on the bug13014 branch.

I’m looking at switching all of this out for the ZIP FileSystemProvider,
as that would let us delete quite a lot of fidly code for contenting with
java.util.zip, and I suspect will also fix the bad mtime bug which trips
up tiling.

–
J.

4

Sounds good.

5

I’ve also experienced the same issue with some of the test builds, but figured it wasn’t worth mentioning since they are after all test builds…

6

Thus spake m3tan:

I’ve also experienced the same issue with some of the test builds, but
figured it wasn’t worth mentioning since they are after all test
builds…

I recommend mentioning problems you find in test builds immediately. We
can’t fix problems until we’re aware of them.

–
J.

7

Got it. I’m just starting to toe tap on the developer side of things and had assumed test builds were just to fix/test a single subset of bugs/features. Especially with the appearance of the SNAPSHOT builds, but will be sure to report all bugs going forward.

8

Any time you see something broken that wasn’t broken before is definitely a good time to report it :slight_smile: Test builds often break things that we didn’t notice we were breaking, that didn’t seem related to the thing we thought we were testing. I mean you’re exactly right about what test builds are built for, it’s just that we don’t always immediately notice the “side effects”.

9

You’ll also see quite a few conversations in the forum which go like this:

“I noticed X is broken just now.”

“X has been broken for 5 years.”

“WHAT!? WHY DID NO ONE TELL US?!”

I hate those conversations and would like to have as few of them as
possible. Reporting bugs promptly helps with that.

–
J.

10

Hold on… haven’t read this part before. This sounds a bit like… there’s a german and russian idiom for this: “shooting sparrows with cannons”. And the I/O code is quite sensitive and needs thorough testing on all 3 OSes and in various network storage scenarios… I’d just quick fix this initial “cannot delete what is not there” bug and prioritize further improvements all the way to the bottom of the todo list.