question about piece movement

I’m working on bug 2921 and have a question.

One aspect of the bug is a piece tries to move into an occupied hex, but it appears to only move half-way in. Like the corner of the moving piece is right on the centre of the bottom piece.

Is this a common VASSAL mechanic used to display something like an unresolved conflict? E.g. if you move a piece onto another one to attack it, is it displayed as half-on the piece until the battle is resolved?

Or is there some other mechanic in VASSAL that causes a piece to only move half-way into a hex?

Thanks in advance!

Bug-fixer-in-training,
Ken

Piece control placement is detemined by a couple of things. If a grid or region
has been defined a piece can snap to, furthermore in the case of grid whether or
not the edges vertices or centers of the grid are valid snap locations for the
piece.

The other factor is if the piece�uses the Does Not Stack trait with the check
mark option to ignore grids enabled

From your description if an edge of a grid were an enabled snap point this could
make a piece appear to be halfway in the hex. The only other way would be no
snap points or DNS w/ �ignore grid�using old fashion drag n drop of the piece
and a bit of luck to place the piece half way in the hex


From: fil512 ken.stevens@sympatico.ca
To: messages@vassalengine.org
Sent: Thu, September 9, 2010 1:39:10 PM
Subject: [messages] Edit: [Developers] question about piece movement

[This message has been edited.]

I’m working on bug 2921 and have a question.

One aspect of the bug is a piece tries to move into an occupied hex, but
it appears to only move half-way in.� Like the corner of the moving
piece is right on the centre of the bottom piece.

Is this a common VASSAL mechanic used to display something like an
unresolved conflict?� E.g. if you move a piece onto another one to
attack it, is it displayed as half-on the piece until the battle is
resolved?

Or is there some other mechanic in VASSAL that causes a piece to only
move half-way into a hex?

Thanks in advance!

Bug-fixer-in-training,
Ken


Read this topic online here:
https://forum.vassalengine.org/t/question-about-piece-movement/3257/1

Tim,

Thanks for chiming in, but I don’t think this is it. The bug depends on the piece under it being invisible. If the piece under it is visible, then it stacks perfectly on top of it, but if the piece is invisible, then it is displaced by half a hex (both up and to the side). Note this is not an “expanded” stack issue. The spacing on expanded stacks is much smaller. When the unit underneath is visible, the two units are clearly stacked.

If the unit underneath starts invisible and then is made visible, the top piece will “snap” back into the centre of the square.

That’s what made me think that there must be some code somewhere that says under certain circumstances (like these two pieces can’t occupy the same hex) then if a player tries to move a piece into that hex, it will move it partway in until the conflict is resolved.

There’s no mechanic like that that you know of?

Ken
P.S. More details are available in the bug report

Ken,

No, there is no special code to offset pieces in hexes.

I suspect what is happening is that in this case, Vassal is either losing track of the correct origin, or, more likely, is not applying the correct offset from the center of the counter to the top left corner. This would appear to make the counter offset.

I have seen something like this quite a while back.

Actually, when you look closer, I think you will find that Vassal does not actually support multiple Invisible traits at all. The Invisibility of a counter is a single entity controlled and reported by the top Hideable trait on the counter.

B.

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

On 9/09/2010 at 4:01 PM fil512 wrote:

Tim,

Thanks for chiming in, but I don’t think this is it. The bug

depends on
the piece under it being invisible. If the piece under it is
visible,
then it stacks perfectly on top of it, but if the piece is
invisible,
then it is displaced by half a hex (both up and to the side).
Note this
is not an “expanded” stack issue. The spacing on expanded
stacks is
much smaller. When the unit underneath is visible, the two
units are
clearly stacked.

If the unit underneath starts invisible and

then is made visible, the
top piece will “snap” back into the centre of
the square.

That’s what made me think that there must be some code

somewhere that
says under certain circumstances (like these two pieces
can’t occupy the
same hex) then if a player tries to move a piece into
that hex, it will
move it partway in until the conflict is
resolved.

There’s no mechanic like that that you know of?

Ken
P.S. More

details are available in the bug report

“Tim M” wrote:

