From: John D. G. <jd...@di...> - 2011-10-31 01:14:13
Attachments:
18EU_20110627_0156_Biff.rails
|
In a three-player game of 18EU under Rails 1.5.1 (save file attached), I found two minor bugs. 1) The train limit is not reduced to two until a 6-train is purchased (should happen when the 5-train is purchased). 2) A company that has a Pullman and hits the limit is given a choice of which train to discard (it's supposed to automatically be the Pullman). |
From: Erik V. <eri...@xs...> - 2011-10-31 11:05:58
|
This has worked in the past, so I suppose that the phase management changes have had some yet undetected side effects. Stefan, if you can wait for a few more days, I'll try to fix these problems for the new release. Erik. > -----Original Message----- > From: John David Galt [mailto:jd...@di...] > Sent: Monday, October 31, 2011 2:14 AM > To: rai...@li... > Subject: [Rails-devel] 18EU: Two remaining bugs related to train limit. > > In a three-player game of 18EU under Rails 1.5.1 (save file attached), I found > two minor bugs. > > 1) The train limit is not reduced to two until a 6-train is purchased (should > happen when the 5-train is purchased). > > 2) A company that has a Pullman and hits the limit is given a choice of which > train to discard (it's supposed to automatically be the Pullman). |
From: Stefan F. <ste...@we...> - 2011-10-31 11:07:55
|
Thanks for that bug report. Some notes for Erik: I do not consider this error as minor, it seems to be a more general issue. I had a quick look at this at seems that this goes back to the methods getCurrentTrainLimit and getTrainLimit(int index) inside PublicCompany It seems that your changes of the PhaseManagement got this out of synch. Maybe this could refactored a little bit, as it requires many steps of function forwarding to get to those methods and involves some conversion to integer and using that as an array index, which most likely is the reason for that problem. I believe a method with something like getTrainLimit(Phase phase, PublicCompany company) would be much safer. If this is located in Phase, PhaseManager or PublicCompany is a matter of taste. Another possibility is to have the phase change trigger a change of an integer state variable that stores the number of trains available for each company. This problem could potentially arise in other games than 18EU. Unfortunately the reduction of the train limit seems to be unreported in the game report, thus it is not covered by our automated tests, so maybe this could be added too. During debugging I realized that the train limit is not shown in the Phase menu, so it appears that is shown nowhere. A (minor) bug is that the tile colors are shown as null for those phases, where the tile laying does not change. I will wait with the new release for a fix from your side, unless you tell me that this bug will require a substantial amount of work and would delay a new release for several days. Stefan Methods Quoted here: /** Get the current maximum number of trains got a given limit index. * @parm index The index of the train limit step as defined for the current phase. Values start at 0. * <p>N.B. the new style limit steps per phase start at 1, * so one must be subtracted before calling this method. */ protected int getTrainLimit(int index) { return trainLimit[Math.min(index, trainLimit.length - 1)]; } public int getCurrentTrainLimit() { return getTrainLimit(gameManager.getCurrentPhase().getTrainLimitIndex()); } On Monday, October 31, 2011 02:14:03 am John David Galt wrote: > In a three-player game of 18EU under Rails 1.5.1 (save file attached), I > found two minor bugs. > > 1) The train limit is not reduced to two until a 6-train is purchased > (should happen when the 5-train is purchased). > > 2) A company that has a Pullman and hits the limit is given a choice of > which train to discard (it's supposed to automatically be the Pullman). |
From: Erik V. <eri...@xs...> - 2011-10-31 11:39:53
|
Stefan, Thanks for your analysis. Today I don't have much time, but tomorrow I'll try to sort out where we are and what I can do about it. Erik. > -----Original Message----- > From: Stefan Frey [mailto:ste...@we...] > Sent: Monday, October 31, 2011 12:11 PM > To: Development list for Rails: an 18xx game > Subject: Re: [Rails-devel] 18EU: Two remaining bugs related to train limit. > > Thanks for that bug report. > > Some notes for Erik: > > I do not consider this error as minor, it seems to be a more general issue. > > I had a quick look at this at seems that this goes back to the methods > getCurrentTrainLimit and getTrainLimit(int index) inside PublicCompany > > It seems that your changes of the PhaseManagement got this out of synch. > Maybe this could refactored a little bit, as it requires many steps of function > forwarding to get to those methods and involves some conversion to integer > and using that as an array index, which most likely is the reason for that > problem. > > I believe a method with something like getTrainLimit(Phase phase, > PublicCompany company) would be much safer. If this is located in Phase, > PhaseManager or PublicCompany is a matter of taste. Another possibility is > to have the phase change trigger a change of an integer state variable that > stores the number of trains available for each company. > > This problem could potentially arise in other games than 18EU. Unfortunately > the reduction of the train limit seems to be unreported in the game report, > thus it is not covered by our automated tests, so maybe this could be added > too. > > During debugging I realized that the train limit is not shown in the Phase > menu, so it appears that is shown nowhere. A (minor) bug is that the tile > colors are shown as null for those phases, where the tile laying does not > change. > > I will wait with the new release for a fix from your side, unless you tell me > that this bug will require a substantial amount of work and would delay a new > release for several days. > > Stefan > > Methods Quoted here: > > /** Get the current maximum number of trains got a given limit index. > * @parm index The index of the train limit step as defined for the current > phase. Values start at 0. > * <p>N.B. the new style limit steps per phase start at 1, > * so one must be subtracted before calling this method. > */ > protected int getTrainLimit(int index) { > return trainLimit[Math.min(index, trainLimit.length - 1)]; > } > > public int getCurrentTrainLimit() { > return > getTrainLimit(gameManager.getCurrentPhase().getTrainLimitIndex()); > } > > > > > > On Monday, October 31, 2011 02:14:03 am John David Galt wrote: > > In a three-player game of 18EU under Rails 1.5.1 (save file attached), I > > found two minor bugs. > > > > 1) The train limit is not reduced to two until a 6-train is purchased > > (should happen when the 5-train is purchased). > > > > 2) A company that has a Pullman and hits the limit is given a choice of > > which train to discard (it's supposed to automatically be the Pullman). > > ---------------------------------------------------------------------------- -- > Get your Android app more play: Bring it to the BlackBerry PlayBook > in minutes. BlackBerry App World™ now supports Android™ Apps > for the BlackBerry® PlayBook™. Discover just how easy and simple > it is! http://p.sf.net/sfu/android-dev2dev > _______________________________________________ > Rails-devel mailing list > Rai...@li... > https://lists.sourceforge.net/lists/listinfo/rails-devel |
From: Erik V. <eri...@xs...> - 2011-11-01 13:25:57
|
It turned out to be a typo in the (new) 18EU phase configuration. The train limit step for phase 5 and beyond was incorrectly specified as "2", it should have been "3". My bad. I have added the train limits to Info|Company (per company), and the train limit steps to Info|Phase (per phase). I have also fixed the allowed tile lay colours for phases where that info was the same as for previous phases, where it was reported as "null". I see no great need for refactoring. Train limits are defined per PublicCompany, so there is nothing wrong with the current high-level call getCurrentTrainLimit(). The number of calls could be reduced by one by merging it with getTrainLimit (index). It's mainly the new train limit *step* mechanism that has made getting the train limit a bit more complex than it was before. Erik. > -----Original Message----- > From: Erik Vos [mailto:eri...@xs...] > Sent: Monday, October 31, 2011 12:40 PM > To: 'Development list for Rails: an 18xx game' > Subject: Re: [Rails-devel] 18EU: Two remaining bugs related to train limit. > > Stefan, > > Thanks for your analysis. > Today I don't have much time, but tomorrow I'll try to sort out where we are > and what I can do about it. > > Erik. > > > -----Original Message----- > > From: Stefan Frey [mailto:ste...@we...] > > Sent: Monday, October 31, 2011 12:11 PM > > To: Development list for Rails: an 18xx game > > Subject: Re: [Rails-devel] 18EU: Two remaining bugs related to train > limit. > > > > Thanks for that bug report. > > > > Some notes for Erik: > > > > I do not consider this error as minor, it seems to be a more general > issue. > > > > I had a quick look at this at seems that this goes back to the methods > > getCurrentTrainLimit and getTrainLimit(int index) inside PublicCompany > > > > It seems that your changes of the PhaseManagement got this out of synch. > > Maybe this could refactored a little bit, as it requires many steps of > function > > forwarding to get to those methods and involves some conversion to > > integer and using that as an array index, which most likely is the > > reason for > that > > problem. > > > > I believe a method with something like getTrainLimit(Phase phase, > > PublicCompany company) would be much safer. If this is located in > > Phase, PhaseManager or PublicCompany is a matter of taste. Another > > possibility is to have the phase change trigger a change of an integer > > state variable > that > > stores the number of trains available for each company. > > > > This problem could potentially arise in other games than 18EU. > Unfortunately > > the reduction of the train limit seems to be unreported in the game > report, > > thus it is not covered by our automated tests, so maybe this could be > added > > too. > > > > During debugging I realized that the train limit is not shown in the > > Phase menu, so it appears that is shown nowhere. A (minor) bug is that > > the tile colors are shown as null for those phases, where the tile > > laying does not change. > > > > I will wait with the new release for a fix from your side, unless you > > tell > me > > that this bug will require a substantial amount of work and would > > delay a > new > > release for several days. > > > > Stefan > > > > Methods Quoted here: > > > > /** Get the current maximum number of trains got a given limit index. > > * @parm index The index of the train limit step as defined for > > the > current > > phase. Values start at 0. > > * <p>N.B. the new style limit steps per phase start at 1, > > * so one must be subtracted before calling this method. > > */ > > protected int getTrainLimit(int index) { > > return trainLimit[Math.min(index, trainLimit.length - 1)]; > > } > > > > public int getCurrentTrainLimit() { > > return > > getTrainLimit(gameManager.getCurrentPhase().getTrainLimitIndex()); > > } > > > > > > > > > > > > On Monday, October 31, 2011 02:14:03 am John David Galt wrote: > > > In a three-player game of 18EU under Rails 1.5.1 (save file > > > attached), I found two minor bugs. > > > > > > 1) The train limit is not reduced to two until a 6-train is > > > purchased (should happen when the 5-train is purchased). > > > > > > 2) A company that has a Pullman and hits the limit is given a choice > > > of which train to discard (it's supposed to automatically be the Pullman). > > > > > ---------------------------------------------------------------------------- > -- > > Get your Android app more play: Bring it to the BlackBerry PlayBook in > > minutes. BlackBerry App World™ now supports Android™ Apps > > for the BlackBerry® PlayBook™. Discover just how easy and > > simple it is! http://p.sf.net/sfu/android-dev2dev > > _______________________________________________ > > Rails-devel mailing list > > Rai...@li... > > https://lists.sourceforge.net/lists/listinfo/rails-devel > > > ---------------------------------------------------------------------------- -- > Get your Android app more play: Bring it to the BlackBerry PlayBook in > minutes. BlackBerry App World™ now supports Android™ Apps for > the BlackBerry® PlayBook™. Discover just how easy and simple it is! > http://p.sf.net/sfu/android-dev2dev > > _______________________________________________ > Rails-devel mailing list > Rai...@li... > https://lists.sourceforge.net/lists/listinfo/rails-devel |
From: John D. G. <jd...@di...> - 2011-11-02 18:12:33
|
On 2011-11-01 06:25, Erik Vos wrote: > It turned out to be a typo in the (new) 18EU phase configuration. The train > limit step for phase 5 and beyond was incorrectly specified as "2", it > should have been "3". My bad. You mean the other way around, right? |
From: Dr. M. B. <dr....@t-...> - 2011-11-02 18:43:27
|
No :) Ist not the limit itself but the steps regarding the limit. So step 1 is Train limit 4 Step 2 is train limit 3 Step 3 is train limit 2 Regards, Martin -----Ursprüngliche Nachricht----- Von: John David Galt [mailto:jd...@di...] Gesendet: Mittwoch, 2. November 2011 19:12 An: Development list for Rails: an 18xx game Betreff: Re: [Rails-devel] 18EU: Two remaining bugs related to train limit. On 2011-11-01 06:25, Erik Vos wrote: > It turned out to be a typo in the (new) 18EU phase configuration. The > train limit step for phase 5 and beyond was incorrectly specified as > "2", it should have been "3". My bad. You mean the other way around, right? ---------------------------------------------------------------------------- -- RSA® Conference 2012 Save $700 by Nov 18 Register now! http://p.sf.net/sfu/rsa-sfdev2dev1 _______________________________________________ Rails-devel mailing list Rai...@li... https://lists.sourceforge.net/lists/listinfo/rails-devel |
From: Erik V. <eri...@xs...> - 2011-11-02 20:20:52
|
I said: the train limit *step*, which increases 1,2,3. The actual major company train limits for these steps are 4,3,2 (and the minor limits are 2,1 - the third value is omitted because it doesn't matter). Erik > -----Original Message----- > From: John David Galt [mailto:jd...@di...] > Sent: Wednesday, November 02, 2011 7:12 PM > To: Development list for Rails: an 18xx game > Subject: Re: [Rails-devel] 18EU: Two remaining bugs related to train limit. > > On 2011-11-01 06:25, Erik Vos wrote: > > It turned out to be a typo in the (new) 18EU phase configuration. The > > train limit step for phase 5 and beyond was incorrectly specified as > > "2", it should have been "3". My bad. > > You mean the other way around, right? > > ---------------------------------------------------------------------------- -- > RSA® Conference 2012 > Save $700 by Nov 18 > Register now! > http://p.sf.net/sfu/rsa-sfdev2dev1 > _______________________________________________ > Rails-devel mailing list > Rai...@li... > https://lists.sourceforge.net/lists/listinfo/rails-devel |