Re: [Rails-devel] 1825 patch

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

Phil:
this already adds a lot of functionality to 1825. Those rules have some nasty 
difficulties, so I think it is good work!

I had a short glance on the code and overall it seems very reasonable.

A few comments below.
Stefan

> There is only one shared component (IE: not in the _1825 game specific
> folder and not in the 1825 data folder) that I've hit with these
> changes and that's the PublicCompany class and it's interface
> PublicCompanyI.  The change I've made here is to add a
> formationOrderIndex to the PublicCompany interface with the attendant
> getters and setters.  The reasoess is currently work in progress, as I 
intend to add true route checks in a non for doing this is that in 1825
> companies always operate in descending par value order.  When two
> companies share the same par value, they always operate in the order
> they were formed.  This value is a simple incrementing integer value
> that is a 0 for the first company at a given par, then a 1 for the
> next and so on.  This seemed the easiest way to control operating
> order for the OR's

I have not created a test case for that, but I would guess that you should 
change the formationOrderIndex from an Integer variable to an IntegerState 
variable, as otherwise it is likely that the mechanism will fail with 
undo-actions.

And I would prefer if everything related to the formationOrderIndex is kept in 
the subclass PublicCompany_1825, as it is only used by those. 
I am usually confused by variables in a base class, which are only used in 
subclass context.

> Route awareness and revenue calculation: Have a variety of issues.
> I'm reasonably confident I can fix these with the framework Stefan has
> put together, I just haven't got round to looking at what needs doing
> yet, so for the moment these two settings default to off.
>
Actually I only read your mail carefully after I have added the required code 
for 1825 revenue calculation. Sorry for not leaving that for you.
A good reason for that was that I had the idea for the 1825 issues already in 
mind during the implementation of the revenue modifier framework. 
And from this I knew that I had to change a few subtle issues there to allow 
the 1825 modifiers show the results correctly.
Without those the task would have been much harder.

So there are now two dynamic modifiers, which allow double heading and prevent 
termination at towns. Feel free to test and adapt them to your needs.

There is still one issue on revenue calculation: There is nothing preventing a 
train visiting two stations on a tile. The current method is to set the city 
attribute for all station of that tile to an identical value (in Tiles.xml). 
Erik already critized that and I am close to suggest that an attribute in 
TileSet.xml might be better (for example a VisitOnlyOnce attribute).

Route awareness is currently work in progress, as I intend to add true route 
checks in a hopefully not too distant future. As 1825 has some rule 
adjustments in that area, it is worth to wait for changes there. I intend to 
provide a modifier interface similar to those used for revenue calculation.