From: Erik V. <eri...@xs...> - 2010-07-13 18:54:14
|
See below [EV2] > - From the code it seems that defining certificate types might help. > [EV] You mean different types for 5% and 10% shares? Or what else? On first > sight I don't see much value in that. In the code you twice (once for buying and selling) extract those shares that have common properties. Certificates currently differ only for percentages (or number shares), certificate counts and president property. It would be easier to refactor that into if there would be a certificateTypes. Or one could proxy that by a getCertificateTypes(), which returns a list of certificates, which represent the different classes. But the main value lies in the for Rails typical two-tier structure, where a type of something is configured first and then instances are created derived from the type (like TrainType, CompanyType) etc. I am much in favor of that approach as it allows a lot of flexibility going foward. [EV2] My thinking was, that certificates of *one* company only differ in share percentage and presidency, and these differences can easily be represented by just two attributes. In several games, train types and company types have many more and more profound differences. That's why I don't see that much added value in certificate types. But I'm not at all opposed to such refactoring. > > - The Buy action (still) do not store the certificate ids, if more than one > certificate is sold. The sell action does not store the ids at all. The > same > > is true for the Exchange for Share action. This poses the risk of reload > errors especially after undos. > > [EV] I remember you have discussed this before, but I haven't yet managed > to understand what exact problem storing certificate IDs would fix. Does > this relate to the known problem that items in lists are generally moved to > a different position after an undo? That shouldn't be difficult to fix, but > I haven't yet got to it. Pending that generic solution, I have fixed a few > apparent cases; there may be others, but I haven't any good examples. Sorry I thought you had mentioned that you already implemented that (but this referred to the brown stock spaces). Yes it is an alternative solution for the reload problems after undo. Advantages: A) It does not require the protection of order of the interchangable objects, which is somehow counterintuitive. B) It allows the use of HashMaps/HashSets to store the interchangable objects. But implementing both is the best protection, especially for future extensions, as - if I remember correctly - you need the order protection for the portfolio certificates already to keep the certificate ordering of the IPO stack in 1835 I could give it a try, but it might break the savefile compatability. If that is the case it should be moved to a later release. [EV2] In any case, sequence preservation is high on my next-to-do list (the main obstacle being a lack of test cases). > [EV] There already is the concept of "off-station" tokens, which e.g. > applies to the 18AL coalfield token. Please note, that the code carefully > distinguishes between "base tokens" and "bonus tokens". Not sure how much > value the existing TokenI interface really has, but there many other > useless interfaces that I would rather ditch than this one. That is exactly the problem TokenI: It is not totally useless (like some others. :-)), but still most of the time baseTokens and bonusTokens are treated separately. I was wondering for example, if I could replace the TokenI variables/methods for cities with a BaseToken signature and Rails would work identical: Or same question reverse: Is there a use case that requires a storage a BonusToken in a city object using the city's tokens variable? [EV2] Don't think so (though you can never know what creative 18xx designers will invent next year), but the reverse does occur. In 1870, base tokens can appear (without functional change) in off-station locations, so for that purpose alone I think we would need something like TokenI to specify the contents of off-station spots on tiles, because in other games BonusTokens may appear on such tiles. 18FL is a bit different in that base tokens may metamorphose into BonusTokens (hotels) - or one could say: that game's tokens have a dual nature: another case for TokenI? In general I would have no problem to change TokenI into BaseToken or BonusToken wherever appropriate - I'm just not so sure if we can rely upon our current understanding of appropriateness. Erik. |