<URL: http://bugs.freeciv.org/Ticket/Display.html?id=40619 >

> [guest - Tue Feb 10 21:49:59 2009]:
> > [book - Di 10. Feb 2009, 02:58:11]:
> > 
> > > [matthias.pfaffer...@mapfa.de - Sun Jan 04 22:03:07 2009]:
> > > 
> > > The gold upkeep patch (version 2)
> > 
> > I have made some adjustments to your patch and attached
> > it as version 3. The most pertinent changes are:
> > 
> > - The option is called 'gold_upkeep_style' instead of
> >   'economic_system'.
> > - I used vectors to randomly select improvements and units
> >   to sell.
> > - I moved the unit gold upkeep stuff from unittools.[ch] to
> >   cityturn.c.
> > - Cities are passed to update_city_activity() in a random
> >   order.
> > 
> > The rest is pretty minor style and design stuff which can
> > be seen in the patch code if you are interested.
> First, thank you for looking at the patch! Some comments:
> In my version of the patch the improvements to sell are also selected 
> randomly using 'city_improve_list_shuffle'. You did not used 
> *_list_shuffle at all, so perhaps this isn't needed and could be 
> deleted?

It could still be useful in the future, if ever a genlist or
speclist needs to be completely shuffled.

The reason I did not use it here is because I think it is
inefficient to shuffle the entire list when only a few
elements need to be "randomly picked". That is, in most
cases only very few buildings or units will have to be
sold for the player to have non-negative gold again.

So the random iteration algorithm using vectors handles
this case as efficiently as possible, while still ensuring
that the runtime costs are no worse than the entire-list-
shuffle version in the worst case when every element of
the list needs to be sold.

> I learned some time ago, that I should _never_ use GOTO in my 
> programs. For something like you did - skipping some parts within a 
> function without using a lot of if's - it is OK / accepted to use? 

I used 'goto' in this case to avoid code duplication
(repeating the memory freeing code) and another level
of indentation (i.e. the same thing could be done by
wrapping the whole thing in 'do { ... } while (0)' and
using 'break' in place of 'goto').

My policy towards 'goto' is to use it only if no better
control flow construct exists in a particular situation
(which in practice reduces to almost never).

This requires some experience to judge correctly (e.g.
what is meant by "better"?), which is probably why it
is easier to just tell someone to never use goto in the
first place (which would be true 90% of the time),
instead of having to explain the full principles and
circumstances under which it might be alright to use.

> > > The patch was tested with an adapted ruleset (gold upkeep).
> > 
> > Can you post this ruleset, so I can test it too? Or if you
> > find anything wrong with this version of the patch you can
> > leave a reply and I will try to fix it.
> A patch for the default ruleset is attached. It changes all shield 
> upkeep to gold upkeep ...

Ok, I guess that will be enough just for testing.

> Would it be possible to let a unit use shield upkeep for one 
> government and gold upkeep for another? Perhaps add a third option to 
> the 'gold_upkeep_style' which just uses 'uk_shield' for units as gold 
> upkeep or which just adds the shield upkeep to the gold upkeep and 
> does not needs the shilds. So there would be no need to change the 
> rulesets.
> This would also open the possibility to have this option as a setting 
> for each government ...

It might be better to do this with effects, but it would
probably further complicate the functions that calculate
unit upkeep (maybe the upkeep values should only be calculated
once at the start of each turn and stored with the unit?).

I would also note that gold and shield upkeep behaviour is
not as symmetric as it might seem. For example, it doesn't
make sense for shield upkeep to be paid by the nation as
a whole (i.e. there is no "shield treasury"), and the
current way gold_upkeep_style=0 is handled doesn't really
make sense either (i.e. why do individual cities get
penalized for the global treasury being negative? They
should have their own "city treasuries" I suppose...). I
wonder if this last behaviour was present in civ2 as well,
or if it is just due to the implementation in freeciv.


Freeciv-dev mailing list

Reply via email to