Piece control
placement is detemined by a couple of things. If a grid
or region
has
been defined a piece can snap to, furthermore in the case of grid

whether or
not the edges vertices or centers of the grid are valid snap
locations
for the
piece.

The other factor is if the piece�uses
the Does Not Stack trait with
the check
mark option to ignore grids
enabled

From your description if an edge of a grid were an enabled
snap point
this could
make a piece appear to be halfway in the hex.
The only other way would
be no
snap points or DNS w/ �ignore
grid�using old fashion drag n drop
of the piece
and a bit of luck
to place the piece half way in the hex


Read this topic online

here:
question about piece movement - #3 by fil512

Tim.

The bug in question comes from my 5th Fleet module. And it is clearly related to having several invisibility-traits in one piece. In order to make the module facilitate playing double-blind with moderator for ALL scenarios from the original game, I had to have some nations equipped with more than one invisibility-trait. That’s because several of the nations in the game changes side from scenario to scenario.

All the invisible-traits, in themselves, do work exactly as they should. They make the pieces invisible to players as specified in each trait and only the players defined as being able to make the piece invisible in each trait, have access to those specific traits. So far so good. From this, one can conclude that Vassal does indeed support more than one invisibility-trait in a single piece.

However, the problem arises once the invisible piece is being covered with a visible piece -in most cases a concealment-counter. If the invisible-trait in use is the one at the top of the piece-menu, then everything is OK. But if the invisible-trait in use is any one but the topmost, the covering, visible piece ends up sitting across the “northeastern” hexside, and then only on the opponet’s screen (the player the piece has been made invisible for).

Hope this gives you a better understanding of the problem. If not I reccommend you run the module and the “problem-demo” logfile. Both found at: mediafire.com/stroar

Hope you guys can solve this. And anyway I thank you for trying.

Cheers

Roar

All the invisible-traits, in themselves, do work exactly as they should.
They make the pieces invisible to players as specified in each trait and
only the players defined as being able to make the piece invisible in
each trait, have access to those specific traits. So far so good. From
this, one can conclude that Vassal does indeed support more than one
invisibility-trait in a single piece.

While it may appear to work for you in simple cases, I can assure you that, internally, Vassal does not fully support multiple Invisible traits.

If it works for your application, that’s fine, but beware if you try to apply more complex procedures like Triggers based on whether or not a piece is actually invisible.

Regards,
Brent.

Brent,

I believe it would be straightforward to change this behaviour so that it works as one would intuitively expect. If I coded a proposed fix (after writing a test for this “bug”) would you be available to review my proposed “fix”?

Thanks,
Ken

Brent,

I believe it would be straightforward to change this behaviour so that
it works as one would intuitively expect. If I coded a proposed fix
(after writing a test for this “bug”) would you be available to review
my proposed “fix”?

Sure, I am happy to review that.

What do you plan to do? I am not sure that there is a simple fix. The problem lies in handling the setProperty() and getProperty() calls for the Invisibility related properties, accross multiple Hideable traite without changing the values already returned. All Hideable traits in the same piece share the one set of property names.

I suspect a better way to go might be to enhance or replace the current Hideable trait with one that supports multiple ‘owners’, rather than trying to support multiple Hideable traite, each with their own ‘owner’.

Regards,
Brent.

Brent,

I’ve written a test that failed, then made code change that resolves bug 2921 as reported, and verified the test passed. For good measure I also ran the load file that the bug reporter provided and found that this fix resolves his bug as reported.

Please review my proposed change. It’s on the branch kstevens-3.1. The files to review are:
in test:
VASSAL.counters.HideableBug2921Test.java
in src:
VASSAL.counters.Hideable.java

Thanks!
Ken

Hey Brent,

Guess you’ve been too busy to review my code change. To make it easier for you, here’s the method in Hideable.java that I changed.

