Bug 12850

I had a look at the “layers bug”: vassalengine.org/tracker/sho … i?id=12850

Made a simple module for testing layers, it’s attached to the bug. It’s setup as follows:

- two layers, L1 and L2
- three at-start stacks
-- stack "Stack L1" contains 2 pieces that both belong to layer L1
--- piece SL1p1
--- piece SL1p2
-- stack "Stack L2" contains 2 pieces that both belong to layer L2
--- piece SL2p1
--- piece SL2p2
-- stack "Stack Mixed" contains 2 pieces, one belonging to layer L1 and the other to layer L2
--- piece P1 (belongs to L1)
--- piece P2 (belongs to L2)

The Vassal reference manual, page “GamePieceLayers.htm” shows what seems to be one of the most important rules concerning layers: “Pieces assigned to different layers will never combine into a stack.”

The editor allows the creation of at-start stacks with several pieces which belong to different layers. This is a contradiction to the rule that I cited above, but ok, all power to the module designer, I guess.

The module file knows nothing about the stacks belonging to certain layers, it only knows that there are <VASSAL.build.module.map.SetupStack> elements for each at-start stack, and inside them is one <VASSAL.build.widget.PieceSlot> element for each gamepiece. The layer information is saved with the pieces. So far so good.

Opening the module in the player, we get the first “real” bug: vassal opened the module, evaluated the stacks, the pieces inside, it should have realized that this one stack with pieces from two different layers is illegal, and should have created 2 different stacks, one for each layer. It should not look nice, the stacks should sit in the same position on top of each other, but the rule “different layers will never combine into a stack” should be followed, and besides, what was the module designer thinking when he put pieces of several layers into the same stack, it’s his fault that one of the stacks is hidden behind another now.

But instead of this we see just one stack with two pieces/counters, breaking the “different layers will never combine into a stack” rule. What follows, is either due to this initial screwup, or separate bugs, I cannot tell it apart at this point:

  • two buttons to switch off layers L1 / L2, one of them does nothing for the mixed stack, the other switches off the whole stack
  • mouse-over above the mixed stack does not show the popup with the stack’s contents
  • bug description contains more e.g. moving the mixed stack leads to weird results

Another problem described in the bug report is that the layer to which a piece belongs can change over time, this can lead to a piece getting stranded in a stack where it doesn’t belong.

My first idea for a solution is to make sure the rule “different layers will never combine into a stack” is always followed. When a module is opened and the game board is built, the engine should evaluate the layer properties and create several stacks in the same position if needed. Then we can test and see if the problems with “mixed layer stacks” still appear.

The dynamic change of a piece’s layer would also be solved in the same way, the engine should evaluate these layer changes and create new stacks or merge stacks if needed, to ensure that there are never mixed layer stacks in the game.

I did not check where all of this happens in the code yet, just put some breakpoints around LayeredPieceCollection and CompoundPieceCollection and saw them fire when I opened the module and pass the stacks/pieces that I built in the editor, it looked like a good place to start enforcing the rule about no mixed stacks.

I think the first thing that we should agree on is the general direction of the bug fix, does the solution that I proposed make sense from a game modeling point of view? Is it ok to strictly enforce the game rules, and solve possible bugs and weird situations by doing that, even if it means having an editor that allows illegal setups, and graphical weirdness where stacks sit on top of and hide each other?

I think your first idea is correct – make sure that it doesn’t combine different layers into stacks. An “At Start Stack” is generally used more as a way to plop a bunch of units into a spot, with less emphasis on the explicit idea that it’s got to be “as one stack”. So yes, just put things in formal stacks if they’re in the same layer, and otherwise just plop them in the same spot.

Probably in some more ideal future paradigm, an At Start stack would actually run a Send to Location but then apply some known Keystroke (like the apply-on-move keystrokes from maps) so that a piece could respond by e.g. giving itself an offset if desired. But that’s beyond the scope of just fixing the bug so that all the weird behaviors when things from different layers are improperly combined into stacks.

While you could do this, it will be slow on large setups and doesn’t solve the problem where the Layer changes in-game until the next time you save and load the game.

Note that the issue of a layer changing in-game leads to exactly the same problem as creating an at-start stack with pieces from different layers in it. The result, to Vassal, is indistinguishable. So, if you did a game start-up scan for layer, it would fix both the at-start problem and any in-game changes that have developed.

My thought was to add clean-up code to the PieceMover as well, so that if you try an move a stack with mixed layer pieces, clean up the mess cleanly then.

The problem here is that neither Vassal, nor the GamePiece ‘know’ that it’s layer has been changed. For it to change, then the Property holding the layer Id must be a Dynamic Property or a Calculated Property on the pieces in question.

You could possibly identify the Dynamic Property that is controlling the layer (on this map), add a listener and do something when the value changes. But this just won’t work with a Calculated property whose value depends on other remote properties. Calculated Properties don’t have a value that changes, they are re-evaluated when asked for their value.

Thanks for taking the time to explain, this helps a lot. My knowledge of properties and how they are evaluated is very limited, I trust your judgement in this matter. Right now I wouldn’t even know how to dynamically change a layer.

I see these two goals then for now:

  1. When loading a new game, identify mixed layer stacks and unfold them into one stack per layer
  2. When moving a stack, detect when it is a mixed layer stack and also unfold the pieces into one stack per layer

For goal 1. I know roughly where to look how a new game is loaded when a module is opened, will have to figure out how that differs from loading a saved game and where to look to change the saved game loading.

Goal 2., my old friend the PieceMover was already mentioned, I still have this completely refactored version of it from analyzing the previous bug, I’ll figure out something.

Btw, I almost got yet another heart attack when I saw the XML format in which the modules are saved, fully qualified class names of Java classes, with package names and everything, what a poor design desigion, this should have been a DSL that is separate from the naming of Java classes, but as things are it’s enough to move a single class into another package and all modules might break cause of it :laughing: But ok this can still be refactored and the vmod language can be separated from the location of the classes, just a question of writing a parser. Would just look weird having fully qualified class names that are not related to any actual classes as the elements of the module DSL. I understand more and more why a complete rewrite is in order, in its current state the Vassal engine is something like a (very sophisticated) POC built to check the viability of such a product and to get some players hooked, and now you want to take the experience gathered with this POC and build a proper product.

Precisely. It’s an excellent prototype, lot’s of lessons learned, but lots of bad decisions (in hindsight) than can not be reversed.