Re: [Rails-devel] Stop properties

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

Erik:
I tried to change my coding in NetworkVertex accordingly.
Thanks a lot this will simplify the previous code needed to adapt the revenue 
code to the algorithm code substantially.

However as usual with coding one realizes some more thing.

I think having those attributes are one thing, checking if runThrough or runTo 
is possible still requires more than that.

The best solution would be adding additional methods for if a runThrough and a 
runTo is possible for a specific company. Thus it also checks the current 
token situation.

Thus something like

public boolean runThroughAllowedFor(PublicCompanyI company)
public boolean runToAllowedFor(PublicCompanyI company)

From my perspective you could then define the 
isRunToAllowed/isRunThroughAllowed for  as private or even remove it, as this 
is only required internally. Similar for the according methods in MapHex or 
Tile, they only need to be package private and not public.

Even better would be if  you could add similar methods. Then all values are 
retrieved from the stop object, instead of sometimes querying hex or station 
instead.

* public int getValueAt(PhaseI phase)
Retrieves the current value (either from the related station for cities/towns 
or via the MapHex for offboard defined values)

* public String getName()
Retrieves the name: Either from the hex or if undefined there from the related 
station.

Again this would allow to restrict access/visibility of a lot of related 
methods in Stop / Station / MapHex.


Some other remarks:

* Are you sure that the time of calling initStopProperties that all other 
objects are already setup correctly? And you should ensure that it is not 
possible to change the values of the runTo and runThrough attributes of hex 
and station after creation of the stop objects, otherwise there could be nasty 
surprises, if someone tries this in the future. The alternative is to go 
through the list of attributes each time the methods are called.

* I currently do not support the loop attribute. The 1825 not scoring twice 
rule is supported by an independent revenue modifier.
1860 has so many strange rules that it surely needs modifier(s), thus there is 
no need for such an attribute now. I suggest to remove or comment it out, as 
long as it not supported (again to avoid surprise).

Stefan


On Friday, August 12, 2011 02:47:02 pm Erik Vos wrote:
> I have implemented (but not yet pushed) the train stop properties that we
> have discussed a while ago, all described below.
> 
> My questions are (in particular so Stefan):
> Q1. Do you like it this way?  Is it usable?
> Q2. What shall I do to the repository?  Push the branch? Or push it merged
> with master?  Or first redo it all because you don't like it? ;-)
> Q3. See point 6: some algorithms classes currently pick up values from
> HexMap.  The current code requires that these values should be taken from
> Stop (see comments below).  Would that be OK for you?  Or should I add
> methods to HexMap and/or Tile that honour defaults?
> 
> Here is the overview (I'll put it into the wiki later):
> 
> 1. City has been renamed to Stop. This object represents a train stop on a
> certain tile that has been laid on a certain hex.
> Stop will become the primary source of all properties that are required to
> define routes and calculate revenues.
> (I have left Station as it is, because this object name equates to the
> corresponding tag names in Tile.xml,
> which I did not want to change after all.  I hope we can live with this
> slight naming inconsistency.)
> 
> 2. The Stop properties can be derived from either MapHex (defined in
> Map.xml) or Tile (defined in TileSet.xml).
> If both exist, MapHex takes precedence, as it is more specific.
> If neither is defined, it will use defaults on the Manager level in the
> same XML files, with the same precedence.
> See item 5 for details and a precedence list.
> 
> 3. The stop properties are:
> 
> - type = {major|minor|offmap|medium|halt|pass|port|mine|null} .
> I have added offmap because usually different defaults apply.
> However, for train running purposes offmap stops should normally be treated
> as major cities.
> Cities on "red" tiles are marked offmap by default.
> "null" applies to plain track tiles, and allows to define a non-default
> "loop" value (1860), see below.
> 
> - runTo = {no|yes|tokenOnly}. Default "yes".
> "no" would apply to e.g. Birmingham in 1851 before phase 4 and
> Elsaß-Lothringen in 1835 except in phase 2.
> "tokenOnly" would apply to e.g. the 18Scan red cities.
> This usage implies that the runTo value may be phase-dependent.
> 
> - runThrough = {no|yes|tokenOnly}.  Default "yes" (exception: offmap "no").
> "yes" obviously only applies if the city is not closed (i.e. filled by
> tokens of other companies).
> "no" would apply to standard off-map areas,
> "tokenOnly" would apply to 1830 Altoona (PRR base, implying a PRR token).
> Is there a need for "always" (i.e., even if closed)? Any other special
> cases?
> 
> 4. A somewhat related new MapHex property (also accessible to Stop, but not
> restricted to tiles with stops) is:
> - loop = {yes|no}, which defines if one train may touch a hex more than
> once.  Default "yes" (exception: offmap "no").
> "no" applies to *all* tiles in 1860.
> 
> 5. Defaults per stop type, and generic defaults (type=null) can be defined
> in Map.xml and TileSet.xml.
> These defaults are parsed and stored in MapManager and TileManager,
> respectively.
> The precedence is (the first non-null value found is used):
> - specific Hex
> - specific Tile
> - Hex default per stop type
> - Tile default per stop type
> - generic Hex default (type=null)
> - generic Tile default (type = null)
> - final default as defined above.
> 
> I believe this all gives enough flexibility, but of course the proof of the
> pudding is in the eating.
> I have tested settings at various but not all levels.  Current usage
> effectively only needs the first and last level.
> 
> 6. Stop, Tile and MapHex all have these four methods:
> Stop.Type getType() or getStopType(),
> Stop.RunTo isRunToAllowed(),
> Stop.RunThrough isRunThroughAllowed(),
> Stop.Loop isLoopAllowed().
> However, only the return values from Stop follow the precedence rules. The
> values from HexMap and Tile only return any *explicit* settings in <Hex>
> and <Tile>, so these are of little use.
> Please let me know if that could be a problem for some use cases.  I would
> have to add separate methods if these return values should also honour the
> defaults.
> 
> (Note: the existing HexMap.Run type has been split and moved to Stop.  The
> algorithms classes still pick up values from HexMap, which is no longer
> correct, see above.)
> 
> I have so far added explicit settings to the following cases (all in
> Map.xml):
> 1830: The PRR [and Reading] bases: runThrough="tokenOnly".
> 1856 Goderich/18EU Hamburg: runThrough="yes" (not really needed, because
> tile -939 already behaves correctly, but that one has been manually
> tweaked).
> 18EU Paris/Berlin/Vienna: loop="no".
> 
> The Stop code temporarily logs all Stop settings in a way that stands out,
> so that checking the effects of any trial settings should be easy.
> 
> Erik.
> 
> 
> 
> ---------------------------------------------------------------------------
> --- Get a FREE DOWNLOAD! and learn more about uberSVN rich system,
> user administration capabilities and model configuration. Take
> the hassle out of deploying and managing Subversion and the
> tools developers use with it.
> http://p.sf.net/sfu/wandisco-dev2dev
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel