Vassal 3.4.xyz errors/warnings

[barbanera]

All of a sudden I am getting reports of modules which had no issues whatsoever for years that are now throwing errors/warnings all over the place when players take certain actions. For example Talon, Pax Transhumanity etc, which were working fine with 3.2.17

This is definitively something related to Vassal 3.3.xx and then 3.4.xx. I am very grateful and happy that development resumed on Vassal 3 after three years of freeze. However, I suspect that the recent flurry of patches and patches of patches is either breaking new things or, more likely, breaking workarounds we module developers had to implement to bypass Vassal 3.2 limits/hydiosincracies/bugs.

I am not sure what could be done about this, except of course having all developers review all their modules and make sure they work with the latest Vassal (when there is a stable version that lasts more than a week, I guess).

Perhaps the engine could automatically display huge warnings when somebody attempts to open a 3.2.17 or earlier module with a more recent version of the engine that they should really download 3.2.17 or earlier? Or even better default back to 3.2.17 behaviour, for back compatibility sake.

[Brent_Easton]

Could you please be a bit more specific? Exactly which errors are being displayed and how do you trigger them? I can’t advise you without knowing what the problem is, and after 5 minutes of playing with the Talon module not knowing what I was doing, I could not generate any errors.

In general, a module that works under 3.2.17 and does not contain custom code should continue to work unchanged in Vassal 3.4, except in a couple of very specific cases. Without more information, I can’t advise you.

I am not sure what could be done about this, except of course having all developers review all their modules and make sure they work with the latest Vassal (when there is a stable version that lasts more than a week, I guess).

Well, instead of suffering in silence and then letting rip with pointless snarky posts that don’t describe your problems, you could supply accurate and detailed information about exactly what your problems are, including Vassal versions, OS type, Log file extracts, screen shots, copies of error text, reproduction instructions, etc. so that we can work out whether it is a bug in 3.4, or some other issue, and work out a plan to resolve the issue. This approach has worked well for other module developers who have had problems with 3.4

Regards.

[barbanera]

Well, I apologise if I sounded snarky.

I repeat that I am grateful that development of Vassal 3 has restarted and we are getting old bug fixed, new features etc. However, things are quite clearly also getting broken in the process, as a) I see a flurry of releases with patches of patches (or perhaps I am halucinating) and b) I am getting reports about errors that don’t show with 3.2.17 in my modules.

I can sure provide details (for a starter please see this thread about Talon issues: boardgamegeek.com/thread/223221 … 7#36446637 ) but my point was more like a general point: could it be possible to default modules saved with Vassal 3.2.17, say, to default to 3.2.17 even if running 3.4.11? I suspect not, but I am no expert.

P.S. I sure plan to update my modules removing whatever odd workaround I was using (that was implicitly deprecated, I guess), once 3.4 is stable. But in the meanwhie backward compatibility would be a plus, if at all possible.

[uckelman]

Thus spake barbanera:

I repeat that I am grateful that development of Vassal 3 has restarted
and we are getting old bug fixed, new features etc. However, things are
quite clearly also getting broken in the process, as a) I see a flurry
of releases with patches of patches (or perhaps I am halucinating) and
b) I am getting reports about errors that don’t show with 3.2.17 in my
modules.

The vast majority of changes during 3.4 have been fixes to bugs which
are either very old or very new. We fixed many, many bugs which long
predate 3.2.17, and we also fixed some bugs which existed nowhere but
one or two 3.4 releases. The former aren’t indicative of new problems,
but rather that we had the manpower to fix them in the past few months.
The latter largely cover changes to things which were new in 3.4, so
you’d obviously never encounter them in 3.2.17.

Bundling up more changes into fewer releases makes it harder for us
to pinpoint when a problem started, which is another reason for more
frequent, smaller releases.

I can sure provide details (for a starter please see this thread about
Talon issues: boardgamegeek.com/thread/223221 … 7#36446637[1]
) but my point was more like a general point: could it be possible to
default modules saved with Vassal 3.2.17, say, to default to 3.2.17 even
if running 3.4.11? I suspect not, but I am no expert.

