rails-devel

[Rails-devel] 1889 Bug (and patch): D Private doesn't get free tile lays

[Bill R. <ro...@gm...>]

In a game I am playing, we encountered a bug in 1.4.2 that is not in 1.4.1.  Specifically, there is a company with the D private, which allows free tiles to be laid in the mountains (in rails XML, these tile lays are free="yes" and extra="no").  The attached .rails file exhibits the problem: Brad's TR does not get offered a free tile lay in F5.

Tracking the problem down lead to the method getSpecialTileLays in OperatingRound: when testing to see if special tile lays with extra="no" are possible, this method checks to see if the tile specified in the SpecialTileLay can be laid.  Unfortunately in the 1889 Private D is not restricted to a specific tile, so the action wasn't being added to the list.  The attached patch changes this to a different (and I think equivalent?) method of testing if normal tile lays are still available -- I check that the map tileLaysPerColour is not empty (as is done by the method areTileLaysPossible when it tests if normal tile lays are available).

It is not clear to me that this is the best solution.  Other solutions might be:

1) Add three SpecialTileLays to Private D in 1889, one for each available tile (7,8,9, in this case)
2) Change SpecialTileLay to keep a list of available tiles, with corresponding changes to OperatingRound and LayTile
3) If the tile in SpecialTileLay is null, treat this as no restriction: get the lists of all available tiles in the restricted hexes and check to see if any of them are valid, adding LayTile actions as appropriate.

Bill Rosgen

Re: [Rails-devel] 1889 Bug (and patch): D Private doesn't get free tile lays

[Stefan F. <ste...@we...>]

On the specific issue:
I am responding as I am the one to blame for 1889:
Indeed the method getSpecialTileLays in OperatingRound is the critical one.

This was a caused by a recent change by Erik, as he cleaned up the Special 
Tile Lay code at several places in the codebase. As he was (most likely) not 
aware of the D property of being able to lay any tile, he assumed that a tile 
id would always be specified.

I would add a check to your fix to capture this case and otherwise keep with 
the old flow, unless Erik has a better idea/other opinion?

In general:
There are two things we could improve to avoid such issues:
A) Better documentation of the XML methods (for example which attributes are 
mandatory or optional and would an omission implies).

B) This problem would have been captured in principle by my automated testing 
of existing game files. However it seems (to me) that those tests got a little 
sidetracked, since the time I got sidetracked by real world issues ;-)
I will update the test settings and include more docs on them in the wiki (by 
collecting my information sent out to the mailing list).

Stefan


On Monday, June 06, 2011 10:28:29 am Bill Rosgen wrote:
> In a game I am playing, we encountered a bug in 1.4.2 that is not in 1.4.1.
>  Specifically, there is a company with the D private, which allows free
> tiles to be laid in the mountains (in rails XML, these tile lays are
> free="yes" and extra="no").  The attached .rails file exhibits the
> problem: Brad's TR does not get offered a free tile lay in F5.
> 
> Tracking the problem down lead to the method getSpecialTileLays in
> OperatingRound: when testing to see if special tile lays with extra="no"
> are possible, this method checks to see if the tile specified in the
> SpecialTileLay can be laid.  Unfortunately in the 1889 Private D is not
> restricted to a specific tile, so the action wasn't being added to the
> list.  The attached patch changes this to a different (and I think
> equivalent?) method of testing if normal tile lays are still available --
> I check that the map tileLaysPerColour is not empty (as is done by the
> method areTileLaysPossible when it tests if normal tile lays are
> available).
> 
> It is not clear to me that this is the best solution.  Other solutions
> might be:
> 
> 1) Add three SpecialTileLays to Private D in 1889, one for each available
> tile (7,8,9, in this case) 2) Change SpecialTileLay to keep a list of
> available tiles, with corresponding changes to OperatingRound and LayTile
> 3) If the tile in SpecialTileLay is null, treat this as no restriction:
> get the lists of all available tiles in the restricted hexes and check to
> see if any of them are valid, adding LayTile actions as appropriate.
> 
> Bill Rosgen

Re: [Rails-devel] 1889 Bug (and patch): D Private doesn't get free tile lays

[Erik V. <eri...@xs...>]

Indeed I'm not familiar with 1889, so this case has escaped my attention.

I think the simplest solution would be to move the check into
checkNormalTileLay().
See attachment.  Works for me.  If you agree, I'll commit this way of fixing
the bug.

