Re: [Rails-devel] 1856 bugs in Rails 1.3 -- undo during CGR formation hangs game

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

Erik:
Just some follow-up to the issue. I explain it with an example, as it 
might be an instructive case to remember for potential issues with the
undo-function in Rails.

This bug is easily reproducible under simplified conditions. I attach the save 
file for the example.

There are several steps involved (see example file)
A) Owner of AR (player David, owns 50%) sells a (single) certificate
B) Undo this action
C) Player A sells a (single) certificate again
D) Other player (Alice) buys that share from the pool
E) Save Game: 
Until then everything looks fine: David owns 40%. Alice owns 10%
F) Reload game: 
Now David owns only 30%, Alice owns 10%, the Pool has 10%!

What happened internally: You have to be aware, that certificates are 
internally identified by ids. Assume that David has single certificates 
called AR-1 to AR-3.

A) David sells 10% certificate (assume that this is certificate AR-1)
B) Undo that action (now AR-1 comes to the bottom of his certificates)
C) David sells 10% again, but now this is certificate AR-2
D) Alice buys AR-2 from the pool
E) save only saves actions, which were not undone (thus actions C and D)
F) on the reload for C) AR-1 gets sold to the Pool, but in D) Alice still buys 
AR-2, but the previous owner is now not the Pool, but David!

The reason is a combination of issues:
1) The BuyCertificates action stores certificate by id
2) The SellShares action only stores percentages sold
3) The Undoing of share moves does not keep the order of certificates

By changing any of the three the problem can be solved.
The main issue is mixing addressing interchangeable objects once by ids and in 
the other case by type. This can only work, if evey selection by type works 
deterministically (thus never from unordered collections etc.) and the undo 
mechanism strictly restores the previous ordering.

Stefan



On Sunday 20 June 2010 20:11:06 Stefan Frey wrote:
> Erik:
> some follow up on the issue with the broken save file in Aliza's bug
> report: As I had the hope I could easily make the file at least reloadable,
> I believe I found a trace to the potential problem:
>
> The save file breaks at the following action:
> Action 378 delft: SellShares: 1 of 10% CPR at $110 apiece
>
> But delft should still have the share, as he bought CPR twice before (at
> actions 238, 244) and sold only once (action 323).
>
> The problematic action is the following:
> Action 330 azure: BuyCertificate: CPR 10% share from Pool price=$110
>
> which gets executed on the reload as:
> 2010-06-20 19:58:33 DEBUG  Action (azure): BuyCertificate: CPR 10% share
> from Pool price=$110
> 2010-06-20 19:58:33 DEBUG  >>> Start MoveSet(index=330)
> 2010-06-20 19:58:33 INFO   azure buys a 10% share of CPR from delft for
> $110. 2010-06-20 19:58:33 DEBUG  Done: Move PublicCertificate: CPR 10%
> share from delft to azure
>
> Thus Rails assume that the certificate bought stems from delf instead from
> the Pool! It seems that action does not correctly identify the certificate
> bought. I checked that this is defined by the certificate ids.
>
> Thus I guess, that due to some undo the stack of certificates got mixed,
> either those in the IPO or in the possession of Delft (similar to the Baden
> issue in 1835). Thus in the game played the other certificate in the
> possession of Delft was sold to the Pool in 323 and then on the reload
> Rails moves the wrong one out of Delft portfolio at action 330 instead
> getting that out of the Pool.
>
> I hope this is of some help, however I do not know how to resolve that
> issue easily, as I do not know, what is required to keep the ordering of
> certificates in portfolios "undo-robust". It might be the better solution
> in the long run to identify the certificates by their properties (thus
> share percentage and previous owner) and be agnostic about which one
> exactly to take.
>
> At least one should check that the current owner of the certificate is
> identical to the previous owner implied by the action, but this does not
> fix the reload problem.
>
> I have attached two trimmed save files, one before and one after the error
> occurs.
>
> Stefan
>
> On Sunday 20 June 2010 14:54:55 Stefan Frey wrote:
> > Aliza:
> > thanks for the savefile.
> > Most of the issues have to be addressed by Erik, but for the revenue
> >
> > calculation I can give some feedback:
> > > (1)  When I undo a tile lay and do something else, the route is not
> > > recalculated.  If I don't pay attention and calculate manually, I get
> > > the run from the previous OR.
> >
> > Unfortunately I cannot recreate the issue you report here:
> > I have the same problem with reloading the file, thus I wonder if the
> > issue occured anytime during the game or only at the specific time you
> > originally saved the game?
> >
> > Do you remember what was the case (but I fear some questions are too
> > detailed):
> >
> > - You were in the revenue phase already, did undo and went to lay tile
> > directly (as the company did not have any token)? Or have you done twice
> > an undo? Was a special tile lay of a private company involved? Which
> > company was operating?
> >
> > - After undoing, did the revenue run disappear (as it should)?
> >
> > - Did the system calculate the revenue run (thus you got the messagebox
> > with the run), but did not take the new (different) tile lay into
> > account?
> >
> > - Or did simply nothing happened at all, thus the old revenue value was
> > still entered?
> >
> > I was able to confuse the tile/token laying mechansim by undos somehow,
> > but even in those cases the revenue calculation still worked. But maybe
> > fixing those errors might fix the (potentially missing) activation of the
> > revenue calculation as well. I will look into that.
> >
> > Stefan
> >
> >
> >
> > -------------------------------------------------------------------------
> >-- --- ThinkGeek and WIRED's GeekDad team up for the Ultimate
> > GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
> > lucky parental unit.  See the prize list and enter to win:
> > http://p.sf.net/sfu/thinkgeek-promo
> > _______________________________________________
> > Rails-devel mailing list
> > Rai...@li...
> > https://lists.sourceforge.net/lists/listinfo/rails-devel