Bug in Restricted Access Trait? Or conceptual error?

In this thread http://www.vassalengine.org/forum/viewtopic.php?f=6&t=3618 I reported a problem with the Restricted Access trait and what I thought was an interaction with a custom trait. I’ve been investigating this, and that leads me to this post.

The bottom line: the “restrictByPlayer” flag in Restricted.java (which is controlled with the “Also belongs to initially-placing player” checkbox in the trait properties editor) doesn’t appear to be working properly. Or, more precisely, the logic in isRestricted() appears to be faulty.

if (restrictByPlayer) { restricted = owningPlayer.length() > 0 && !GameModule.getUserId().equals(owningPlayer); }

I created a very simple module, RestrictedTest, which is attached to this post. It defines two player sides, A and B. It contains a single piece, with the following traits:

BasicPiece
TextLabel (with CTRL-L defined as key command to invoke)
RestrictedAccess
Delete

In the RestrictedAccess trait, I checked the “also belongs to initially-placing player” box.

If I run this module and create an online game, with two computers connected, using different passwords, I would expect that the creator of the piece would be able to change the label, and both players would be able to delete the piece. However, both players can access all traits.

I ran VASSAL in a debugger with a breakpoint in isRestricted() at the code snippet above. The other player (non-debugger) created a piece. I clicked on it with the debugger machine, hit the breakpoint, and discovered that the String owningPlayer matched the password of the debugger computer player, not the player that created it. Since both players could access the traits, I surmise that when the piece gets created locally the Restricted trait’s owningPlayer variable is set to the password of the local machine and not the password of the piece’s owner/creator.

If I don’t check “also belongs to initially-placing player” the trait works as expected – but I don’t want to have to assign pieces to individual sides; my module needs players to own pieces that they drag from a common palette.

I haven’t had any time recently to spend on this, but I’m pretty sure this is an engine bug. I just want to confirm that the behavior I’m seeing is NOT the intended/designed behavior:

The Restricted Access trait’s “restrictByPlayer” option (controlled with the “Also belongs to initially-placing player” checkbox in the trait properties editor) doesn’t work. The code in isRestricted() compares owningPlayer to GameModule.getUserId(); however, in my experiments these two values are always equal, because owningPlayer is set by Restricted.setProperty() to be equal to GameModule.getUserId(). It looks to me as though owningPlayer is supposed to be assigned by mySetState() such that when the piece is created it would have the original owner’s user id as its owner; but that isn’t happening.

Am I correct that this is not the intended behavior? If so I can submit a bug report, but I don’t want to do that if I’m exhibiting some kind of gross conceptual error as to how the trait is intended to work.

As a side issue, I note that the player user ids (i.e. passwords) are stored unencrypted. I find this troublesome – if I can see the user’s password by running VASSAL in a debugger, so could anyone else with a modicum of technical ability. I suppose this should be an RFE?