We’d rather put effort into ensuring modules work with the current version.

There’s no way we can fix problems no one tells us about, so if you’re
having a problem with the current version of VASSAL, let us know so we can
look into it.

J.

[Brent_Easton]

We don’t have the manpower to monitor every sub forum of every gaming site on the internet. Unless issues are brought to our attention on the Vassal forums, they will generally not be seen by the Vassal developers.

I’ve looked at that thread and it still means nothing to me as I don’t know the module and it seems to cover a number of issues. However, by blundering around I managed to reproduce what I believe is the problem being described.

Looking at your buildfile, I believe the problem you are seeing are due to an issue where $$ variables like $OldLocation$ have been indiscriminately mixed into Beanshell expressions with an incorrect syntax. This worked by chance in version 3.2, but was never meant to work and was not supported by any documentation. A number on inter-related bugs and issues required that this be fixed and unfortunately there, was no way to do this without ‘breaking’ the old way of it working.

Essentially, where you have expressions that look like this:

{If(TalonController==$PlayerName$,"Talon",If(TerranController==$PlayerName$,"Terran","n\/a"))}

they need to be changed to look like this:

{If(TalonController=="$PlayerName$","Talon",If(TerranController=="$PlayerName$","Terran","n\/a"))}

or

{If(TalonController==PlayerName,"Talon",If(TerranController==PlayerName,"Terran","n\/a"))}

will also work for most expressions except for Global Key Command expressions where you want the $$ expressions pre-evaluated on the source piece.

This ONLY applies to the use of $xxx$ variables inside the Beanshell {} braces. They MUST be surrounded by double quotation marks.
There are earlier discussions about this on the forum, but I can’t put my finger on them at the moment. Other uses of $$ variables, in Report Formats for example, do not require the double quotes.

In the short term, you can document your module page with a message recommending that players use Vassal 3.2.17 for the module. There is no problem with this.

While we release updates to Vassal regularly, there is generally no need for you to upgrade immediately as each one is released unless you are specifically affected by one of the bugs addressed in that release.

Regards.

[shilinski]

This ONLY applies to the use of $xxx$ variables inside the Beanshell {} braces. They MUST be surrounded by double quotation marks.

But not if the property resolves to an integer. No double quotes needed.

Also, the property definition (e.g. marker trait) must appear in the trait list above the beanshell check that uses it. The beanshell won’t see it if it comes later. This issue drove me nuts last night.

[Brent_Easton]

But not if the property resolves to an integer. No double quotes needed.

But double quoting an integer won’t cause problems.

Also, the property definition (e.g. marker trait) must appear in the trait list above the beanshell check that uses it. The beanshell won’t see it if it comes later. This issue drove me nuts last night.

In what context Stan? This will be dependent on the Trait that holds the Beanshell expression and in general, they should search for properties starting at the bottom-most trait in the piece and work up. If a particular trait is not doing this, I need to know.

Thx.

[shilinski]

But not if the property resolves to an integer. No double quotes needed.
But double quoting an integer won’t cause problems.

The point I’m making is the quotes aren’t there I think for the $$. They are there to tell Beanshell that the stuff replacing the $$ property is a STRING. I would think if Beanshell didn’t see a string (quotes) or an integer, then it would interpret what’s there as a property, effectively double-dereferencing what’s between the $$.

Also, the property definition (e.g. marker trait) must appear in the trait list above the beanshell check that uses it. The beanshell won’t see it if it comes later. This issue drove me nuts last night.

In what context Stan? This will be dependent on the Trait that holds the Beanshell expression and in general, they should search for properties starting at the bottom-most trait in the piece and work up. If a particular trait is not doing this, I need to know.

I almost wrote a separate new thread about this last night because I didn’t think where markers were in the trait list mattered, but instead I went to bed. Here’s a simplified scenario: Suppose I have 10 “culture” pieces scattered on the map. Each culture piece has a marker to uniquely identify it (e.g. culture=1, culture=5, etc.)

I also have a deck of “goods” pieces belonging to the cultures, and as I draw them, each one is suppose to go to its matching culture piece. For example a “3” goods should get sent to the culture 3 piece.

In each “goods” piece, I have the following 2 traits (greatly simplified example):
marker: myCulture = (3, for example)
send to location: another counter selected by properties, {culture == $myCulture$}
This code works. Goods 3 went to culture 3.

But of course, it’s not what I first coded. Instead, I had the lines reversed in order. i.e.:
send to location (to a culture piece as above)
marker: myCulture =
This did NOT work. I think the myCulture property is undefined at send-to-location time. I had to move the myCulture definition higher up in the trait list to get it to work.

In my real case, I had a dozen goods go to their cultures, but instead they all vanished. After some frantic searching, I found them huddled and stacked under a bigger playing card like bugs under a leaf.
An interesting feature. So the question I asked myself then is am I coding to spec or am I coding to behavior? I suspect behavior, but what do I know.

Stan

[Brent_Easton]

The point I’m making is the quotes aren’t there I think for the $$. They are there to tell Beanshell that the stuff replacing the $$ property is a STRING. I would think if Beanshell didn’t see a string (quotes) or an integer, then it would interpret what’s there as a property, effectively double-dereferencing what’s between the $$.

Yes, correct, we are in agreement. My point was just that if you don’t want to have to worry about whether or not the result of the $xxx$ will return an integer, just always use quotes and you will be fine.

{hereX == "$CurrentX$" }

will work just as well as

{hereX == $CurrentX$ }

when CurrentX returns an integer. You don’t have to remove the quotes just because the CurrentX returns an integer.

But of course, it’s not what I first coded. Instead, I had the lines reversed in order. i.e.:
send to location (to a culture piece as above)
marker: myCulture =
This did NOT work. I think the myCulture property is undefined at send-to-location time. I had to move the myCulture definition higher up in the trait list to get it to work.

This is a bug in SendToLocation. Pretty much WHENEVER you have to move traits around to make exposed properties visible, it is a bug in the trait that is evaluating the expression. In this case SendToLocation.

It’s a 2 line fix I will submit for 3.4.12 and 3.5.

Regards.

[barbanera]

@Brent Easton

We don’t have the manpower to monitor every sub forum of every gaming site on the internet. Unless issues are brought to our attention on the Vassal forums, they will generally not be seen by the Vassal developers.

I just linked to that thread, on the BGG website, for you to kindly peruse since you were asking for specific issues rather than a “pointless” rant. I thought it was easier and quicker to read about the (recent) issue with the Talon module there, rather then copy and paste the same content here.

I never ever meant to imply the Vassal developers should monitor the WWW, dark web, milnet, decnet, arpanet etc for threads about all obscure modules out there.

Looking at your buildfile, I believe the problem you are seeing …

Thanks for taking the time to look at the buildfile and pinpoint what’s most likely an issue (of many?) with that module that I will need to address. This definitively saved me a lot of time head scratching about it.

However, again, my point was not asking for the developers to find the bugs (or deprecated coding) hiding in my modules, but to make a general point that backward compatibility is apparently not 100% guaranteed with every new release of Vassal.

I think that’s a fact. I also think this fact is not made clear to casual users - those who just update the engine and try to run an older module with it - and perhaps it should.

Like a big splash screen saying “WARNING: you are attempting to run a module that was developed with an older version of the module (3.2.17) and backward compatibility is not 100% guaranteed. If you notice any issues we suggest you run this module under version 3.2.17 of the engine before contacting the module developer and warning him about the issue.”

Or something much shorter to that effect. In the meanwhile, sure, I will add disclaimers to all wiki pages for my modules to check and use 3.2.17 first. Till I have time to check them one by one and track down any unexpected malfunction, that is.

[JoelCFC25]

If you don’t have any custom Java code in your module (and it looks like you don’t), there are only two things I’m currently aware of that would result in problems, and you were unintentionally using one of them as described above. The other one–that afflicts many of the COIN game modules–is Beanshell expression usage of map/property names containing spaces but not encased in quotes.

