Check-ins for 3.2

Brent! You out there tonight?

My AutoExecGKC is inheriting a null ‘name’ from GlobalKeyCommand; so I need to know from Brent what the rules are for 'name’s of GKC’s to know how to fix the error.

Hi Pieter,
I am on IRC now.
Brent.

Pieter,
It may be blank, but it shouldn’t be null.
Brent.

try svn7902 in pgeerkens-3.2

Make that svn7904. I forgot to defer to super when not assigning visibility myself.

Thus spake pgeerkens:

Make that svn7904. I forgot to defer to super when not assigning
visibility myself.

That fixes the exception I had.

There’s one more thing which I think should be done here before I merge:
You say in the documentation that if there’s more than one AutoExecGKC,
the execution order is undefined, so it’s better to create a
MultiActionButton nested in a single AutoExecGKC if multiple commands
are desired. This makes me think that we simply shouldn’t permit there
to be more than one AutoExecGKC in the build tree, since the alternative
will lead to confusing, undesired behavior.

There’s a class calledSingleChildInstance which is used to prevent dupes
from being added to the build tree. Maybe AutoExecGKC should be set up
with one of those. Thoughs?


J.

Good idea. Let me check that out and see what is involved with implementing that.

Hmm! A first quick test and validator does nothing, not even a report of improper activity. I will check it out.

From memory, the validators only run when you try and save the module.

*********** REPLY SEPARATOR ***********

On 19/10/2011 at 4:28 PM pgeerkens wrote:

Hmm! A first quick test and validator does nothing, not even a report of
improper activity. I will check it out.


Brent Easton
Analyst/Programmer
University of Western Sydney
Email: b.easton@exemail.com.au

I think I can safely say that SingleChildInstance is a stub that never got completed. Not only is it only capable of saying “Naughty boy!”, it doesn’t even ever say that because the instantiators appear never to call the validate method:

[code]/**

  • Ensures that at most a single instance of a given type
  • belongs to a given parent
    */
    public class SingleChildInstance implements ValidityChecker {
    private AbstractConfigurable target;
    private Class<?> childClass;

public SingleChildInstance(AbstractConfigurable target,
Class<?> childClass) {
this.childClass = childClass;
this.target = target;
}

public void validate(Buildable b, ValidationReport report) {
if (b == target && target.getComponentsOf(childClass).size() > 1) {
report.addWarning(
"No more than one " +
ConfigureTree.getConfigureName(childClass) +
" allowed in " +
ConfigureTree.getConfigureName(target));
}
}
}[/code]

Now the question is how should it work? It seems to me that the method validate of interface ValidityChecker needs to return a boolean with semantics “reject!” in addition to reporting the violation. Also, I believe this check should control the “enabled” status of the component on the drop-down menu, not be an after-the-fact error logger. Thoughts?

Pieter,

I believe that’s how the current validator works. From memory, at the time you save a module, all Buildables in the module are traversed and have any attached ValidityCheckers run and an ‘Are you sure?’ dialog is shown at the end. No attempt is made to prevent the illegal components from being saved. They are treated as warnings rather than errors.

B.

*********** REPLY SEPARATOR ***********

On 19/10/2011 at 4:52 PM pgeerkens wrote:

I think I can safely say that SingleChildInstance is a stub that
never got completed. Not only is it only capable of saying “Naughty
boy!”, it doesn’t even ever say that because the instantiators appear
never to call the __validate __method:

Code:
/**

  • Ensures that at most a single instance of a given type
  • belongs to a given parent
    */
    public class SingleChildInstance implements ValidityChecker {
    private AbstractConfigurable target;
    private Class<?> childClass;

public SingleChildInstance(AbstractConfigurable target,
Class<?> childClass) {
this.childClass = childClass;
this.target = target;
}

public void validate(Buildable b, ValidationReport report) {
if (b == target && target.getComponentsOf(childClass).size() > 1) {
report.addWarning(
"No more than one " +
ConfigureTree.getConfigureName(childClass) +
" allowed in " +
ConfigureTree.getConfigureName(target));
}
}
}

Now the question is how should it work? It seems to me that the method
__validate __of interface __ValidityChecker __needs to return a boolean
with semantics “reject!” in addition to reporting the violation. Also, I
believe this check should control the “enabled” status of the component
on the drop-down menu, not be an after-the-fact error logger. Thoughts?


Read this topic online here:
https://forum.vassalengine.org/t/check-ins-for-3-2/4306/30


