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

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

[kauf - Sat Aug 30 02:54:47 2003]:

> Here is a patch that adds I_DEMOLISHED Impr_Status. It is needed for
> general effects.

+#define I_NONE       '0'   /* Improvement not built */
+#define I_ACTIVE     '1'   /* Improvement built, and having its effect */
+#define I_OBSOLETE   '2'   /* Built, but obsoleted by a tech */
+#define I_REDUNDANT  '3'   /* Built, but replaced by wonder/other
building */
+#define I_DEMOLISHED '4'   /* Destroyed, but effects survive */

I don't buy the logic here.  What if an improvement is both demolished
and redundant?  What if it's demolished, redundant, and obsolete?

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?

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.

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?

-----

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.

jason