Re: [Rails-devel] new bug with 1856 bonuses in rails 1.9 and later + partial fix

[games.bazin <gam...@or...>]

(Sorry, previous msg again sent in bogus format. I'm sending again as raw txt + I'm adding partial debuging info.)

Hello all.

I made some further research on this new 1856 bonus bug.

The situation before the first T5 is bought is :
GW and GT have the bridge bonus, one bridge bonus still available from GW (GW had initially purchased the bridge company)
LPS, CA and BBG have the tunnel bonus, no further tunnel available (LPS had initially purchased the tunnel company)

After the T5 is bought : normal behaviour :
bridge bonus is available from bank
tunnel bonus is not available

Bug situation appears after CGR formation. GW, GT, LPS, CA and BBG merge into CGR.

== in 1.9 :
CGR has 1 bridge and 1 tunnel
tunnel is available from LPS (!)
bridge is available from bank

== in 2.0RC1 and in origin/rails_2_develop and in origin/rails_2_maintenance :
CGR has 2 bridges and 2 tunnels (!) (duplicate bonus token is a known bug ; what is semi-surprising here is that CGR only gets 2 and not 3 tunnel bonuses)

bridge is available from bank
tunnel is available from LPS (!)


== in origin/eba_r2dev thank's to Stefan's last bug correction (*), the situation is a little different but still buggy :

CGR has 1 bridge and 1 tunnel (correct behaviour)
bridge is available from bank
tunnel is not available at all (it should be available from bank)

(*) i'm refering to :
daca42d adapted the change for removal of bonus tokens, still based on name/value, renamed equals to equalsBonus method


=> This is a result of two different bugs in the code if i'm not mistaken

1) in CGRFormationRound.java 
at some point in the code there is : 

if (sp.getId().equalsIgnoreCase(bonus.getName())) {
sp.makeResellable();
log.debug("BonusToken "+bonus.getName()+" made sellable again");
break;
}

The bug is that the condition in the if is always false and the bonus is never made sellable again.
The "if" should be :

if (sp.getName().equalsIgnoreCase(bonus.getName())) {
sp.makeResellable();
log.debug("BonusToken "+bonus.getName()+" made sellable again");
break;
}

If tested with this new code, origin/eba_r2dev reverts to 1.9 behaviour : 
CGR has 1 bridge and 1 tunnel (correct behaviour)
bridge is available from bank
tunnel is available from LPS (!)

2) The source of the "LPS instead of bank" bug comes from the buyTrain() function in OperatingRound_1856.java, at the first time a T5 train is bought.

There, 
if (postPhase.getId().equals("5")) {
// Make Bridge and Tunnel tokens buyable from the Bank.
for (SpecialProperty sp : gameManager.getCommonSpecialProperties()) { // EBA debug inserted here
if (sp instanceof SellBonusToken) {
SellBonusToken sbt = (SellBonusToken)sp;
// FIXME: Is it ipo or pool portfolio?
// Assume it is pool
sbt.setSeller(bank.getPool());
log.debug("SP "+sp.getId()+" is now buyable from the Bank");
}
}

The issue in this situation is that the "for" loop on SpecialProperty sp runs only once, for the bridge, but not for the tunnel.
I inserted a debug in the "for" loop to doublecheck, before the "if", and the for ran only once.
However, I'm not yet fluent enough in the code and in java to debug the gameManager.getCommonSpecialProperties() part.

Hope it helps


Note1 : to reproduce the bug, use the attached file. This is a 1.9 save file just before CGR creation. Then proceed with CGR creation (done, none, none, ok, none)
Note2 : the first T5 in this game is bought in OR5.1 by the THB
Eric.