messages mailing list
messages@vassalengine.org
vassalengine.org/mailman/listinfo/messages


No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1411 / Virus Database: 1522/3962 - Release Date: 10/19/11


No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1411 / Virus Database: 1522/3962 - Release Date: 10/19/11


Brent Easton
Analyst/Programmer
University of Western Sydney
Email: b.easton@exemail.com.au

Hi Brent,

Yes, I understand form a backwards-compatibility standpoint why they are
only warnings when detected after-the-fact,; but going forward it would be
nice to prevent creation of unsuitable structures.

I have been looking at ConfigureTree, and expect to get the changes done by
tomorrow. I will add an interface SingleChildInstanceOnline (or whatever
Joel suggests as an alternative that ugly moniker) and for all children
implementing that interface I will set/clear the enabled flag of the
drop-down menu action and the valid paste-target flag on whether the current
child count of that type is =0 or >0.

It must be your lunch-time about now I expect. My other project this week
is to finish up GridShear to obsolesce the current GridEditor (almost).: The
UI will be:
Drag-and-drop one hex center/corner onto the map-grid as a first fixed
point;
Drag-and-drop a second hex center/corner as a rotation/scaling about FP1 to
size and roughly align the grid, also setting a second fixed-point; then
Drag-and-drop a third hex corner/center as a shearing operation about the
first two.

Step three is already done, and works nicely, relying on the existing Grid
Editor to achieve the effect of 1 & 2. The twist introduced by cheap /old
scanners is easily eliminated, and combined with my proper hex-snapping
algorithm I believe behaves really professional.

Pieter
-----Original Message-----
From: messages-bounces@vassalengine.org
[mailto:messages-bounces@vassalengine.org] On Behalf Of Brent Easton
Sent: Wednesday, October 19, 2011 10:19 PM
To: messages@vassalengine.org
Subject: Re: [messages] [Developers] Re: Check-ins for 3.2

Pieter,

I believe that’s how the current validator works. From memory, at the time
you save a module, all Buildables in the module are traversed and have any
attached ValidityCheckers run and an ‘Are you sure?’ dialog is shown at the
end. No attempt is made to prevent the illegal components from being saved.
They are treated as warnings rather than errors.

B.

*********** REPLY SEPARATOR ***********

On 19/10/2011 at 4:52 PM pgeerkens wrote:

I think I can safely say that SingleChildInstance is a stub that
never got completed. Not only is it only capable of saying “Naughty
boy!”, it doesn’t even ever say that because the instantiators appear
never to call the __validate __method:

Code:
/**

  • Ensures that at most a single instance of a given type
  • belongs to a given parent
    */
    public class SingleChildInstance implements ValidityChecker {
    private AbstractConfigurable target;
    private Class<?> childClass;

public SingleChildInstance(AbstractConfigurable target,
Class<?> childClass) {
this.childClass = childClass;
this.target = target;
}

public void validate(Buildable b, ValidationReport report) {
if (b == target && target.getComponentsOf(childClass).size() > 1) {
report.addWarning(
"No more than one " +
ConfigureTree.getConfigureName(childClass) +
" allowed in " +
ConfigureTree.getConfigureName(target));
}
}
}

Now the question is how should it work? It seems to me that the method
__validate __of interface __ValidityChecker __needs to return a boolean
with semantics “reject!” in addition to reporting the violation. Also,
I believe this check should control the “enabled” status of the
component on the drop-down menu, not be an after-the-fact error logger.
Thoughts?


Read this topic online here:
https://forum.vassalengine.org/t/check-ins-for-3-2/4306/30


messages mailing list
messages@vassalengine.org
vassalengine.org/mailman/listinfo/messages


No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1411 / Virus Database: 1522/3962 - Release Date: 10/19/11


No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1411 / Virus Database: 1522/3962 - Release Date: 10/19/11


Brent Easton
Analyst/Programmer
University of Western Sydney
Email: b.easton@exemail.com.au

Hi Pieter,

Sounds good, I am all for more active validations and preventatives of obvious errors.

On the subject of the GridEditor, I am currently in the process reworking all of the separate editors that work with a Board displayed in the background

  • Deck and Setup Stack position configurer
  • Region Grid region position editor
  • Zone shape configurer

These are all implemented separately, include large amounts of duplicate code and are buggy to boot. I am consolidating them all into a generic BoardEditor class that looks after the board display, controls, scrolling etc. and a BoardEditorModel interface that each definition class must implement. This is something that has needed doing for a long time.

