Re: [Rails-devel] 1856 bugs in Rails 1.3 -- undo duringCGRformation hangs game

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

Yes, I agree that finding the seller is the catch.
MoveableHolder requires all portfolios (potential sellers) to have a name,
so perhaps what we need is a Map to link names to portfolios.

Thinking about it: identifying the seller and the share type bought is only
role currently fulfilled by passing the cert (ID), because after extracting
that info the cert ID itself is ignored. So why not pass seller name and
share type explicitly? Detecting the wrong seller is exactly the problem we
face.

One other issue then is to make the old and new version of BuyCertificate
compatible for deserialization, because I would hate to invalidate all my
test cases. But that should be doable.

Erik.

-----Original Message-----
From: Stefan Frey [mailto:ste...@we...] 
Sent: Sunday 18 July 2010 20:04
To: Development list for Rails: an 18xx game
Subject: Re: [Rails-devel] 1856 bugs in Rails 1.3 -- undo duringCGRformation
hangs game

Sorry: I did not disregard your observation, I forgot to go deeper into the 
details of the buy process:

You are right that you select the actual shares by type from the portfolio
of 
the seller.
But how do you define the seller ("from")?
	PublicCertificateI cert = action.getCertificate();
        Portfolio from = cert.getPortfolio();

This is the owner of the certificate used in the action, which was stored by

the ID.

ACTION: Player B buys a 10% share, Rails select #2 (buys are stored by id),
this moves the 10% share from A to B, leaving 10% in the pool!

For the case below, what happens exactly is that you have stored certificate

#2. The owner is still A. Thus you have wrongly defined from and then you 
select one the shares from the wrong owner.

Thus another fix would have been to use the from field of the action instead

of the from of certificate, but this does not go to the root of the problem:

The real problem is that in general you have to decide either to use
reference 
by ID or reference by type, but not a mixture of both (at least for the 
storage of actions).

I am not really decided, which direction I prefer, in general I would prefer

IDs, but this is not by a wide margin. You will know better, which method is

more widespread in Rails so far, thus I follow your decision.

The good thing will be that in both cases the ordering of interchangeable 
objects will play no role anymore. By type the ordering is meaningless, by
ID 
there is always an order defined. 

Stefan



