Label text causing deep freeze

[derimmer]

https://www.gamesquad.com/forums/index.php?threads/vasl-freezes-when-moving-axis-minor-units-with-concealed-marker.201331/#post-2180031

The use of a "(" or ")" in a label seems to cause VASL to freeze. It may be connected to HIP status. This is the SD/SP problem.

[geezer09]

Hum, i cant see to reproduce this bug. What do you mean by SD/SP?

[jrvjrv]

Can't reproduce. In vasl 6.7.0-beta1, added a 6-2-8, added a label (ctrl-L) with parentheses "(test)", worked like a charm. Added a draggable overlay label with background, changed top line (ctrl-L) to "(test)", worked like a charm. This on vasl as above, vassal 3.7.15, linux. Can you provide steps to reproduce?

[jrvjrv]

vasl 6.6.10, vassal 3.7.15, linux
Steps to reproduce:

1. add a Soviet 6-2-8 to board.
2. label it (ctrl-L) with "(test)"
3. HIP it (ctrl-H)
4. move it one hex in any direction
   cpu utilization for process goes haywire.

[derimmer]

@jrvjrv - correct. The GS thread contained references to HIP and moving units as a trigger.

@geezer09 SD/SP are the initials of the players involved in the other reported instance. In that case it was all handled by email so I had no link to post. It was a reminder to me more than anything else. I have learned that if I don't add such references, when I come back to work on something I can't remember when/where the problem was reported. LOL.

The consequences of the freeze seem to be catastrophic: the game freezes, cannot be unfrozen, must be shut down using Task Manager or other tool and all is lost (unless they are using 6.7.0-beta1 with autosave!). Which is why I put out an alert.

[geezer09]

Found the bug in VASLGameInterface.java line 226

userEnd = piecename.indexOf(")");

I can fix the bug by changing to

 userEnd = piecename.lastIndexOf(")");

What happens is that if your unit is hipped and has "()" in label then it has the following name
8-3-8 AEsq ((TEST))(HIP)

The parsepiecename function then mistakenly looks for the first "(" and first ")" to remove that so you end up with
8-3-8 AEsq )(HIP)

I am not sure what the intent of the function is and wether in the above case it should return

8-3-8 AEsq
or
8-3-8 AEsq (HIP)

@derimmer , Git says your the last one to have changed this code, maybe you can elucidate?

[derimmer]

Yes, I added the parsepiecename function back in February as part of my work to have los work with Terrain/Hindrance counters and draggable overlays.

If you look at the calling method - updatePiece(), you will see that for some counters I need to test the counter name and in some instances, VASSAL was adding information to the counter name that I could not work with and so I tried to strip it out for LOS purposes via parsepiecename. Obviously, I did it badly.

@geezer09 to answer your specific question, I am pretty sure that I don't want things like "(HIP)" to be included but I need to check in more detail. And certainly, we can add a test upfront to ensure that only non-unit counters get tested anyway.

I suspect that what I should have done was change/update the CounterMetadata.xml file to handle the name issues there.

I am currently losing the war with the bridge draggable overlays. . . . VASSAL is adding Layer names to the counter name. Helpful in some situations I am sure, but certainly not with bridge overlays.