Erik.

> -----Oorspronkelijk bericht-----
> Van: Stefan Frey [mailto:ste...@we...]
> Verzonden: woensdag 8 juni 2011 21:05
> Aan: Development list for Rails: an 18xx game
> Onderwerp: Re: [Rails-devel] 1889 Bug (and patch): D Private doesn't get
free
> tile lays
> 
> On the specific issue:
> I am responding as I am the one to blame for 1889:
> Indeed the method getSpecialTileLays in OperatingRound is the critical
one.
> 
> This was a caused by a recent change by Erik, as he cleaned up the Special
Tile
> Lay code at several places in the codebase. As he was (most likely) not
aware
> of the D property of being able to lay any tile, he assumed that a tile id
would
> always be specified.
> 
> I would add a check to your fix to capture this case and otherwise keep
with
> the old flow, unless Erik has a better idea/other opinion?
> 
> In general:
> There are two things we could improve to avoid such issues:
> A) Better documentation of the XML methods (for example which attributes
> are mandatory or optional and would an omission implies).
> 
> B) This problem would have been captured in principle by my automated
> testing of existing game files. However it seems (to me) that those tests
got
> a little sidetracked, since the time I got sidetracked by real world
issues ;-) I
> will update the test settings and include more docs on them in the wiki
(by
> collecting my information sent out to the mailing list).
> 
> Stefan
> 
> 
> On Monday, June 06, 2011 10:28:29 am Bill Rosgen wrote:
> > In a game I am playing, we encountered a bug in 1.4.2 that is not in
1.4.1.
> >  Specifically, there is a company with the D private, which allows
> > free tiles to be laid in the mountains (in rails XML, these tile lays
> > are free="yes" and extra="no").  The attached .rails file exhibits the
> > problem: Brad's TR does not get offered a free tile lay in F5.
> >
> > Tracking the problem down lead to the method getSpecialTileLays in
> > OperatingRound: when testing to see if special tile lays with extra="no"
> > are possible, this method checks to see if the tile specified in the
> > SpecialTileLay can be laid.  Unfortunately in the 1889 Private D is
> > not restricted to a specific tile, so the action wasn't being added to
> > the list.  The attached patch changes this to a different (and I think
> > equivalent?) method of testing if normal tile lays are still available
> > -- I check that the map tileLaysPerColour is not empty (as is done by
> > the method areTileLaysPossible when it tests if normal tile lays are
> > available).
> >
> > It is not clear to me that this is the best solution.  Other solutions
> > might be:
> >
> > 1) Add three SpecialTileLays to Private D in 1889, one for each
> > available tile (7,8,9, in this case) 2) Change SpecialTileLay to keep
> > a list of available tiles, with corresponding changes to
> > OperatingRound and LayTile
> > 3) If the tile in SpecialTileLay is null, treat this as no restriction:
> > get the lists of all available tiles in the restricted hexes and check
> > to see if any of them are valid, adding LayTile actions as appropriate.
> >
> > Bill Rosgen
> 
> 
>
----------------------------------------------------------------------------
--
> EditLive Enterprise is the world's most technically advanced content
> authoring tool. Experience the power of Track Changes, Inline Image
Editing
> and ensure content is compliant with Accessibility Checking.
> http://p.sf.net/sfu/ephox-dev2dev
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel

Re: [Rails-devel] 1889 Bug (and patch): D Private doesn't get free tile lays

[Stefan F. <ste...@we...>]

I agree.

