[Freeciv-Dev] Re: Wonder Variant

[Jeff Mallatt <jjm@xxxxxxxxxxxx>]

This is looking better.  My only major complaint is about the
interpretation of the .amount field.  I really don't like a magic
threshold, above which the "units" of the value changes.  The units are
implicit in the named effect (perhaps, we should make them explicit).  In
the one place where it makes a difference, you have:

  effect0.effect   = "MakeContent"
  effect0.amount   = 9999              ;percent
  effect1.effect   = "MakeContent"
  effect1.amount   = 1                 ;in case no temple

I don't even understand how this would work (I guess it depends upon your
interpretation of "percent" -- percent of total citizens or percent of
unhappy citizens).  Anyway, if "MakeContent" always took a "citizen count"
you could simply use the first effect, and get rid of the second.

Maybe every effect should be named, e.g., "MakeContent#" (for a count) or
"MakeContent%" for a percent.  Not all would need both.

The rest of these comments are more minor.  First, the "obsoletes" field
seems misnamed to me -- things aren't really obsoleted, they are just
"disabled" until the wonder is itself obsoleted or destroyed.

Naming an effect, e.g. "Darwin" or "Marco", seems to me to be not very
descriptive of the effect.  Perhaps more descriptive names (e.g.,
"GiveAdvances#" or "HaveEmbassies%") would be better.

Now for a real nit-pick, I like consistent formatting.  Switching back and
forth from:

  effect0.effect   = "MakeContent"
  effect0.amount   = 9999              ;percent
  effect1.effect   = "MakeContent"
  effect1.amount   = 1                 ;in case no temple

to:

  effect      = { "effect", "amount", "range", "cond"
                  "ReduUnrest", 1, "Player", "Republic"
                  "ReduUnrest", 2, "Player", "Democracy"
                }

just confuses me (maybe I'm dense).  I'd just format everything as in the
second example.

jjm