[Freeciv-Dev] Re: patch: split up F_SUBMARINE (PR#347)

[David Pfitzner <dwp@xxxxxxxxxxxxxx>]

On Wed, 26 Apr 2000 Jeff Mallatt wrote:

> At 2000/04/24 08:06 , David Pfitzner wrote:
> >The unit flag F_SUBMARINE currently incorporates several effects.
> >For alternate rulesets it may be useful to have these effects
> >separately, so this patch splits F_SUBMARINE into:
> >
> >  F_SUBMARINE_INVIS - invisibility effect (usefulness reduced by fog, 
> >             but still some effect vs cities and visibility-2 units);
> >  F_MISSILE_CARRIER - transport_capacity is for F_MISSILE units only;
> >  F_NO_LAND_ATTACK  - cannot attack vs land (no shore bombardment).

> I did some quick tests, and this all seems to work.
> 
> One minor nit-pick: in the flag_names[] array you use underscores, whereas
> all previously existing names use intercaps.

Yeah, but it is consistent with how things have been done more 
recently with government flags and tech flags (and maybe others),
and I do prefer the underscores :-)

Certainly I prefer the underscores in the all-uppercase defines
rather than running them all together, and having done that it
seemed better to have underscores in the ruleset identifiers 
(though mixed-case), in the same ways as gov/tech flags.

> At 2000/04/24 08:18 , David Pfitzner wrote:
> >David Pfitzner wrote:
> >
> >> this patch splits F_SUBMARINE into:
> >> 
> >>   F_SUBMARINE_INVIS - invisibility effect (usefulness reduced by fog, 
> >>            but still some effect vs cities and visibility-2 units);
> >
> >On second thoughts (sorry), should probably call this, eg, 
> >F_PARTIAL_INVIS, to be more general.  (I didn't want to call it 
> >F_INVISIBILITY since one could consider the possibility of units 
> >which were "fully" invisible (Civ CFP?))
> 
> Hum...  I might be tempted to leave it F_SUBMARINE_INVIS, because the
> invisibility of this kind of unit results from a specific cause: it's under
> water (i.e. sub-marine!).  I could envision separate F_STEALTH_INVIS or
> F_CAMOUFLAGE_INVIS.  A new unit that could go on land or under water and
> that would have F_SUBMARINE_INVIS, would derive the benefit only when under
> water; it would be normally visible when on land.
>
> I realize that to implement this concept correctly, one would have to check
> the terrain type, which means a few mods to your patch.

Yeah, my thinking was along similar lines when calling it 
F_SUBMARINE_INVIS in the patch.  The main reason against this is 
that with this patch, you could successuflly use this flag with eg, 
land or air units, to achieve a "partial invisibility" effect for
those units, and it would be a bit strange to call it a "submarine 
invisibility" effect in such cases :-)

One other note on the patch is that the function is_hiding_unit()
in unit.c is not fully generalized to handle invisibility of cargo 
units on aircraft carriers and transports with F_SUBMARINE_INVIS.
(I may just add a comment on this in the patch, and apply the patch,
and come back to this later.)

-- David