Create account / Log in

Vassal 3.4.xyz errors/warnings

Issues with the Vassal engine.

Moderators: uckelman, Tim M

Re: Vassal 3.4.xyz errors/warnings

Postby barbanera » December 9th, 2020, 10:34 pm

m3tan wrote: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?
barbanera
 
Posts: 467
Joined: January 12th, 2012, 2:27 pm

Re: Vassal 3.4.xyz errors/warnings

Postby Brent Easton » December 9th, 2020, 11:14 pm

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.
User avatar
Brent Easton
 
Posts: 3310
Joined: December 21st, 2007, 3:06 am
Location: Berry, NSW, Australia

Re: Vassal 3.4.xyz errors/warnings

Postby m3tan » December 10th, 2020, 12:58 am

barbanera wrote: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.
User avatar
m3tan
 
Posts: 232
Joined: August 12th, 2018, 11:49 pm

Re: Vassal 3.4.xyz errors/warnings

Postby barbanera » December 10th, 2020, 9:46 am

Brent Easton wrote: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.
barbanera
 
Posts: 467
Joined: January 12th, 2012, 2:27 pm

Re: Vassal 3.4.xyz errors/warnings

Postby m3tan » December 10th, 2020, 5:00 pm

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...
User avatar
m3tan
 
Posts: 232
Joined: August 12th, 2018, 11:49 pm

Re: Vassal 3.4.xyz errors/warnings

Postby barbanera » December 10th, 2020, 5:50 pm

Except my clock was right 24-7 under 3.2.17 ;-) And, while I am not a professional programmer anymore, I have programmed in my life, for many years, both as a hobby and as job, btw.

Anyway, I made my point and I won't press it further.

So, m3tan, I added those Alerts for Pax Emancipation directly in the map global key commands, Like this: {BasicName=="Cooperative Market setup" && (NumberOfPlayers != 0 ? Alert("ERROR: the cooperative phase has been setup already.") : 1) == 1}
{(BasicName=="Competitive Market setup") && (NumberOfPlayers == 0 ? Alert("ERROR: the cooperative phase has to be setup first.") : (CurrentEra == "COMPETITIVE ERA" ? Alert("ERROR: the competitive phase has been setup already.") : 1) == 1)}

They work under 3.2.17 and 3.4.11, no hiccups.

What do you think? Deprecated coding again? Thanks.
barbanera
 
Posts: 467
Joined: January 12th, 2012, 2:27 pm

Re: Vassal 3.4.xyz errors/warnings

Postby barbanera » December 12th, 2020, 9:44 am

Brent, regarding the Talon module I removed those Alerts from the CPs and fixed (hopefully) all unquoted $variables$ in beanshells. I saved this new release under 3.2.17 and those reported problems are now gone, apparently, when the module is opened with 3.4.11. The new release is in my development folder

https://drive.google.com/drive/u/1/folders/0B1ftnUlDGp-dSHVVdzNkX2VVbGM.

However, I just noticed another issue: under 3.2.17 the ship counters in the Counters window (top left of toolbar) show a text label like "SP=32" (for the Ship points setup costs) along the top left edge of the counter. This text label is not showing anymore with 3.4.11.

This text label is towards the bottom of the "Ship" prototype (first one in the prototypes section) and is just displaying $SPlabel$ where SPlabel is a CP set to

Code: Select all
If(CurrentMap=="","SP = "+ShipPoints,"")


Basically I was using the fact that CurrentMap is undefined in a piece palette window - and defined elsewhere in the module. (I checked, and indeed CurrentMap is still blank/undefined in piece palettes, even under 3.4.11, see the extra central yellow text label now displaying Map=$CurrentMap$.)

What gives? Perhaps CPs are now NOT calculated at all in game piece palettes with 3.4.11? Or the above condition returns false because now undefined does not mean blank and I have to use some other statement? (Actually, is there something like an undef function/operator in Beanshell?)

P.S. I just noticed that the "SP=..." text label is not showing even in the module editor under 3.4.11 (but it is under 3.2.17).
barbanera
 
Posts: 467
Joined: January 12th, 2012, 2:27 pm

Re: Vassal 3.4.xyz errors/warnings

Postby Brent Easton » December 14th, 2020, 10:12 am

Perhaps CPs are now NOT calculated at all in game piece palettes with 3.4.11?


Yes, that would be correct. Calculated Properties being calculated off-map result in spurious Bad Data Errors being generated when Properties are referenced that are not set up correctly in off-map pieces. It looks like this was included in 3.4.4 as part of another fix to Calculated Properties and so was not documented properly in the Change notes. My apologies for this.

It looks like you have managed to find a valid use-case requiring off-map CP calculation.

I can offer you two options:

1. Instead of
Code: Select all
$SPlabel$
as your Text Label text, use
Code: Select all
{CurrentMap=="" ? "SP = "+ShipPoints : ""}
. This will work now.

2. Wait for a while until I have time to add a 'Calculate when off-map?' checkbox to Calculated Property.

Regards.
User avatar
Brent Easton
 
Posts: 3310
Joined: December 21st, 2007, 3:06 am
Location: Berry, NSW, Australia

Re: Vassal 3.4.xyz errors/warnings

Postby barbanera » December 14th, 2020, 10:35 pm

Oh, wow! Since there is no Expression Builder icon/button showing, I had never noticed that even with Vassal 3.2 (or perhaps even earlier) text label values could use Beanshell expressions.

