On Wed, 27 May 2015 23:30:39 +0200 Fenyo <feny...@gmail.com> wrote: > Ok, let's see thoose implementation problems with the "Seasons unhardcoded!" > patch.
I have had a better look at the MoreSeasons branch now. However the
more I look at the FreeCol turn handling code that it is based on, the
more underlying problems I find, which I think are leading MoreSeasons
to be way more complex than it needs to be. If we are going to allow
the seasons to be defined in the spec we need a better foundation.
Here are the first set of problems:
- "gameOptions.years" is currently defined as "gameOptions.year" in
GameOptions.java. Doh!
- Turn.ages conflate the founding father recruitment with the
year the seasons split. They should be independently specifiable.
Indeed the FF part does not need to be in Turn at all.
- The third age (1700) is hard coded in Turn.java, and nowhere else.
- The "independence turn" (1780) is hard coded in IGC, and nowhere else.
Patches to fix these are either committed or under test.
There are also some test problems with TurnSelector. Fortunately, this
is no longer used, so it has been removed.
With that in place I can fix Turn so that it only needs to know the
number of seasons (using an initialization call from Specification,
like is used for the season-year). That will almost completely remove
any need for Turn routines to need Specification parameters. Patch
remains under test.
Then I think we can really simplify things and get rid of Season
completely. After all, it is effectively just a number equal to:
(turn - 1) % numberOfSeasons ; turn >= season-year
<irrelevant> ; otherwise
The naming could just be:
model.season.0=Spring
model.season.1=Autumn
There is code in NameCache for reading groups of indexed messages like
model.season.*.
All that would be needed then to make the number of seasons variable
is a single game option (in gameOptions.years) that controls the
initialization of Turn:
<integerOption id="model.option.numberOfSeasons"
value="2" defaultValue="2" minimumValue="2"/>
Mods would override this and provide the alternate names as above.
For the mods with many seasons it would be easier to make the display
routines check Messages.containsKey and on failure fall back to using
something like:
model.season.default=Week %number%
That seems like a better result to me. No Season.java required. No
funky double message interpretation. Turn.java is probably shorter.
One new integer option rather than a whole new specification section.
Detail is moved to the spec and messages files where it belongs.
Cheers,
Mike Pope
pgpROCjrGWmA7.pgp
Description: OpenPGP digital signature
------------------------------------------------------------------------------
_______________________________________________ Freecol-developers mailing list Freecol-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freecol-developers
