I have a repeatable problem
– No Retreat! module v0.4 – (in development, about to be released)
– Vassal 3.1.16
Sequence
– playing live
– player 1 sends a unit to any of the boxes (e.g., Destroyed box)
– player 1 drags the unit back onto the board
– player 1 sees the unit move
– player 2 does NOT see the unit move – he sees it as remaining in the box
Underlying
– “send to box” is Send To Location to Zone on Selected Map
Yes – both synchronized
– all previous moves were visible to both (e.g., including the original “Send To Location”)
– future moves, of other pieces, are also visible to both
Re-synchronzing does fix the problem of course. But it also very hard for the players to know there was a problem.
I have a repeatable problem
– No Retreat! module v0.4 – (in development, about to be released)
– Vassal 3.1.16
Sequence
– playing live
– player 1 sends a unit to any of the boxes (e.g., Destroyed box)
– player 1 drags the unit back onto the board
– player 1 sees the unit move
– player 2 does NOT see the unit move – he sees it as remaining in
the box
Underlying
– “send to box” is Send To Location to Zone on Selected Map
Is there an older version of VASSAL where the problem doesn’t occur?
I tried it on 3.1.14 – same issue – couldn’t find any earlier versions to try
The above description was NOT quite accurate – it actually requires STACKING to tickle the problem
– No Retreat! module v1.0 – (now online)
– Vassal 3.1.16 or 3.1.14
Sequence
– playing live
– player 1 sends a unit to any of the boxes (e.g., Destroyed box)
– player 1 sends a second unit to the same box
– player 1 expands the stack in the box
– player 1 drags the top unit back onto the board – no problem here
– player 1 drags the remaining unit back onto the board
– player 1 sees the unit move
– player 2 does NOT see the unit move – he sees it as remaining in the box
Notes
– the units are still synchronized – if either player moves it, the “moved” flag appears on their individual views
– if player 1 sends the unit BACK to the box, and then moves it back onto the board, both players see the move correctly
– if player 2 moves the unit out of the box, player 1 does NOT see that move either
Replaying a log file, for this same module, I saw the same thing happen.
– two units are sent to the Destroyed box.
– one is moved there manually.
– the other is moved with Send To Location.
– one is then moved manually on top of the other.
– later they are removed
– the top unit is moved successfully
– the bottom unit is moved (you can see the note in the log), but does not appear to move
– a disorganized counter is created on that unit – it appears in the hex the unit “moved” to
Log file attached. From v1.0 of the No Retreat: The Russian Front module.
Narrowing it down (found the repository of old releases).
This problem appeared in either 3.1.7 or 3.1.8
– not present in 3.1.6
– did not test 3.1.7 (no windows executable in the repository of old releases)
– present in 3.1.8
I think the first thread I linked to is more identical to the behavior you are describing, but as I said the other links may be related and/or similar.
they were supposedly fixed so the changes (should) already be in the later Vassal builds but it is quite possible something got missed.
I believe the appropriate fix is to delete lines 663 and 664 from StackMetrics.java
fixedParent = createStack(fixed, true);
// gs.addPiece(fixedParent);
// fixed.getMap().addPiece(fixedParent);
comm = comm.append(fixedParent.getMap().placeAt(
fixedParent, fixedParent.getPosition()));
index = 1;
}
Reasoning
I believe the intent of the original change was to use placeAt, instead of the code that was already there
fixedParent = createStack(fixed, true);
GameModule.getGameModule().getGameState().addPiece(fixedParent);
fixed.getMap().addPiece(fixedParent);
comm = comm.append(new AddPiece(fixedParent));
index = 1;
But the change to placeAt should have removed the addPiece to the GameState and the addPiece to the map, in additional to doing the placeAt. Reason is that those two commands will be done by the placeAt. Except – since the piece has already been added to the GameState, the “wrong” arm is taken in the placeAt – placeAt treats the stack as if it is being moved instead of being created.
Ok,
Have has a good look at this (I remember it was a nightmare last time) and have realised there is a fairly straightforward solution to this problem.
Instead of trying to mimic what the Map PieceMover does (badly), the movement type traits should just be using the already inbuilt PieceMover that every map has and handles everything associated with moving a piece such as setting the Oldxxxx properties and adding the movement marker if required.
map.placeOrMerge() should not be used as a general purpose piece mover. The real PieceMover takes a completely different code path.
I don’t think Joel has merged it into the trunk yet, but check out my modifications to PieceMover.java under pgeerkens-3.2; I refactored some common code, while making a small change to extend the functionality of Clear Moved Flag on all Pieces. svn7842 I believe. The change to PieveMover and PieceSlot may be independent of everythign else in that check-in.
Hi Pieter,
I have finally tracked down the original problem. It’s in map.placeOrMerge() which is behaving differently to the PieceMover. When a stackable piece is moved to an empty location, map.placeOrMerge() is placing it there as a naked piece. In the same situation, the PieceMover places the piece there in it’s own new Stack of 1. Naked stackable pieces cause problems when other pieces are dropped on them.
Having traits try and run the PieceMover is actually a bit problematic, so I think I will just fix the bug and do a bit of refactoring in the traits.
Hi Pieter,
I have finally tracked down the original problem. It’s in
map.placeOrMerge() which is behaving differently to the PieceMover. When
a stackable piece is moved to an empty location, map.placeOrMerge() is
placing it there as a naked piece. In the same situation, the PieceMover
places the piece there in it’s own new Stack of 1. Naked stackable
pieces cause problems when other pieces are dropped on them.
Having traits try and run the PieceMover is actually a bit problematic,
so I think I will just fix the bug and do a bit of refactoring in the
traits.