I am going for solution #1 then. But perhaps a "Calculate when off-map?" functionality for CPs would still be nice, because one never knows who might have, or have had, need for that.

Thanks.
barbanera
 
Posts: 467
Joined: January 12th, 2012, 2:27 pm

Re: Vassal 3.4.xyz errors/warnings

Postby barbanera » December 19th, 2020, 2:35 pm

Brent, I don't know why, but changing Beanshells to double quote things breaks the Talon module now. In some instances, at least, haven't tested them all.

For example, in the "Terran Ship Token" prototype changing a trigger property match from

Controller != $PlayerName$

to

Controller != "$PlayerName$"

Breaks an entire setup chain. Those triggers should fire only when Controller is not equal to $PlayerName$, but they are firing with the double quotes now, which is befuddling. Controller was just setup to be equal to $PlayerName$ when a player "takes control" of a faction, by using certain commands. This is not a 3.4.11 thing, though: removing the quotes the code "works" (by chance?) as before.

However, with 3.4.11 some tokens (like map features) that are placed at setup from a startup gkc are not setup properly anymore. For example, see the Empire War map, Terran or Talon tabs: every other row is empty under 3.4.11 now.

Since you were kind enough to open bug tracking for Talon issues, I could give specifics to reproduce everything. However, I am starting to despair. I don't think I can update Talon to 3.4.11 compliance, making also sure I don't leave behind unsupported unquoted expressions which might stop working in a future release etc. There are too many subtle/hidden changes that break things beyond a quick update job. Or, at least, a job worth doing spending a reasonable amount of time on. This being quite a complex module, that is.

I will just post a warning to use 3.2.17 for it, which was fully tried and tested for this module at the time. Most people will not notice that warning and assume the module was broken to start with, but that's life... Thanks for you help.
barbanera
 
Posts: 467
Joined: January 12th, 2012, 2:27 pm

Re: Vassal 3.4.xyz errors/warnings

Postby Brent Easton » December 20th, 2020, 12:01 am

When I was looking at your module, I noticed a couple of old-style property match expressions that are NOT Beanshell expressions - they did NOT have the {} around them. If you modify those expressions and add the quotes, it will break your module unless you also add the {}.
User avatar
Brent Easton
 
Posts: 3310
Joined: December 21st, 2007, 3:06 am
Location: Berry, NSW, Australia

Re: Vassal 3.4.xyz errors/warnings

Postby m3tan » December 20th, 2020, 4:50 am

I've used the "$$" expressions extensively since their introduction and they've worked flawlessly with a variety of operators such as COUNT and !~ and nested with multiple && and ||. If I had to guess, you have some stray syntax that is non-Beanshell or the culprit is a tangentially related expression that is indirectly causing the "$$" expressions to appear to be failing.
User avatar
m3tan
 
Posts: 232
Joined: August 12th, 2018, 11:49 pm

Re: Vassal 3.4.xyz errors/warnings

Postby barbanera » December 20th, 2020, 9:09 am

Brent Easton wrote:When I was looking at your module, I noticed a couple of old-style property match expressions that are NOT Beanshell expressions - they did NOT have the {} around them. If you modify those expressions and add the quotes, it will break your module unless you also add the {}.


Yeah, I made sure NOT to change the non Beanshell expressions.

m3tan wrote:If I had to guess, you have some stray syntax that is non-Beanshell or the culprit is a tangentially related expression that is indirectly causing the "$$" expressions to appear to be failing.


Ok, this is getting weirder. As I mentioned I have this property check in a trigger trait in the Terran Ship Token prototype (only trigger there) which originally was as follow and, most importantly, was working as intended under both 3.2.17 and 3.4.11 (i.e. NOT triggering when Controller matches PlayerName):

Code: Select all
{((isCasualty==1) || (CurrentMap!=Faction && CurrentMap!=Faction+" 1000") || (Controller!=$PlayerName$))}

It also works as above if I switch to Controller!=PlayerName (which I guess would be the recommended syntax).

However, If I switch to Controller!="$PlayerName$" it does not work as intended in my module, because the trait triggers (with fatal consequences for a certain setup routine).

What gives? I don't understand.
barbanera
 
Posts: 467
Joined: January 12th, 2012, 2:27 pm

Re: Vassal 3.4.xyz errors/warnings

Postby barbanera » December 20th, 2020, 9:49 am

BTW, here are a couple screen shots relative to the other setup issue I mentioned (where pieces won't show up in 3.4.11).

Talon (Empire War map, Terran tab) under 3.2.17:

TalonMap3.2.17.png
TalonMap3.2.17.png (226.61 KiB) Viewed 1844 times

and under 3.4.11:

TalonMap3.4.11.png
TalonMap3.4.11.png (199.89 KiB) Viewed 1844 times

Discouraging...
barbanera
 
Posts: 467
Joined: January 12th, 2012, 2:27 pm

Re: Vassal 3.4.xyz errors/warnings

Postby uckelman » December 21st, 2020, 12:02 am

Help me out here---what are the missing images? Where are they defined? It's hard to help when I don't know what I'm looking for.
User avatar
uckelman
Site Admin
 
Posts: 9227
Joined: December 10th, 2007, 9:48 am
Location: Durham, England

PreviousNext

Return to Technical Support & Bugs

Who is online

Users browsing this forum: Klaus-Dieter and 3 guests