Can someone help me locate the correct java class file?

[jestew]

Greetings,

I'm looking at fixing the following bug:

http://www.vassalengine.org/tracker/sho ... i?id=13307

that effects Text Labels for game pieces that use transparency. I've attempted to dig through the VASSAL engine java code for the very first time and it's deep and wide, so if anyone could point me in the right direction I'd be grateful.

What class reads in the data from color-picker and stores it in the game piece object?

I've tried modify the text file for the module (buildfile) to insert a transparency directly in the text to see if it renders correctly - but it doesn't.

Here is an example piece:

Code: Select all

<VASSAL.build.module.PrototypeDefinition name="test">+/null/label;;Change Label;10;0,0,0;190,190,190;t;0;c;0;b;c;$pieceName$ ($label$);Dialog;0;0;TextLabel;test   piece;;;;/test   null;263;6;;0</VASSAL.build.module.PrototypeDefinition>

There are only three integers stored for foreground (0,0,0) and background (190,190,190). Whatever serializer is being used is throwing away the transparency info.

Thanks for any guidance! Cheers, jas...

[Brent Easton]

Vassal uses the standard Java Color Picker to select colors in the Text Label Trait and this picker supports transparency.

However, the Text Label trait in Vassal has never supported the setting of the Transparency of a background color. There are only 2 options, 100% or 0%. You can select 100% transparency by bringing up the Color Picker and clicking Cancel.

The class you want to look at is VASSAL.counters.Labeler.

You will need to add a new field to hold the transparency, encode and decode as appropriate, and set/read it from the Color Pickers in the Configurer.

This is a great project to start with and I am happy to help

[jestew]

Great! Thanks Brent! I'll get chugging and then ask questions here.

How's life in NSW these days?? I live in Old South Wales - and it's cold and wet and windy.

[jestew]

OK. It works. I can now create a Text Label with transparent text and background that persists after a save and reload.

Here's the diff for ColorConfigurer.java:

Code: Select all

93,97c93,94
<     if (c == null) {
<       return null;
<     }
<     else if (c.getTransparency() == c.OPAQUE) {
<       return c.getRed() + ","
---
>     return c == null ? null :
>         c.getRed() + ","
100,106d96
<     }
<     else {
<       return c.getRed() + ","
<         + c.getGreen() + ","
<         + c.getBlue() + ","
<         + c.getAlpha();
<     }
120,130c110,112
<         if (st.countTokens() > 3) { // has alpha value
<           return new Color(Integer.parseInt(st.nextToken()),
<                            Integer.parseInt(st.nextToken()),
<                            Integer.parseInt(st.nextToken()),
<                            Integer.parseInt(st.nextToken()));
<         }
<         else { // no alpha
<           return new Color(Integer.parseInt(st.nextToken()),
<                            Integer.parseInt(st.nextToken()),
<                            Integer.parseInt(st.nextToken()));
<         }
---
>         return new Color(Integer.parseInt(st.nextToken()),
>                          Integer.parseInt(st.nextToken()),
>                          Integer.parseInt(st.nextToken()));


Anything else you need me to do for this one?

Cheers, jas...

[Flint1b]

Usually we do this with GitHub PRs, there is a CI pipeline that would check your code for various possible errors, code formatting mistakes, compilation failures, broken unit tests, etc. Will also give us the opportunity to do a code review before merging this.

With just the diff, one of us would have to do this whole procedure

[Brent Easton]

Hi Jas,

Cold, Sunny and Windy here

Get yourself set up on github and fork the vassal repository as Yan suggests. Then you can push a PR into the system and it makes it easy for us to evaluate, test and review it. Diffs are painful to read and time consuming to process. Does it work? This particular version will have compile errors.

From what I can see you have got to the heart of the matter and Fixing the ColorConfigurer encoder/decoder is quite a neat solution.

The main things to look out for testing trait changes are
a) does it save and load the new feature
b) what happens when you load an earlier version of the module that does not have the new feature. Does it convert the old version cleanly to an equivalent in the new?
c) what happens if you load the new feature using an older version of Vassal? While this is not recommended, it will happen. Ideally in this case, the opacity should be just quietly ignored. It looks like this should be the case here.

Cheers,
Brent.

[jestew]

Hi Yan, Brent,

OK. I made some time and gitted everything instead. I was lazy the other day and just grabbed the .zip file... Teach me.

Luckily, I had already downloaded all the maven libs - so the build just screamed today.

I tested everything out with a clean build. Then modified the ColorConfigurer class and rebuilt. Then tested again. All is well.

For your questions Brent:
a) Yes.
b) Yes.
c) Yes. Older vassal just loses the transparency and displays the BG as black.

I'm not an expert git user - I'll have to remind myself tomorrow how to make a pull request...

Let me know if there's something you want me to do first. Like make a test for the bug...

[Brent Easton]

Adding a test for the ColorConfigurer would be great. You could also have a look at writing a SerializeTest for Labeler. Our Unit Testing department is seriously understaffed