public Object getProperty(Object key) { if (HIDDEN_BY.equals(key)) { return hiddenBy; } else if (Properties.INVISIBLE_TO_ME.equals(key) && invisibleToMe()) { return Boolean.TRUE; } else if (Properties.INVISIBLE_TO_OTHERS.equals(key)) { return invisibleToOthers() ? Boolean.TRUE : Boolean.FALSE; } else if (Properties.VISIBLE_STATE.equals(key)) { return String.valueOf(invisibleToOthers()) + invisibleToMe() + piece.getProperty(key); } else { return super.getProperty(key); } }

The behaviour that I changed was INVISIBLE_TO_ME no longer stops at the first Hideable trait, but rather cascades through all traits performing an “or” operation, so that if at least one of the traits says I should be invisible, then I will be invisible. This resolves bug 2921 as reported. I have a couple of concerns about my fix though.

  1. Should the behaviour of getLocalizedProperty() match that of getProperty() ?
  2. If INVISIBLE_TO_ME behaves this way, should others work this way as well (e.g. INVISIBLE_TO_OTHERS and VISIBLE_STATE) ?

Thanks for your help!
Ken

Hi Ken,

  1. If a piece has Hideable traits, but is not invisible to me, then it previously returned Boolean.FALSE. With your changes, it will now return null. If any module is specifically checking that INVISIBLE_TO_ME != Boolean.FALSE, it will now fail. Unlikely, but possible.

  2. Yes, INVISIBLE_TO_OTHER would definitely need to do whatever INVISIBLE_TO_ME does.

  3. I think VISIBLE_STAT is fine, it already builds a state from all traits in the counter.

  4. HIDDEN_BY is problem also, only the top Hideable is ever exposed.

  5. Yes, getLocalizedProperty() should use the same algorithm to calculate a given property.

  6. There are performance implications of this change. The Invisibility properties used to be completely satisified by the top-most Hideable trait (often one of the top-most change). This change will require a full search through the entire trait stack in case there are additional Hideable traits. You may find that modules with extremely heavy trait per piece counts with an Invisible on top notice a performance hit. [NOTE that this is already an issue where there is NO Hideable trait at all in the Piece stack, we still have to search the entire getProperty() train every time to find that there is not Hideable trait. ]

Regards,
Brent.

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

On 15/09/2010 at 8:19 PM fil512 wrote:

Hey Brent,

Guess you’ve been too busy to review my code change. To make it easier
for you, here’s the method in Hideable.java that I changed.

Code:
public Object getProperty(Object key) {
if (HIDDEN_BY.equals(key)) {
return hiddenBy;
}
else if (Properties.INVISIBLE_TO_ME.equals(key) && invisibleToMe())
{
return Boolean.TRUE;
}
else if (Properties.INVISIBLE_TO_OTHERS.equals(key)) {
return invisibleToOthers() ? Boolean.TRUE : Boolean.FALSE;
}
else if (Properties.VISIBLE_STATE.equals(key)) {
return String.valueOf(invisibleToOthers()) +
invisibleToMe() + piece.getProperty(key);
}
else {
return super.getProperty(key);
}
}

The behaviour that I changed was INVISIBLE_TO_ME no longer stops at the
first Hideable trait, but rather cascades through all traits performing
an “or” operation, so that if at least one of the traits says I should
be invisible, then I will be invisible. This resolves bug 2921 as
reported. I have a couple of concerns about my fix though.

  1. Should the behaviour of getLocalizedProperty() match that of
    getProperty() ?
  2. If INVISIBLE_TO_ME behaves this way, should others work this way as
    well (e.g. INVISIBLE_TO_OTHERS and VISIBLE_STATE) ?

Thanks for your help!
Ken

OK. I will write a bunch of unit tests that assert the expected behavior
and then change the code to make these tests pass.

What’s the difference between getLocalizedProperty() and getProperty()? Is
there a way to implement that without all the copy/paste code?

-----Original Message-----
From: messages-bounces@vassalengine.org
[mailto:messages-bounces@vassalengine.org] On Behalf Of Brent Easton
Sent: September 16, 2010 6:31 PM
To: messages@vassalengine.org
Subject: Re: [messages] [Developers] Re: question about piece movement

