Highlighting not Changing during Online game

I run the modified module on two clients and connect to the same server. On one client, I run Vassal through a debugger, and I set up break-points in

I change control of one zone on the client that is not running the debugger, and I expect to see a break in VASSAL.build.module.properties.GlobalProperty.decode when the client receives the message from the other client. However, no such break ever occurs (with the right kind of command, that is).

Then I change the control on the client that is running the debugger, and I expect to see a break in VASSAL.build.module.properties.GlobalProperty.encode when the client ships off a message to the other non-debugged client, but that break never occurs.

At no point do I see a break in VASSAL.build.module.properties.ZoneProperty.getPropertyId.

So something fishy is going on. When either client changes a global property, then that change should be propagated to the other clients, but that never seems to happen in this case. How can that be?

It is kinda clear why it works with a Syncronise action, because that means the client requests a full save state from the other client, which will include updates to global properties. But why are global property changes not propagated?

Very weird indeed.

Yours,
Christian

Update (and possible explanation and fix)

Debugging a bit further, I set a break point in VASSAL.build.module.properties.ChangePropertyCommandEncoder.encoder and execute a control change on the debugged client. What I then see is that method is called with a ChangePropertyCommand, that has the container id Map:Southeast Mexico - which also matches the container source id of Map:Southeast Mexico. I think we’re closing in on the culprit. The above container IDs should really be Map:1940 Setup:Southeast Mexico, so that the change goes to the right zone in the right board in the right map.

Doing

[] print container
container = "VASSAL.build.module.map.boardPicker.board.mapgrid.Zone@117e987c"

tells us the source container is a Zone - OK, no surprise. Then

[] print container.getMutablePropertiesContainerId()
 container.getMutablePropertiesContainerId() = "Map:Southeast Mexico"

tells us that the method VASSAL.build.module.map.boardPicker.board.mapgrid.Zone.getMutablePropertiesContainerId is probably wrong. Indeed, if we look at the code, we see

  @Override
  public String getMutablePropertiesContainerId() {
    return (getMap() == null ? "" : getMap().getMapName()) + ":" + getConfigureName();
  }

which should probably be

  @Override
  public String getMutablePropertiesContainerId() {
    return ((getMap() == null ? "" : getMap().getMapName()) + ":"
            (getBoard() == null ? "" : getBoard().getName()) + ":"
              + getConfigureName());
  }

With that, the property would be identified by the full path to the Zone, not skipping the board as is currently done.

However, the above change is, I fear, far from trivial. It will have vast consequences because Vassal with our without that fix will not really be compatible (unless other precautions are taken). On the other hand, this change will not affect saves or logs, because they use a different mechanism to encode and decode property values. I think I will leave it to someone with a better overview of the consequences - c.f. @uckelman - to decide if this fix should be made or not.

If the fix should be made, then it should probably be made now so that it is ready for the upcoming 3.8.0 release.

Incidentally, the above analysis points to a work-around for your problem - albeit a rather tedius one: In your second board, rename all zones so that they do not have the same names as those in your first map.

Yours,
Christian