Now, the GridEditor falls into this class as well, but because of it’s complexity and because you are working on it, I have left it be, but once I have finished, you might want to have a look and see if it would be useful.

BTW, the map I sent you from the new Devil’s Cauldron game with the twisted grid was actually the original artwork, not from a scanner. Even the artist has no idea how he managed to screw it up!

Cheers,
Brent.

*********** REPLY SEPARATOR ***********

On 19/10/2011 at 10:39 PM pgeerkens wrote:

Hi Brent,

Yes, I understand form a backwards-compatibility standpoint why they are
only warnings when detected after-the-fact,; but going forward it would be
nice to prevent creation of unsuitable structures.

I have been looking at ConfigureTree, and expect to get the changes done
by
tomorrow. I will add an interface SingleChildInstanceOnline (or whatever
Joel suggests as an alternative that ugly moniker) and for all children
implementing that interface I will set/clear the enabled flag of the
drop-down menu action and the valid paste-target flag on whether the
current
child count of that type is =0 or >0.

It must be your lunch-time about now I expect. My other project this week
is to finish up GridShear to obsolesce the current GridEditor (almost).:
The
UI will be:
Drag-and-drop one hex center/corner onto the map-grid as a first fixed
point;
Drag-and-drop a second hex center/corner as a rotation/scaling about FP1 to
size and roughly align the grid, also setting a second fixed-point; then
Drag-and-drop a third hex corner/center as a shearing operation about the
first two.

Step three is already done, and works nicely, relying on the existing Grid
Editor to achieve the effect of 1 & 2. The twist introduced by cheap /old
scanners is easily eliminated, and combined with my proper hex-snapping
algorithm I believe behaves really professional.

Pieter
-----Original Message-----
From: messages-bounces@vassalengine.org
[mailto:messages-bounces@vassalengine.org] On Behalf Of Brent Easton
Sent: Wednesday, October 19, 2011 10:19 PM
To: messages@vassalengine.org
Subject: Re: [messages] [Developers] Re: Check-ins for 3.2

Pieter,

I believe that’s how the current validator works. From memory, at the time
you save a module, all Buildables in the module are traversed and have any
attached ValidityCheckers run and an ‘Are you sure?’ dialog is shown at the
end. No attempt is made to prevent the illegal components from being saved.
They are treated as warnings rather than errors.

B.

*********** REPLY SEPARATOR ***********

On 19/10/2011 at 4:52 PM pgeerkens wrote:

I think I can safely say that SingleChildInstance is a stub that
never got completed. Not only is it only capable of saying “Naughty
boy!”, it doesn’t even ever say that because the instantiators appear
never to call the __validate __method:

Code:
/**

  • Ensures that at most a single instance of a given type
  • belongs to a given parent
    */
    public class SingleChildInstance implements ValidityChecker {
    private AbstractConfigurable target;
    private Class<?> childClass;

public SingleChildInstance(AbstractConfigurable target,
Class<?> childClass) {
this.childClass = childClass;
this.target = target;
}

public void validate(Buildable b, ValidationReport report) {
if (b == target && target.getComponentsOf(childClass).size() > 1) {
report.addWarning(
"No more than one " +
ConfigureTree.getConfigureName(childClass) +
" allowed in " +
ConfigureTree.getConfigureName(target));
}
}
}

Now the question is how should it work? It seems to me that the method
__validate __of interface __ValidityChecker __needs to return a boolean
with semantics “reject!” in addition to reporting the violation. Also,
I believe this check should control the “enabled” status of the
component on the drop-down menu, not be an after-the-fact error logger.
Thoughts?


Read this topic online here:
https://forum.vassalengine.org/t/check-ins-for-3-2/4306/30


messages mailing list
messages@vassalengine.org
vassalengine.org/mailman/listinfo/messages


No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1411 / Virus Database: 1522/3962 - Release Date: 10/19/11


No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1411 / Virus Database: 1522/3962 - Release Date: 10/19/11


Brent Easton
Analyst/Programmer
University of Western Sydney
Email: b.easton@exemail.com.au


messages mailing list
messages@vassalengine.org
vassalengine.org/mailman/listinfo/messages


messages mailing list
messages@vassalengine.org
vassalengine.org/mailman/listinfo/messages


No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1411 / Virus Database: 1522/3962 - Release Date: 10/19/11


No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1411 / Virus Database: 1522/3962 - Release Date: 10/19/11


Brent Easton
Analyst/Programmer
University of Western Sydney
Email: b.easton@exemail.com.au