Hi Ken,

  1. If a piece has Hideable traits, but is not invisible to
    me, then it previously returned Boolean.FALSE. With your
    changes, it will now return null. If any module is
    specifically checking that INVISIBLE_TO_ME != Boolean.FALSE,
    it will now fail. Unlikely, but possible.

  2. Yes, INVISIBLE_TO_OTHER would definitely need to do
    whatever INVISIBLE_TO_ME does.

  3. I think VISIBLE_STAT is fine, it already builds a state
    from all traits in the counter.

  4. HIDDEN_BY is problem also, only the top Hideable is ever exposed.

  5. Yes, getLocalizedProperty() should use the same algorithm
    to calculate a given property.

  6. There are performance implications of this change. The
    Invisibility properties used to be completely satisified by
    the top-most Hideable trait (often one of the top-most
    change). This change will require a full search through the
    entire trait stack in case there are additional Hideable
    traits. You may find that modules with extremely heavy trait
    per piece counts with an Invisible on top notice a
    performance hit. [NOTE that this is already an issue where
    there is NO Hideable trait at all in the Piece stack, we
    still have to search the entire getProperty() train every
    time to find that there is not Hideable trait. ]

Regards,
Brent.

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

On 15/09/2010 at 8:19 PM fil512 wrote:

Hey Brent,

Guess you’ve been too busy to review my code change. To
make it easier
for you, here’s the method in Hideable.java that I changed.

Code:
public Object getProperty(Object key) {
if (HIDDEN_BY.equals(key)) {
return hiddenBy;
}
else if (Properties.INVISIBLE_TO_ME.equals(key) &&
invisibleToMe())
{
return Boolean.TRUE;
}
else if (Properties.INVISIBLE_TO_OTHERS.equals(key)) {
return invisibleToOthers() ? Boolean.TRUE : Boolean.FALSE;
}
else if (Properties.VISIBLE_STATE.equals(key)) {
return String.valueOf(invisibleToOthers()) +
invisibleToMe() + piece.getProperty(key);
}
else {
return super.getProperty(key);
}
}

The behaviour that I changed was INVISIBLE_TO_ME no longer
stops at the
first Hideable trait, but rather cascades through all traits
performing
an “or” operation, so that if at least one of the traits
says I should
be invisible, then I will be invisible. This resolves bug 2921 as
reported. I have a couple of concerns about my fix though.

  1. Should the behaviour of getLocalizedProperty() match that of
    getProperty() ?
  2. If INVISIBLE_TO_ME behaves this way, should others work
    this way as
    well (e.g. INVISIBLE_TO_OTHERS and VISIBLE_STATE) ?

Thanks for your help!
Ken


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

getLocalizedProperty looks up possible translations.

They’re totally redundant if there is no translation. I’m not sure about
depricating one or the other though. I can’t think of a reason to have
both, however.

  • M.

On 19 September 2010 01:06, Ken Stevens ken.stevens@sympatico.ca wrote:

OK. I will write a bunch of unit tests that assert the expected behavior
and then change the code to make these tests pass.

What’s the difference between getLocalizedProperty() and getProperty()? Is
there a way to implement that without all the copy/paste code?

-----Original Message-----
From: messages-bounces@vassalengine.org
[mailto:messages-bounces@vassalengine.org] On Behalf Of Brent Easton
Sent: September 16, 2010 6:31 PM
To: messages@vassalengine.org
Subject: Re: [messages] [Developers] Re: question about piece movement

Hi Ken,

  1. If a piece has Hideable traits, but is not invisible to
    me, then it previously returned Boolean.FALSE. With your
    changes, it will now return null. If any module is
    specifically checking that INVISIBLE_TO_ME != Boolean.FALSE,
    it will now fail. Unlikely, but possible.

  2. Yes, INVISIBLE_TO_OTHER would definitely need to do
    whatever INVISIBLE_TO_ME does.

  3. I think VISIBLE_STAT is fine, it already builds a state
    from all traits in the counter.

  4. HIDDEN_BY is problem also, only the top Hideable is ever exposed.

  5. Yes, getLocalizedProperty() should use the same algorithm
    to calculate a given property.

  6. There are performance implications of this change. The
    Invisibility properties used to be completely satisified by
    the top-most Hideable trait (often one of the top-most
    change). This change will require a full search through the
    entire trait stack in case there are additional Hideable
    traits. You may find that modules with extremely heavy trait
    per piece counts with an Invisible on top notice a
    performance hit. [NOTE that this is already an issue where
    there is NO Hideable trait at all in the Piece stack, we
    still have to search the entire getProperty() train every
    time to find that there is not Hideable trait. ]

