Bug ID: 2933236

[jimcot]

This bug is caused by two threads trying to work on the map at the same time. One thread is updating the saved game with the new module definitions (traits for units in this case) while another is trying to draw the map (and the game pieces). I made a fix to delay drawing the map until the update is completed and the game is saved.

I tried updating the bug in sourceforge’s tracker, but wasn’t able to do so.
I’ve not put the patch into subversion yet. I’m not sure if my solution is a good/bad one or if there’s a better solution out there.

Feedback welcome.

[uckelman]

Thus spake “jimcot”:

This bug is caused by two threads trying to work on the map at the same time.
One thread is updating the saved game with the new module definitions (trai
ts for units in this case) while another is trying to draw the map (and the g
ame pieces). I made a fix to delay drawing the map until the update is compl
eted and the game is saved.

This does not surprise me. We’ll systematically remove all such problems
when we carry out my model-view separation project; until then, we have
to fix these things as they turn up.

I tried updating the bug in sourceforge’s tracker, but wasn’t able to do so.

When did you try? I gave you the necessary permissions sometime over the
weekend.

I’ve not put the patch into subversion yet. I’m not sure if my solution is a
good/bad one or if there’s a better solution out there.

Feedback welcome.

Please commit the patch to your branch—otherwise we can’t see what you
did.

–
J.

---

Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

[jimcot]

@uckelman

This does not surprise me. We’ll systematically remove all such problems
when we carry out my model-view separation project; until then, we have
to fix these things as they turn up.

I was thinking that separating loading the save game from displaying it was a better solution. Sounds like that’s already in the work.

When did you try? I gave you the necessary permissions sometime over the
weekend.

I tried Sunday, just before I posted it here.

Please commit the patch to your branch—otherwise we can’t see what you
did.

I will tonite when I get home.

[uckelman]

Thus spake “jimcot”:

I was thinking that separating loading the save game from displaying it was a
better solution. Sounds like that’s already in the work.

It’s planned, but it’s a big job and some ways off (3.3?). If there’s a
straightfoward fix for this problem which can be implemented now, we should
do it.

When did you try? I gave you the necessary permissions sometime over the
weekend.

I tried Sunday, just before I posted it here.

That might have been prior to when I changed your permissions.

Please commit the patch to your branch—otherwise we can’t see what you
did.

I will tonite when I get home.

Looking forward to it.

–
J.

---

Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

[jimcot]

Ok, my patch is in. It simply prevents drawing the map while an update is in progress. Check the branch jimcot-3-1.

[uckelman]

Thus spake “jimcot”:

Ok, my patch is in. It simply prevents drawing the map while an update is in
progress. Check the branch jimcot-3-1.

This looks like it will work. Two comments:

1. GameState.gameUpdating should probably be made volatile, since
   GameState.isUpdating() might be called from a different thread from
   GameState.setup().

2. The new GameState.setup() should be marked in its javadoc as a method
   which isn’t really part of the public API. We don’t want people relying
   on it to exist, since this fix is only temporary, until the model-view
   project is done.

I’ll make those changes and merge to the trunk.

J.

---

Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

[uckelman]

Thus spake Joel Uckelman:

Thus spake “jimcot”:

Ok, my patch is in. It simply prevents drawing the map while an update is

in

progress. Check the branch jimcot-3-1.

This looks like it will work. Two comments:

1. GameState.gameUpdating should probably be made volatile, since
   GameState.isUpdating() might be called from a different thread from
   GameState.setup().

2. The new GameState.setup() should be marked in its javadoc as a method
   which isn’t really part of the public API. We don’t want people relying
   on it to exist, since this fix is only temporary, until the model-view
   project is done.

I’ll make those changes and merge to the trunk.

Changes merged to 3.1@6391,6391.

Would you give this a try, just to confirm that the problem is fixed?

Also, I see a whole bunch of other bugs which might be dupes of this one,
namely: 2817006, 2872381, 2901974, 2905695, 2915112. Could you check those
out? If they appear to be dupes, then they can all me marked as dupes and
closed.

When I close bug reports where the user left contact info, I always email
those people to let them know that the bugs they reported have been resolved.
My form letter looks like this, in the case where we think the bug is fixed:

Hi,

You reported this bug against VASSAL x.y.z:

[link to bug]

Thanks for the bug report. We believe the bug is fixed in our current
development build, x.y.z-svnNNNN, which you can download here:

nomic.net/~uckelman/tmp/vassal/

Please try this, or any later build, and let us know whether it resolves
the problem for you.

(In the present case, the build I’m uploading now is 3.1.14-svn6393.)

–
J.

---

Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

[jimcot]

I reviewed all the bugs you listed, and they do all appear to be the same. I went ahead and closed them all as duplicates.

When I get home tonite, I’ll verify the fix you put in is good.

[jimcot]

Fix appears good in 3.1 branch.

[uckelman]

Thus spake “jimcot”:

Fix appears good in 3.1 branch.

Thanks for verifying. Merged to trunk@6396.

It’s great having somebody working on bugs again… Onward!

–
J.

---

Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)