On Sunday 18 July 2010 19:26:06 Erik Vos wrote:
> OK, it's getting clear to me now.
>
> You disregard my observation that the certificate ID specified in a buy
> action is NOT used, but that the certificate transferred is selected
> afresh. It's not so easy to sort out if this second certificate selection
> process will give the same result as the first, or not. Neither is it
clear
> to me if this detail makes the situation better or worse.
>
> In any case, I'm getting more and more convinced that the use of unique
IDs
> is the weak spot here, and that BuyCertificate should be modified to
> specify shares in a similar way as SellShares does.
>
> Erik.
>
> -----Original Message-----
> From: Stefan Frey [mailto:ste...@we...]
> Sent: Sunday 18 July 2010 00:07
> To: Development list for Rails: an 18xx game
> Subject: Re: [Rails-devel] 1856 bugs in Rails 1.3 -- undo during
> CGRformation hangs game
>
> Erik:
> as a follow up to your current fixes:
> Maybe it is my fault that I am not able to fully explain the issue.
> I do NOT claim that there is anything wrong with your fixes, thus
> I will try to focus on why I think removing the mixture of referring by
> type
>
> and by id protects against several cases of (future) potential problems.
>
> There are two (main) cases two distinguish:
> A) The interchangable objects (like 10% shares) are stored in an ordered
> collection (like an arraylist).
>
> Then the mixing of actions that refer by ID and by type is a (necessary,
> but
>
> not sufficient) condition to the undo/reload-related problems we had.
> The additional conditions are that the ordering of the storage collection
> is
>
> not observed during undos and that undos are not stored in the save files.
>
> I will give a simple example for that:
> START: Player A has two 10% shares of company B&O (order in the portfolio
> #1
>
> and #2). The pool is empty.
> ACTION: Player A sells a 10% share, Rails selects #1
> UNDO: order in the portfoliio is now (#2, #1)
> ACTION: Player A sells a 10% share, Rails selects #2
> ACTION: Player B buys a 10% share from the Pool, Rails selects #2
>
> On Reload the sequence is:
> ACTION: Player A sells a 10% share, Rails selects #1 (sells are stored by
> type)
> ACTION: Player B buys a 10% share, Rails select #2 (buys are stored by
id),
> this moves the 10% share from A to B, leaving 10% in the pool!
>
> Thus fixing of any of the three conditions (mixing reference type,
ordering
> of
> portfolios, non-saving of undos) above potentially prevents the problem.
>
> B) The interchangable objects are stored in an un-ordered collection (liek
> a
>
> HashSet or HashMap)
>
> Here the problem arises much simpler: Here mixing of id and type
> referencing
>
> alone is sufficient to cause a reload problem. Now not even a undo is
> necessary.
>
> Again a simple example:
> START: Player A has two 10% shares of company B&O (unordered portfolio #1
> and
> #2). The pool is empty.
> ACTION: Player A sells a 10% share, Rails selects (randomly) #2
> ACTION: Player B buys a 10% share, Rails selects #2
>
> On Reload the sequence can be (in 50% of the cases):
> ACTION: Player A sells a 10% share, Rails selects (randomly) #1 (sells are
> stored by type)
> ACTION: Player B buys a 10% share, Rails selects #2  (buys are stored by
> id),
> this moves a 10% share from A to B, leaving 10% in the pool!
>
> This problem does not occur (at least it was not observed so far), but it
> can
> only be avoided by either not mixing referencing by id and by type or
never
> use an unordered collection.
>
> It is mainly case B) that I want to protects us from, as an unordered
> collection seems a valid choice for interchangable objects.
>
> But I agree that it might be the best to wait for the time, where the
> downward
> compatibility will be broken anyway.
>
> Stefan
>
> On Saturday 17 July 2010 13:55:35 you wrote:
> > Subject: Re: [Rails-devel] 1856 bugs in Rails 1.3 -- undo during CGR
> > formation
> > hangs game
> > Date: Monday 21 June 2010
> > From: Stefan Frey <ste...@we...>
> > To: "Development list for Rails: an 18xx game"
> > <rai...@li...>
> >
> > 1) The BuyCertificates action stores certificate by id
> > 2) The SellShares action only stores percentages sold
> >
> > 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
> >
> > [EV] IIRC, in a follow-up you suggested to always use certificate IDs in
> > the share buying and selling action.
> >
> > However, there is a reason behind this difference. Whereas you always
buy
> > *certificates* (normally one), you don't sell certificates but *shares*.
> > The share/certificate distinction makes no difference except in the case
> > where you sell half a president's certificate.
> > Say player A has a 20% presidency and player B has 2 10% shares. Player
A
> > can then sell a 10% share - even if he does not own a 10% certificate.
> > That's why I think selling can be done in no other way than by selling
> > *shares*.
> >
> > Although certificates are normally bought one by one, the 1830 brown
zone
> > case requires enabling more than one cert at a time. As you say, the
cert
> > type is passed around via its ID, but this ID is not actually used when
>
> the
>
> > purchase is executed: one (or more) certs are then searched afresh in
the
> > selling portfolio.
> >
> > I have not yet fully grasped why you conclude that this difference
> > between selling and buying logic did contribute to the Undo-caused
> > save/restore problem that has (hopefully) just been fixed. Could it be
> > the possibility that a share offered for buying is not the share
actually
> > bought (as explained above)? If so, that should be easy to fix: just buy
> > the share with the ID that has been passed around, and then (in the
brown
> > case) search for additional shares if requested.
> > (It might even have been that way originally; I vaguely remember having
> > changed this to simplify the code - that might then have been a
misguided
> > simplification.)
> >
> > If there would be a real need (or strong wish) to align buying and
> > selling on this aspect, I think we should go for using shares (rather
> > than certificates) all the way. But I'm not yet convinced that there is
a
>
> strong
>
> > case for such a change. In any case, such a change would need careful
> > precautions to avoid invalidating all current saved files.
> >
> > Erik
>
>
---------------------------------------------------------------------------
>- --
> This SF.net email is sponsored by Sprint
> What will you do first with EVO, the first 4G phone?
> Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel
>
>
>
---------------------------------------------------------------------------
>--- This SF.net email is sponsored by Sprint
> What will you do first with EVO, the first 4G phone?
> Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel



----------------------------------------------------------------------------
--
This SF.net email is sponsored by Sprint
What will you do first with EVO, the first 4G phone?
Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first
_______________________________________________
Rails-devel mailing list
Rai...@li...
https://lists.sourceforge.net/lists/listinfo/rails-devel