Before you get to far along, take a look at my implementation of
AbstractAttributeListConfigurable, and my use of it in
RegularHexGridNumbeirng and AbstractUIHexGrid. I was able to eliminate
literally hundreds of lines of code in these two classes alone, solely to do
with implementing AbstractConfigurable, using generics and reflection. And
the code to do so is already written and is less than half the size of the
eliminated code. It sure makes the result easier to read.

It’s all in PGeerkens-3.2

I could use a good word with Joel to get this into the trunk sooner than
later. (And I did 90% of the heavy lifting last weekend bringing my branch
up-to-date with everything you and he have done in the trunk up to Sunday
night.)

Pieter

-----Original Message-----
From: messages-bounces@vassalengine.org
[mailto:messages-bounces@vassalengine.org] On Behalf Of Brent Easton
Sent: Wednesday, October 19, 2011 11:01 PM
To: messages@vassalengine.org
Subject: Re: [messages] [Developers] Re: Check-ins for 3.2

Hi Pieter,

Sounds good, I am all for more active validations and preventatives of
obvious errors.

On the subject of the GridEditor, I am currently in the process reworking
all of the separate editors that work with a Board displayed in the
background

  • Deck and Setup Stack position configurer
  • Region Grid region position editor
  • Zone shape configurer

These are all implemented separately, include large amounts of duplicate
code and are buggy to boot. I am consolidating them all into a generic
BoardEditor class that looks after the board display, controls, scrolling
etc. and a BoardEditorModel interface that each definition class must
implement. This is something that has needed doing for a long time.

Now, the GridEditor falls into this class as well, but because of it’s
complexity and because you are working on it, I have left it be, but once I
have finished, you might want to have a look and see if it would be useful.

BTW, the map I sent you from the new Devil’s Cauldron game with the twisted
grid was actually the original artwork, not from a scanner. Even the artist
has no idea how he managed to screw it up!

Cheers,
Brent.

*********** REPLY SEPARATOR ***********

On 19/10/2011 at 10:39 PM pgeerkens wrote:

Hi Brent,

Yes, I understand form a backwards-compatibility standpoint why they
are only warnings when detected after-the-fact,; but going forward it
would be nice to prevent creation of unsuitable structures.

I have been looking at ConfigureTree, and expect to get the changes
done by tomorrow. I will add an interface SingleChildInstanceOnline (or
whatever Joel suggests as an alternative that ugly moniker) and for all
children implementing that interface I will set/clear the enabled flag
of the drop-down menu action and the valid paste-target flag on whether
the current child count of that type is =0 or >0.

It must be your lunch-time about now I expect. My other project this
week is to finish up GridShear to obsolesce the current GridEditor
(almost).:
The
UI will be:
Drag-and-drop one hex center/corner onto the map-grid as a first fixed
point; Drag-and-drop a second hex center/corner as a rotation/scaling
about FP1 to size and roughly align the grid, also setting a second
fixed-point; then Drag-and-drop a third hex corner/center as a shearing
operation about the first two.

Step three is already done, and works nicely, relying on the existing
Grid Editor to achieve the effect of 1 & 2. The twist introduced by
cheap /old scanners is easily eliminated, and combined with my proper
hex-snapping algorithm I believe behaves really professional.

Pieter
-----Original Message-----
From: messages-bounces@vassalengine.org
[mailto:messages-bounces@vassalengine.org] On Behalf Of Brent Easton
Sent: Wednesday, October 19, 2011 10:19 PM
To: messages@vassalengine.org
Subject: Re: [messages] [Developers] Re: Check-ins for 3.2

Pieter,

I believe that’s how the current validator works. From memory, at the
time you save a module, all Buildables in the module are traversed and
have any attached ValidityCheckers run and an ‘Are you sure?’ dialog is
shown at the end. No attempt is made to prevent the illegal components from
being saved.
They are treated as warnings rather than errors.

B.

*********** REPLY SEPARATOR ***********

On 19/10/2011 at 4:52 PM pgeerkens wrote:

I think I can safely say that SingleChildInstance is a stub that
never got completed. Not only is it only capable of saying “Naughty
boy!”, it doesn’t even ever say that because the instantiators appear
never to call the __validate __method:

Code:
/**

  • Ensures that at most a single instance of a given type
  • belongs to a given parent
    */
    public class SingleChildInstance implements ValidityChecker {
    private AbstractConfigurable target;
    private Class<?> childClass;

public SingleChildInstance(AbstractConfigurable target,
Class<?> childClass) {
this.childClass = childClass;
this.target = target;
}

public void validate(Buildable b, ValidationReport report) {
if (b == target && target.getComponentsOf(childClass).size() > 1) {
report.addWarning(
"No more than one " +
ConfigureTree.getConfigureName(childClass) +
" allowed in " +
ConfigureTree.getConfigureName(target));
}
}
}

Now the question is how should it work? It seems to me that the
method __validate __of interface __ValidityChecker __needs to return a
boolean with semantics “reject!” in addition to reporting the
violation. Also, I believe this check should control the “enabled”
status of the component on the drop-down menu, not be an after-the-fact
error logger.
Thoughts?


Read this topic online here:
https://forum.vassalengine.org/t/check-ins-for-3-2/4306/30


messages mailing list
messages@vassalengine.org
vassalengine.org/mailman/listinfo/messages


No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1411 / Virus Database: 1522/3962 - Release Date:
10/19/11


No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1411 / Virus Database: 1522/3962 - Release Date:
10/19/11


Brent Easton
Analyst/Programmer
University of Western Sydney
Email: b.easton@exemail.com.au


messages mailing list
messages@vassalengine.org
vassalengine.org/mailman/listinfo/messages


messages mailing list
messages@vassalengine.org
vassalengine.org/mailman/listinfo/messages


No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1411 / Virus Database: 1522/3962 - Release Date: 10/19/11


No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1411 / Virus Database: 1522/3962 - Release Date: 10/19/11


Brent Easton
Analyst/Programmer
University of Western Sydney
Email: b.easton@exemail.com.au

svn-7934 has changes to ConfigureTree and AutoExecGKC to enforce SingleChildInstance on Add & Paste actions. SingleChildInstanceOnline is the interface that triggers the check, and it includes a static function delegate for classes wishing to implement the interface. (Name suggestions welcome.)

Thus spake “Brent Easton”:

From memory, the validators only run when you try and save the module.

Oh. That seems broken. Is letting you create something invalid, but then
not save it, desirable behavior?


J.

I’m not saying that’s good, just that that’s the way it was designed by Rodney as a static validator on save. Neither does it prevent the save, it just warns of errors. You may in fact have a reason for the ‘invalid’ structures.

*********** REPLY SEPARATOR ***********

On 20/10/2011 at 2:08 AM Joel Uckelman wrote:

Thus spake “Brent Easton”:

From memory, the validators only run when you try and save the module.

Oh. That seems broken. Is letting you create something invalid, but then
not save it, desirable behavior?


J.


Brent Easton
Analyst/Programmer
University of Western Sydney
Email: b.easton@exemail.com.au

Thus spake pgeerkens:

I could use a good word with Joel to get this into the trunk sooner than
later. (And I did 90% of the heavy lifting last weekend bringing my branch
up-to-date with everything you and he have done in the trunk up to Sunday
night.)

I’m not sitting on it because I think there’s someting wrong with it,
I’m sitting on it because it’s longer than the other commits and I
haven’t had a sufficiently large chunk of time to grok it in one go.
It does not help that we’re in the middle of moving house from Amsterdam
to Tilburg right now, and that my wife Sara is expecting in about three
weeks.

So, to summarize: It’s my fault. I’m sorry. I’ll try to get to it soon. :slight_smile:


J.

Hi Joel,

Don’t take me so seriously. :slight_smile:

I brought my branch completely up to date last weekend, (and it was several hours of work, which attests to how busy we have both been), as a way both of helping you out and getting my hands on the recent work by you and Brent. I will get my branch up to date again this weekend also.

You are in for a very special time soon. My eldest daughter turned 20 two weeks ago, and I was fortuante enough to have 8 weeks of paternal leave when that was still rare (in Caada at least). The year she was 4 I had a gaming group meeting at my house on Friday evenings, and I collected a bunch of spare counters (The HQ’s from AH-Waterloo maybe) in a plastic container for her to play with on an old mapboard. She was thrilled.

Remember to make time to enjoy these moments, as they pass all too quickly. :smiley:

Thus spake pgeerkens:

svn-7934 has changes to __ConfigureTree __and __AutoExecGKC __to enforce
SingleChildInstance on Add & Paste actions.
SingleChildInstanceOnline is the interface that triggers the check,
and it includes a static function delegate for classes wishing to
implement the interface. (Name suggestions welcome.)

Merged to trunk@7952, except 7934 which depends on the Attribute code
which I haven’t merged yet.


J.