v3.3.3-beta1 Restriction check broken

I just started using v3.3.3-beta1, and have discovered that Restriction checking appears to be broken in some way.

To be precise, this code:

{GetProperty($PlayerSide$+"_steelGP")==0}

is now always returning true, so the associated command is always restricted.

This is in the Terraforming Mars module (should be demonstrable with any version currently on the wiki, not just the new version I’m working on.)

Edit: Possibly the same bug as reported here: viewtopic.php?f=3&t=12268#p63483 ?

Looks like all the restrictions using GetProperty($PlayerSide$+"_") are now broken, making the module unplayable in 3.3.3-beta1.

Thanks, I agree, it looks like same bug that m3tan reported, in another guise. Looking into it.

Regards.

I’m not so sure this is the same problem.

Is this the exact property String you are using?

GetProperty($PlayerSide$+"_xyz")

I’m not sure I understand what this is supposed to achieve. This worked before?

So if $PlayerSide$ = German, this gets pre-processed to become

GetProperty(German+"_xyz")

Then, if the value of the Property German is “true”, you get

GetProperty(“true_xyz”)

and read the value of the true_xyz property???

I inherited this code, but yes, it was using GetProperty($PlayerSide$+"_steelGP") to get the property of (for example) “Green_steelGP”, if the current player is “Green”. Should that not have been working? Shouldn’t be too difficult to change them all to GetProperty(PlayerSide+"_steelGP") if you believe that should work, instead.

Overlapping emails.

That should never have worked.

GetProperty(PlayerSide+"_steelGP") is definitely correct for the side will be Green and you want the value of the property Green_steelGP

LOL. I guess VASSAL was just ignoring the “$” signs before? OK, I’ll do a global search-and-replace on the buildFile tomorrow, and hopefully that will fix it. Thanks for looking into it.

Hang on, let me guess. Did they create a Global Property name Green that has the value “Green”? That would would have made it work.

It’s also possible that the $PlayerSide$ was being replaced by PlayerSide.

The fact that you are seeing different behaviour is still concerning and I will look into it, but the the behaviour of $$ variables inside Beanshell was not consistent before and I surprised it worked at all.

Best to avoid them if you can, except in GKC’s.

There is no Global Property named “Green” (or the other 4 player colors), so Beanshell in VASSAL 3.3.2 and below must have been interpreting “$PlayerSide$” as simply “PlayerSide”.

I just did a global search-and-replace to get rid of all the “$”-quoted properties within Beanshell code in the buildFile–there were literally hundreds of them!–and tried the module in VASSAL 3.3.1, and it seems to working fine (except for one case where my search-and-replace accidentally removed a “$” that was used literally, followed by the “$” marking the start of a property in the next Report). I’ll try it in 3.3.3-beta1 again as soon as I’m sure it’s working OK in 3.3.1.

For anyone who’s curious (or needs to do the same), the regex search string I used was: (\{[^\}\$]*)\$([^\$]*)\$ , while the replace string was simply \1\2 .

To avoid clobbering a literal “$”, I believe (\{[^\}\$]*)\$([^\$\}]*)\$ would work, but I haven’t tested it. Now that I think about it, it probably isn’t necessary to escape the special characters within the brackets [], but oh well, it worked anyway!

Ah, that rings a bell and I think explains the other problem as well.

I believe that $PlayerSide$ was indeed being converted to/treated as PlayerSide in 3.3.2 and this is in fact the reason that $$ variables did not work correctly in GKC’s.

When I fixed the GKC issue, I broke this old fall-back handling of $$ variables in non GKC situations.

I will need to meditate on this and come up with a better, compatible solution that works on both situations.

In the meantime, removing the $$ variable references from anything other than GKC usage will definitely help future-proof your module.

Regards.

Okay, verified that the module now works OK in 3.3.3-beta1. Yay!

As far as making 3.3.3 work with the old incorrect syntax the module had been using, I think you would need an option to enable the old behavior, as the new behavior actually makes more sense and might be something people want to actually use!

Perhaps have the option default to the old (buggy) behavior if the module was last saved in version 3.3.2 or earlier, but default to the new behavior for newly created modules?

On the other hand, I have no idea how many other modules besides Terraforming Mars relied on the old behavior, so it may not be worth bothering with.

So in relation to:

Count("{LocationName=="$LocationName$"}")

Is definitely the correct syntax right? Because I also tried:

Count("{LocationName==$LocationName$}") and that seemed to almost work except it didn’t like strings with blank spaces.

Yes, that syntax is correct.

I have found the issue and am building a fix now. Since Sum and Count are basically cut-down GKC’s, there was a bit of extra coding I had to add to make them act like a GKC and handle the $$ variables properly.

Your problem actually turned out to be pretty straight-forward and is not directly related to the one reported by jrwatts which is a more subtle compatibility issue.

That second format is even more dodgy and depending on what is happening with this other issue, that would be interpreted as either

Count("{LocationName==LocationName}")

or

Count("{LocationName==Loc1}")

Where Loc1 is the LocationName value of one of the pieces. Neither of these will do what you want. The first will always be true and the second will be looking for a property whose name is the location of one of the pieces.

Rgds.