Regards,
Brent.

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

On 15/09/2010 at 8:19 PM fil512 wrote:

Hey Brent,

Guess you’ve been too busy to review my code change. To
make it easier
for you, here’s the method in Hideable.java that I changed.

Code:
public Object getProperty(Object key) {
if (HIDDEN_BY.equals(key)) {
return hiddenBy;
}
else if (Properties.INVISIBLE_TO_ME.equals(key) &&
invisibleToMe())
{
return Boolean.TRUE;
}
else if (Properties.INVISIBLE_TO_OTHERS.equals(key)) {
return invisibleToOthers() ? Boolean.TRUE : Boolean.FALSE;
}
else if (Properties.VISIBLE_STATE.equals(key)) {
return String.valueOf(invisibleToOthers()) +
invisibleToMe() + piece.getProperty(key);
}
else {
return super.getProperty(key);
}
}

The behaviour that I changed was INVISIBLE_TO_ME no longer
stops at the
first Hideable trait, but rather cascades through all traits
performing
an “or” operation, so that if at least one of the traits
says I should
be invisible, then I will be invisible. This resolves bug 2921 as
reported. I have a couple of concerns about my fix though.

  1. Should the behaviour of getLocalizedProperty() match that of
    getProperty() ?
  2. If INVISIBLE_TO_ME behaves this way, should others work
    this way as
    well (e.g. INVISIBLE_TO_OTHERS and VISIBLE_STATE) ?

Thanks for your help!
Ken


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


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

getLocalizedProperty looks up possible translations.

They’re totally redundant if there is no translation.� I’m not sure about depricating one or the other though.� I can’t think of a reason to have both, however.


- M.

On 19 September 2010 01:06, Ken Stevens <ken.stevens@sympatico.ca> wrote:

OK. �I will write a bunch of unit tests that assert the expected behavior

and then change the code to make these tests pass.



What’s the difference between getLocalizedProperty() and getProperty()? �Is

there a way to implement that without all the copy/paste code?


