[Freeciv-Dev] Re: (PR#5568) add I_DEMOLISHED Impr_Status

["Mike Kaufman" <kaufman@xxxxxxxxxxxxxxxxxxxxxx>]

WARNING: proposal for reducing modpack flexibility discussed below. Read if
you care about such.

On Fri, Sep 12, 2003 at 12:02:40AM -0700, Jason Short wrote:
> 
> I don't buy the logic here.  What if an improvement is both demolished
> and redundant?  What if it's demolished, redundant, and obsolete?

an obsolete building can't be a redundant building since the former's 
effects are suppressed forever via tech advance while the latter's are
suppressed temporarily by another buliding.

A demolished building can't be either obsolete or redundant since 
obsolete and redundant buildings exist while demolished buildings don't.
 
> This patch doesn't introduce these problems, but it extends them.  The
> handling just isn't very clean, and with a new status added the logic
> will become even less clear.  How do we tell if an improvement's effect
> should apply?  Is this explained anywhere?

it seems the only place this is explained is right here. An improvement's
effects are only active when the improvement is I_ACTIVE or I_DEMOLISHED
and for the latter only the surviveable effects are active.
 
> I don't like the idea of conscripting an enumeration into a series of
> character values just to make network and savegame handling easier. 
> This confuses three parts of the code that should be completely separate.

sure. ok.

> It just doesn't seem like this is going in the right direction.  Perhaps
> later gen-effects patches will clean this all up, but why can't we do
> the cleaning first and then add the new functionality rather than add
> the new features as a hack?

Sure, this can be done all at once, but this is not a "new feature" in that
it isn't necessary to gen-effects. It is critical for gen-effects.
Otherwise the client won't know that survivable effects are still active.

> As a side note, I think I_DEMOLISHED should be used for any building
> that used to exist and no longer does.  This logic seems cleaner than
> only marking "special" buildings as demolished.  Then the appropriate
> building flag can be checked when we look to see if the effect is active.

that is an awful idea. mainly because that would increase the computational
load simply by forcing checks of destroyed buildings for no reason.

frankly I hate the idea of survivable effects. In fact, how's about this:
Right now there are two (2) surviving effects in all the rulesets. They
would be Enable_Space and Enable_Nuke. These are both World-ranged effects
and are the whole reason there is this mess. Would it be simpler to just
special case these two properties? I foresee:

game.can_space = bool
game.can_nuke = bool

and/or:

pplayer->can_space
pplayer->can_nuke

and/or:

pcity->can_build_space
pcity->can_build_nuke

In the absolutely simplest case, we have the (immediate) effects
Enable_Space and Enable_Nuke, which are active as soon as the improvement
(or tech etc) is built and then are deleted soon afterwards. The only work
they do is to flip game.can_space and game.can_nuke to TRUE; These
variables would of course have to be saved in savegames and transmitted to
the client in the game_info struct. Presto, surviving effects. Downsides:
these effects are Global-ranged only and always survive improvement
destruction.

There are alternatives. We could have four effects: Enable_Space,
Enable_Nuke, Enable_Space_Survives, Enable_Nuke_Survives. The first two
effects would be normal effects. The latter two would act like those in the
previous paragraph.

Alternately, we could allow Player ranged effects as well but that would
require the pplayer->can_space and pplayer->can_nuke variables. I don't
really want to go for smaller range than that because it gets messy and
demands island-range and the such.

This is such a great idea, in fact, that unless I hear great bitching with
good reason behind it, this is what I'm going to do. There shall be only
two surviving effects and they shall be special cases. (This means that the
I_DEMOLISHED flag will not be necessary.)

-mike