The good news is that I think both of these can likely be fixed fairly quickly with some savvy find/replace in the buildfile using a good quality text editor, and then the module should work equally well in VASSAL 3.2.17 and 3.4.x.

[barbanera]

Speaking about the Talon module, I tried searching and replacing all instances of

[code]==$PlayerName$

or

!=$PlayerName$[/code]

in beanshells with the quoted version ("$PlayerName$") but that didn’t seem to fix the first issue reported. Which is: drag a piece from the game piece palette to the main map and 3.4.11 will now spew “Invalid Location” warnings before finishing. These are warnings I added for instances where said ships were moving to wrong areas/windows, but should definitively not trigger in this basic case. It might be some similar lack of quotes on other properties in beanshells and I will have to investigate that (were not Talon such a beast just to remember whatever I was thinking at the time…).

Anyway, looking at, hopefully simpler issues which were also reported, I have a module - Pax Emancipation 1.5 - which is giving a weird behaviour now, which it seems unrelated to Beanshells. It actually might also be related to the odd behaviour in Talon. Namely, popup warnings that should not come out coming out anyway.

To reproduce it, if anybody cares to look into this, one needs to follow these steps:

1. open the Pax Emancipation 1.5 module,
2. click on the “Idea Cards” button in the toolbar,
3. click on “SETUP COOPERATIVE ERA” there

With 3.2.17 a popup comes out asking to choose between 2 or 3 players and cards are dealt to the main map etc.
With 3.4.11 a warning pops up first saying “The game was already setup.” Then everything proceeds fine.

What that “SETUP COOPERATIVE ERA” does is run “setupMarket” on a (hidden via negative X,Y values) piece in that window, called “Cooperative Market setup”. The setupMarket command has several traits, but the relevant ones should be, in trait order (reading top down):

1. Set Global Property with name NumberOfPlayers (default is 0 in the Global Properties at module level), command “setup2” to select betwen 2 and 3, command setup1 to reset

2. Trigger Action with name “Give error (setup already done)”, which fires when NumberOfPlayers!=0 and runs “reportError” (-> the warning popup)

3. Trigger Action with name “Setup Market (not yet done)”, which fires when NumberOfPlayers=0 and runs “setup2”

I don’t understand why “Give error” triggers now, since NumberOfPlayers is hard set at 0 and I still even have to get the prompt to change it to either 2 or 3. Did something change with Set Global Property? More likely, I think that the old non-Beanshell syntax for the property check in the trigger (NumberOfPlayers!=0) is not parsed anymore. Was this deprecated syntax or an unwanted result of some bug fix? Thanks.

[m3tan]

I downloaded your Pax module and did a little tinkering. I believe the issue is that your Alert is embedded in a Calculated Property. I haven’t looked at how CPs are evaluated but my guess is that something has changed between 3.2 and 3.4 with how/when they are recalculated. Your Trigger Action is not causing the Alert. It’s the CP. The danger with placing an Alert in a CP is that any event that causes the CP to be recalculated will trigger it. I’ve always used a Dynamic Property for Alerts, rather than a Trigger Action + Calculated Property. It affords me much more control over when it will trigger. Anyway, I substituted in a DP in place of your CP and the Alert no longer occurs unless NumberOfPlayers!=0. Perhaps a developer can provide more detail on what has changed with CPs and whether or not your problem is fixable. If not you may need to convert all your CP based Alerts to DPs. Hope that helps.

[Brent_Easton]

To reproduce it, if anybody cares to look into this, one needs to follow these steps:

Why would we not care to look? This is what we do. If you had reported this problem to us when it was first reported to you back in early July, we would have investigated and resolved it back then and saved you and your players this angst. We cannot solve problems that people don’t tell us about.

You will also need to change the following cases, taking careful care of whether or not there is a space after the ==/!=