On Wednesday, June 08, 2011 10:17:30 pm Erik Vos wrote:
> Indeed I'm not familiar with 1889, so this case has escaped my attention.
> 
> I think the simplest solution would be to move the check into
> checkNormalTileLay().
> See attachment.  Works for me.  If you agree, I'll commit this way of
> fixing the bug.
> 
> Erik.
> 
> > -----Oorspronkelijk bericht-----
> > Van: Stefan Frey [mailto:ste...@we...]
> > Verzonden: woensdag 8 juni 2011 21:05
> > Aan: Development list for Rails: an 18xx game
> > Onderwerp: Re: [Rails-devel] 1889 Bug (and patch): D Private doesn't get
> 
> free
> 
> > tile lays
> > 
> > On the specific issue:
> > I am responding as I am the one to blame for 1889:
> > Indeed the method getSpecialTileLays in OperatingRound is the critical
> 
> one.
> 
> > This was a caused by a recent change by Erik, as he cleaned up the
> > Special
> 
> Tile
> 
> > Lay code at several places in the codebase. As he was (most likely) not
> 
> aware
> 
> > of the D property of being able to lay any tile, he assumed that a tile
> > id
> 
> would
> 
> > always be specified.
> > 
> > I would add a check to your fix to capture this case and otherwise keep
> 
> with
> 
> > the old flow, unless Erik has a better idea/other opinion?
> > 
> > In general:
> > There are two things we could improve to avoid such issues:
> > A) Better documentation of the XML methods (for example which attributes
> > are mandatory or optional and would an omission implies).
> > 
> > B) This problem would have been captured in principle by my automated
> > testing of existing game files. However it seems (to me) that those tests
> 
> got
> 
> > a little sidetracked, since the time I got sidetracked by real world
> 
> issues ;-) I
> 
> > will update the test settings and include more docs on them in the wiki
> 
> (by
> 
> > collecting my information sent out to the mailing list).
> > 
> > Stefan
> > 
> > On Monday, June 06, 2011 10:28:29 am Bill Rosgen wrote:
> > > In a game I am playing, we encountered a bug in 1.4.2 that is not in
> 
> 1.4.1.
> 
> > >  Specifically, there is a company with the D private, which allows
> > > 
> > > free tiles to be laid in the mountains (in rails XML, these tile lays
> > > are free="yes" and extra="no").  The attached .rails file exhibits the
> > > problem: Brad's TR does not get offered a free tile lay in F5.
> > > 
> > > Tracking the problem down lead to the method getSpecialTileLays in
> > > OperatingRound: when testing to see if special tile lays with
> > > extra="no" are possible, this method checks to see if the tile
> > > specified in the SpecialTileLay can be laid.  Unfortunately in the
> > > 1889 Private D is not restricted to a specific tile, so the action
> > > wasn't being added to the list.  The attached patch changes this to a
> > > different (and I think equivalent?) method of testing if normal tile
> > > lays are still available -- I check that the map tileLaysPerColour is
> > > not empty (as is done by the method areTileLaysPossible when it tests
> > > if normal tile lays are available).
> > > 
> > > It is not clear to me that this is the best solution.  Other solutions
> > > might be:
> > > 
> > > 1) Add three SpecialTileLays to Private D in 1889, one for each
> > > available tile (7,8,9, in this case) 2) Change SpecialTileLay to keep
> > > a list of available tiles, with corresponding changes to
> > > OperatingRound and LayTile
> > > 3) If the tile in SpecialTileLay is null, treat this as no restriction:
> > > get the lists of all available tiles in the restricted hexes and check
> > > to see if any of them are valid, adding LayTile actions as appropriate.
> > > 
> > > Bill Rosgen
> 
> ---------------------------------------------------------------------------
> - --
> 
> > EditLive Enterprise is the world's most technically advanced content
> > authoring tool. Experience the power of Track Changes, Inline Image
> 
> Editing
> 
> > and ensure content is compliant with Accessibility Checking.
> > http://p.sf.net/sfu/ephox-dev2dev
> > _______________________________________________
> > Rails-devel mailing list
> > Rai...@li...
> > https://lists.sourceforge.net/lists/listinfo/rails-devel

Re: [Rails-devel] 1889 Bug (and patch): D Private doesn't get free tile lays

[Erik V. <eri...@xs...>]

Done.