> -----Original Message-----
> From: [messages-bounces@vassalengine.org](mailto:messages-bounces@vassalengine.org)
> [mailto:[messages-bounces@vassalengine.org](mailto:messages-bounces@vassalengine.org)] On Behalf Of Brent Easton
> Sent: September 16, 2010 6:31 PM
> To: [messages@vassalengine.org](mailto:messages@vassalengine.org)
> Subject: Re: [messages] [Developers] Re: question about piece movement
>
> Hi Ken,
>
> 1. If a piece has Hideable traits, but is not invisible to
> me, then it previously returned Boolean.FALSE. With your
> changes, it will now return null. If any module is
> specifically checking that INVISIBLE_TO_ME != Boolean.FALSE,
> it will now fail. Unlikely, but possible.
>
> 2. Yes, INVISIBLE_TO_OTHER would definitely need to do
> whatever INVISIBLE_TO_ME does.
>
> 3. I think VISIBLE_STAT is fine, it already builds a state
> from all traits in the counter.
>
> 4. HIDDEN_BY is problem also, only the top Hideable is ever exposed.
>
> 5. Yes, getLocalizedProperty() should use the same algorithm
> to calculate a given property.
>
> 6. There are performance implications of this change. The
> Invisibility properties used to be completely satisified by
> the top-most Hideable trait (often one of the top-most
> change). This change will require a full search through the
> entire trait stack in case there are additional Hideable
> traits. You may find that modules with extremely heavy trait
> per piece counts with an Invisible on top notice a
> performance hit. [NOTE that this is already an issue where
> there is NO Hideable trait at all in the Piece stack, we
> still have to search the entire getProperty() train every
> time to find that there is not Hideable trait. ]
>
> Regards,
> Brent.
>
> *********** REPLY SEPARATOR �***********
>
> On 15/09/2010 at 8:19 PM fil512 �wrote:
>
> >Hey Brent,
> >
> >Guess you've been too busy to review my code change. �To
> make it easier
> >for you, here's the method in Hideable.java that I changed.
> >
> >
> >Code:
> > �public Object getProperty(Object key) {
> > � �if (HIDDEN_BY.equals(key)) {
> > � � �return hiddenBy;
> > � �}
> > � �else if (Properties.INVISIBLE_TO_ME.equals(key) &&
> invisibleToMe())
> >{
> > � � �return Boolean.TRUE;
> > � �}
> > � �else if (Properties.INVISIBLE_TO_OTHERS.equals(key)) {
> > � � �return invisibleToOthers() ? Boolean.TRUE : Boolean.FALSE;
> > � �}
> > � �else if (Properties.VISIBLE_STATE.equals(key)) {
> > � � � �return String.valueOf(invisibleToOthers()) +
> > � � � � � � � invisibleToMe() + piece.getProperty(key);
> > � � �}
> > � �else {
> > � � �return super.getProperty(key);
> > � �}
> > �}
> >
> >
> >
> >The behaviour that I changed was INVISIBLE_TO_ME no longer
> stops at the
> >first Hideable trait, but rather cascades through all traits
> performing
> >an "or" operation, so that if at least one of the traits
> says I should
> >be invisible, then I will be invisible. �This resolves bug 2921 as
> >reported. �I have a couple of concerns about my fix though.
> >
> >1) Should the behaviour of getLocalizedProperty() match that of
> >getProperty() ?
> >2) If INVISIBLE_TO_ME behaves this way, should others work
> this way as
> >well (e.g. INVISIBLE_TO_OTHERS and VISIBLE_STATE) ?
> >
> >Thanks for your help!
> >Ken
>
>
>
> _______________________________________________
> messages mailing list
> [messages@vassalengine.org](mailto:messages@vassalengine.org)
> [vassalengine.org/mailman/listinfo/messages](http://www.vassalengine.org/mailman/listinfo/messages)
>
>

_______________________________________________
messages mailing list
[messages@vassalengine.org](mailto:messages@vassalengine.org)
[vassalengine.org/mailman/listinfo/messages](http://www.vassalengine.org/mailman/listinfo/messages)



Thus spake Michael Kiefte:

getLocalizedProperty looks up possible translations.

They’re totally redundant if there is no translation. I’m not sure about
depricating one or the other though. I can’t think of a reason to have
both, however.

getLocalizedProperty is for display, getProperty is for doing computations.
If we need the value of a property as input for some algorithm, it helps
to have it in a known format, rather than in whatever random language was
set by the user. (Brent, correct me if I’m wrong about this.)


J.

Yes, Joel is right. getProperty and getLocalizedProperty should generally return the same values if no translation is in use. I can see that they don’t in Hideable, I am not sure why. What it is doing is causing the properties in traits underneath an active Invisible trait to be hidden.

getProperty should always return the actual values specified by the module designer for a property, regardless of any Translation is use. getProperty should generally be used internally within Vassal

getLocalizedProperty may return a translated value for a property and should generally only be used in reports to players etc. where translated values have been specified for a property.

Not all properties can be translated within a translation, it depends on the trait.

In the case of Hideable, none of the properties are translatable, so getProperty and getLocalizedProperty should be refactored to remove the duplicated code. Or have getLocalizedProperty() call getProperty(0 for those properties.

B.

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

On 20/09/2010 at 12:37 PM Joel Uckelman wrote:

Thus spake Michael Kiefte:

getLocalizedProperty looks up possible translations.

They’re totally redundant if there is no translation. I’m not sure about
depricating one or the other though. I can’t think of a reason to have
both, however.

getLocalizedProperty is for display, getProperty is for doing computations.
If we need the value of a property as input for some algorithm, it helps
to have it in a known format, rather than in whatever random language was
set by the user. (Brent, correct me if I’m wrong about this.)


J.

Yay! I like the sound of that.