If(TalonController==$PlayerName$
If(TerranController==$PlayerName$
If(OwnerID==$highlightShipID$
If(TargetID==$highlightShipID$
{LocationName!=$OldLocationName$
CurrentMap == $OldMap$
ControllerName == $PlayerName$}
Sector == $CurrentSector$
Controller == $PlayerName$}
!= $PlayerName$)}
{Controller != $PlayerName$
{(BasicName != $CurrentSector$
CurrentZone != $CurrentSector$
Sector != $CurrentSector$

Note that there are 3 cases of != $OldTargetName$ or != $OldLocationName$ on lines 206, 231 and 412 that are NOT withing {} braces. These are old-style expressions that should not be changed unless you upgrade them to include the Beanshell braces.

I have done this, but the Illegal location alert is still appearing.

Investigating further.

Regards.

[barbanera]

@Brent Easton

Why would we not care to look? This is what we do. If you had reported this problem to us when it was first reported to you back in early July, we would have investigated and resolved it back then and saved you and your players this angst. We cannot solve problems that people don’t tell us about.

Wait, what do you mean “July”? This was reported December 5.

You will also need to change the following cases, taking careful care of whether or not there is a space after the ==/!=

If(TalonController==$PlayerName$
If(TerranController==$PlayerName$
If(OwnerID==$highlightShipID$
If(TargetID==$highlightShipID$
{LocationName!=$OldLocationName$
CurrentMap == $OldMap$
ControllerName == $PlayerName$}
Sector == $CurrentSector$
Controller == $PlayerName$}
!= $PlayerName$)}
{Controller != $PlayerName$
{(BasicName != $CurrentSector$
CurrentZone != $CurrentSector$
Sector != $CurrentSector$

Note that there are 3 cases of != $OldTargetName$ or != $OldLocationName$ on lines 206, 231 and 412 that are NOT withing {} braces. These are old-style expressions that should not be changed unless you upgrade them to include the Beanshell braces.

Thanks a lot, will do.

[barbanera]

@m3tan

I downloaded your Pax module and did a little tinkering. I believe the issue is that your Alert is embedded in a Calculated Property. I haven’t looked at how CPs are evaluated but my guess is that something has changed between 3.2 and 3.4 with how/when they are recalculated. Your Trigger Action is not causing the Alert. It’s the CP. The danger with placing an Alert in a CP is that any event that causes the CP to be recalculated will trigger it. I’ve always used a Dynamic Property for Alerts, rather than a Trigger Action + Calculated Property. It affords me much more control over when it will trigger. Anyway, I substituted in a DP in place of your CP and the Alert no longer occurs unless NumberOfPlayers!=0. Perhaps a developer can provide more detail on what has changed with CPs and whether or not your problem is fixable. If not you may need to convert all your CP based Alerts to DPs. Hope that helps.

Thanks a lot, m3tan. I knew that my use of Triggers+Report traits+Calculated Properties combos to popup warnings was rather archaic, but that the CPs were causing this issue I would neve have suspected.

Hope a developer chimes in providing more details to understand better what changed with CP code.

In the meanwhile, if I understand your hint, you suggest using a DP set to {NumberOfPlayers == 0 ? Alert(“You have already setup the game.”) : “”)} on “setupMarket” rather then use the “Give Error” trigger which fires the Report trait which writes out the CP named Error, correct?

[Brent_Easton]

Wait, what do you mean “July”? This was reported December 5.

My mistake. Looks like that was a different issue in the same thread that was triggering the same ‘Bad Location’ alert box.

m3tan said:

I haven’t looked at how CPs are evaluated but my guess is that something has changed between 3.2 and 3.4 with how/when they are recalculated.

Ah, thanks m3tan, very good. That will likely be the issue. Vassal makes absolutely no contract on how and when Calculated Properties might by re-calculated. They will be re-calculated ‘as-needed’, but that isn’t necessarily only when you want to know the value. Vassal will sometimes want to know the value as well.

Any processing that depends on a particular Calculated Property being evaluated at a specific time and only at that time will fail eventually. Normally, this is not a problem, it doesn’t matter how often a Calculated property is evaluated. However, in your case, creating a CP that consists of a side-effect (an alert message) is causing the problem. The purpose of Calculated Properties are designed to return a value, not display error messages.

There was a deep structural change made to improve the performance of a very slow part of the Vassal code and to fix a number of long-standing bugs over the setting of the ‘Old…’ property names. One side effect of this is that Calculated Properties are now re-evaluated whenever a piece moves or has a Command executed on it, so that the old value of a CP prior to a move/command is now accessible using OldCPname.

The fact that your module worked at all on 3.2 is purely by chance, and unfortunately, this is going to be the third case where we cannot maintain compatibility between 3.2 and 3.4. m3tan has developed a very simple and neat way to implement on-demand alerts that should be easy to migrate into your module. It looks like the only places this is used is in the Initiative Counter and Round Tracker At-start stacks and the ‘Ship Reports 4’ prototype.

BTW, for a very simple way to get a big improvement in speed in your module, look at replacing all of your chained If(x, y, z) functions in your Beanshell with ( x ? y : z) [or ( (x) ? (y) : (z) ) is probable safer]. Chained If() statements (where you have an If() inside an If()) get exponentially slower to evaluate the more If()'s you have chained inside each other.

[m3tan]

@barbanera

In the meanwhile, if I understand your hint, you suggest using a DP set to {NumberOfPlayers == 0 ? Alert(“You have already setup the game.”) : “”)} on “setupMarket” rather then use the “Give Error” trigger which fires the Report trait which writes out the CP named Error, correct?

You got it. I’m efficiency-obsessed so I try to embed my Alerts to an existing Trait. You can add them to any Beanshell expression. Just understand they would trigger every time unless you embed it inside a ? as in your example above. I only create a Dynamic Property (named “Alert”) if there is no other place to add them. The DP is actually an empty shell so its name and value don’t matter. Naming it Alert just serves as an obvious indicator of its purpose…

The ? was a godsend for me because I rely heavily on Prototypes. The downside of “over prototyping” is that it becomes very difficult to trigger actions in the proper sequence because you cannot rearrange the order of Traits once they are embedded in a Prototype. I was struggling with getting things to Report or Trigger in the exact sequence I wanted until I discovered the beauty of a well-placed ? instead of a Trigger Action.

[barbanera]

@Brent Easton

There was a deep structural change made to improve the performance of a very slow part of the Vassal code and to fix a number of long-standing bugs over the setting of the ‘Old…’ property names. One side effect of this is that Calculated Properties are now re-evaluated whenever a piece moves or has a Command executed on it, so that the old value of a CP prior to a move/command is now accessible using OldCPname.

This is commendable and well, but, as you yourself state, this is a “deep structural change” implying “that Calculated Properties are now re-evaluated whenever a piece moves or has a Command executed on it” and allowing the introduction of a new property OldCPname. This strikes me as exactly the kind of change that breaks backward compatibility.

Perhaps using CPs for alerts was a bad idea, never intended to be supported, but how was I to know? It worked, hence it was implicitly supported.

Any change not strictly related to closing security holes should be backward compatible, I think imho. In this case CPs should only be recalculated when a piece moves or a command is executed on it (or any other circumstance when it was not recalculated in 3.2.17) only when not running a module created with 3.2.17. Otherwise it should default back to the old behaviour (with the OldCPname variable not being supported in this case).

Just my 2c, and of course this being a free project I am not complaining at all. But this was what this thread was about: not asking for specific hints to solve my particular issues (which I would have asked in due course and I am grateful you and others took on to address right away, it’s helping a lot) but to make a general point that if backward compatibility could be more forcefully enforced that would be a major bonus.

[m3tan]

A Calculated Property suggests that its value is reevaluated (calculated) without external input. Increasing the frequency that this happens is not destroying backward compatibility, it’s improving performance. The fact that your Alert worked is akin to a broken clock being right twice a day - not implicit support. I’m not one of the developers but I am a programmer by trade. It should not have worked. I’m not trying to judge here. Your module is excellent by any standard, especially if you aren’t a programmer, but that’s my perspective as a professional…