> -----Oorspronkelijk bericht-----
> Van: Stefan Frey [mailto:ste...@we...]
> Verzonden: woensdag 8 juni 2011 23:14
> Aan: Development list for Rails: an 18xx game
> Onderwerp: Re: [Rails-devel] 1889 Bug (and patch): D Private doesn't get
free
> tile lays
> 
> I agree.
> 
> On Wednesday, June 08, 2011 10:17:30 pm Erik Vos wrote:
> > Indeed I'm not familiar with 1889, so this case has escaped my
attention.
> >
> > I think the simplest solution would be to move the check into
> > checkNormalTileLay().
> > See attachment.  Works for me.  If you agree, I'll commit this way of
> > fixing the bug.
> >
> > Erik.
> >
> > > -----Oorspronkelijk bericht-----
> > > Van: Stefan Frey [mailto:ste...@we...]
> > > Verzonden: woensdag 8 juni 2011 21:05
> > > Aan: Development list for Rails: an 18xx game
> > > Onderwerp: Re: [Rails-devel] 1889 Bug (and patch): D Private doesn't
> > > get
> >
> > free
> >
> > > tile lays
> > >
> > > On the specific issue:
> > > I am responding as I am the one to blame for 1889:
> > > Indeed the method getSpecialTileLays in OperatingRound is the
> > > critical
> >
> > one.
> >
> > > This was a caused by a recent change by Erik, as he cleaned up the
> > > Special
> >
> > Tile
> >
> > > Lay code at several places in the codebase. As he was (most likely)
> > > not
> >
> > aware
> >
> > > of the D property of being able to lay any tile, he assumed that a
> > > tile id
> >
> > would
> >
> > > always be specified.
> > >
> > > I would add a check to your fix to capture this case and otherwise
> > > keep
> >
> > with
> >
> > > the old flow, unless Erik has a better idea/other opinion?
> > >
> > > In general:
> > > There are two things we could improve to avoid such issues:
> > > A) Better documentation of the XML methods (for example which
> > > attributes are mandatory or optional and would an omission implies).
> > >
> > > B) This problem would have been captured in principle by my
> > > automated testing of existing game files. However it seems (to me)
> > > that those tests
> >
> > got
> >
> > > a little sidetracked, since the time I got sidetracked by real world
> >
> > issues ;-) I
> >
> > > will update the test settings and include more docs on them in the
> > > wiki
> >
> > (by
> >
> > > collecting my information sent out to the mailing list).
> > >
> > > Stefan
> > >
> > > On Monday, June 06, 2011 10:28:29 am Bill Rosgen wrote:
> > > > In a game I am playing, we encountered a bug in 1.4.2 that is not
> > > > in
> >
> > 1.4.1.
> >
> > > >  Specifically, there is a company with the D private, which allows
> > > >
> > > > free tiles to be laid in the mountains (in rails XML, these tile
> > > > lays are free="yes" and extra="no").  The attached .rails file
> > > > exhibits the
> > > > problem: Brad's TR does not get offered a free tile lay in F5.
> > > >
> > > > Tracking the problem down lead to the method getSpecialTileLays in
> > > > OperatingRound: when testing to see if special tile lays with
> > > > extra="no" are possible, this method checks to see if the tile
> > > > specified in the SpecialTileLay can be laid.  Unfortunately in the
> > > > 1889 Private D is not restricted to a specific tile, so the action
> > > > wasn't being added to the list.  The attached patch changes this
> > > > to a different (and I think equivalent?) method of testing if
> > > > normal tile lays are still available -- I check that the map
> > > > tileLaysPerColour is not empty (as is done by the method
> > > > areTileLaysPossible when it tests if normal tile lays are
available).
> > > >
> > > > It is not clear to me that this is the best solution.  Other
> > > > solutions might be:
> > > >
> > > > 1) Add three SpecialTileLays to Private D in 1889, one for each
> > > > available tile (7,8,9, in this case) 2) Change SpecialTileLay to
> > > > keep a list of available tiles, with corresponding changes to
> > > > OperatingRound and LayTile
> > > > 3) If the tile in SpecialTileLay is null, treat this as no
restriction:
> > > > get the lists of all available tiles in the restricted hexes and
> > > > check to see if any of them are valid, adding LayTile actions as
> appropriate.
> > > >
> > > > Bill Rosgen
> >
> > ----------------------------------------------------------------------
> > -----
> > - --
> >
> > > EditLive Enterprise is the world's most technically advanced content
> > > authoring tool. Experience the power of Track Changes, Inline Image
> >
> > Editing
> >
> > > and ensure content is compliant with Accessibility Checking.
> > > http://p.sf.net/sfu/ephox-dev2dev
> > > _______________________________________________
> > > Rails-devel mailing list
> > > Rai...@li...
> > > https://lists.sourceforge.net/lists/listinfo/rails-devel
> 
>
----------------------------------------------------------------------------
--
> EditLive Enterprise is the world's most technically advanced content
> authoring tool. Experience the power of Track Changes, Inline Image
Editing
> and ensure content is compliant with Accessibility Checking.
> http://p.sf.net/sfu/ephox-dev2dev
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel