Created attachment 3559 [details]
see post of around 10 february 2012 on the forum

See forum thread:
http://www.vassalengine.org/forum/viewtopic.php?f=3&t=4905

Global property changes on zones, maps, and the game module are not correctly transferred through the server.  It appears that ChangePropertyCommandEncoder encodes the original command as a string which consists only of the property name, old value, and new value. Then the decoder (GameModule.decode) goes through each of the registered ChangePropertyCommandEncoders (which consist of every zone, map, and module) until it finds one that successfully sets the property, then terminates.  I may be missing something, but I cannot find any encoding of the MutablePropertyContainer which actually contained the original property - furthermore, my tests suggest that the ChangePropertyCommand will, in fact, be executed on the first MutablePropertyContainer which contains a property with the same name - first zones, then maps, then the module.

Suggested fix: Add getID() method to interface MutablePropertiesContainer, encode with the ChangePropertyCommand, and check for a match upon decoding.  Maps can reference the map name; zones should reference both map name and zone name.

-Seth

http://www.vassalengine.org/forum/viewtopic.php?f=3&t=4905

> Suggested fix: Add getID() method to interface MutablePropertiesContainer,
> encode with the ChangePropertyCommand, and check for a match upon decoding. 
> Maps can reference the map name; zones should reference both map name and zone
> name.

Seth, I think you have diagnosed the problem correctly, along with the correct
way to approach the solution.

One thing we have to be careful of is to ensure compatibility with existing
logfiles. Best way to do this is probably to implement a new Command prefix in
ChangePropertyCommandEncoder 

protected static final String COMMAND_PREFIX2 = "MutableProperty2\t";

to handle encoding with the correct Id, and leave support for the original
Command prefix in decode().

Had a quick look tonight and there are quite a few implementations of
MutablePropertiesContainer, but its not too bad. Also something strange in that
Map implements MutablePropertiesContainer, but also has a separate
MutablePropertiesContainer defined. 

I have some time to look at it over this weekend unless you want to have a go?

Brent.

Brent/Seth:

I'm not adept at Java but if there is any way I can help while you guys work on it this weekend, let me know.

-Bob

Brent-3.2@8216 has a fix for this problem.

Same named properties in different property containers where not having the identity of the container encoded in SetPropertyCommand or ChangePropertyCommand and their associated CommandEncoders. 

This meant that all references to the same named property in different containers where handled by the first found reference to that property name.

Merged to trunk@8218.

Merged to 3.1@8223.