[Freeciv-Dev] Re: gen-impr stuff

["andi payn" <paynfc@xxxxxxxxxxx>]

Ben Webb:

> Cool! It would have been handy if you'd let me... know...
My ISP screwed up my account in December; I've been able to get back
online temporarily (with a dialup account that often hangs up for no
reason, which is a poor substitute for the DSL connection I had
before), and posted to this list as soon as I was able. I should have
also posted to the freecivac-devel list, but I didn't think about it.

> ... I wrote some code this afternoon which looks like it's
> an almost exact duplicate of your own. ;(
Well, that means I'm on the right track, which is comforting after
spending a couple of weeks sure I was probably lost in the
wilderness....

> ... It may be that I'm mistaken about a few things, because
> it _is_ a rather big patch...
Plus it's a patch against old versions of both the CVS tree and your
patch, plus it includes all kinds of things not related to impr-gen
(some of which aren't completely finished), plus it's a hack written
to make the game playable for me against the AI and a few
(non-computer-savvy) friends rather than a well-written patch, plus
it's only my second serious effort to dig into the FreeCiv code (the
first being the vet-levels stuff).

With all those strikes against it, I'm thankful that anyone bothered
to look at it. (I wasn't even going to upload it originally, until a
couple of people asked me to.)

> 1. You don't actually seem to handle _all_ the effects...
Some effects have no effect. As your surmised, it's because they were
all effects that weren't handled by FreeCiv before impr-gen, and
either they didn't affect my games or I didn't know exactly how they
should be handled.

> 2. You've either got an old version of my patch, or you've
> intentionally recoded some things for reasons unclear to me.
A little of both. In a few cases, I had to recode to make it work
with other changes I'd made. Also, I replaced all of the iterations
(both iter and pointer) to use the macros I wrote (to be more in
keeping with the rest of FreeCiv, and so I could change the way
those iterations worked without having to make changes all over the
place).

> 3. ... your handling of "happy Wonders" (e.g. Shakespeare's
> Theatre) is wrong.
I just took the code straight out of your old patch. If you've fixed
it since then, that's the reason.

The bugs with the AI's handling of happy and content improvements,
however, are entirely my own (and I know at least one big one, which
I've already posted about).

> 4. You overuse is_effect_activated() horrifically.
I wasn't sure about this. I first wrote it to use is_under_effect all
over the place, and found some serious AI bugs. So I replaced most of
those uses with the new function is_under_active_effect, and the bugs
all went away. This is probably overkill--from what you say, it looks
like only a few places, all in the AI code, will need to use this new
function.

However, I didn't see the exploding CPU usage. I've been running a
server with multiple AIs with one local Gtk client and two remote
clients, and played some pretty big games, and my P200 kept up no
problem. And a quick profiling shows not much time at all spent in
that function. That doesn't mean the extra calls shouldn't be
eliminated, but I think the concerns are overstated.

> 5. As far as I can tell, you do not honour the "survives" field.
I didn't look at this at all. If your older patch didn't handle it,
then my code doesn't either. It didn't seem to affect any of my games,
but then I don't think Apollo has been destroyed in any of my games,
so that doesn't mean anything.

> 6. I don't like your Enable_Space and Enable_Nuke handling anyway.
Neither do I. First of all, Enable_Nuke has a typo, so it's
world-range only. Enable_Space works either world-range or
player-range. But they should both be available at the city
level. It's because I used existing infrastructure rather than putting
the code where it really belongs. It allows me to play with the
standard rulesets or with an additional player-range "small wonder"
needed to enable space (someone has to build Sputnik, after which,
every player can build their own Apollo, after which they can build a
spaceship), but that's as far as it goes; it should be fully general.

> 7. Looks like your handling of the various Upgrade effects (e.g.
> Leonardo) only works for Player-range. I think mine is better. ;)
Well, it also works for world-range, except that the _One effects will
upgrade one unit for each player, rather than one overall. But yes, it
should be done better; it wasn't handled at all in the old patch I
downloaded, so I just reused existing infrastructure for my new code.

> 8. Easy on the effect iterations!
Not everything that looks like an iteration really is; some of the
macros just iterate through an EFT_LAST-terminated array.

However, there are some places, especially in the AI, where I couldn't
see any way around iteration.

I tried to merge as many iterations as possible. For example, I added
even more stuff to do_bonuses_pollution (and made the name horribly
inaccurate--is Meltdown_Disorder a bonus or a pollution modifier? no),
merged Apollo (the Reveal_Cities and Reveal_Map) and Marco together,
etc. However, there are probably many more places where this can be
done.

Also, in many places (especially the military AI), the code should
precalculate some things.

For example, get the world-range + player-range totals and
island-range total arrays for all the military effects the AI cares
about just once, then for each city you can just total up the
city-range effects. Even better, make a list of all buildings with
those city-range effects at startup, and just iterate through those
instead of iterating through everything.

Similarly, when considering building content/happy improvements, I
can see a way to avoid most of the repetition by precalculating a lot
of stuff. A few hundred bytes of storage could save a lot of loops.

However, while the problem should definitely be fixed, the CPU usage
can't be too bad, as again, my P200 had no problem keeping up while
handling lots of other things at the same time.

> 9. You're absolutely right about the pollution modifiers.
Phew! I thought I was just missing something obvious!

> ... I'm more inclined to add them all and subtract from 100...
That doesn't work. First of all, if you have a Recycling Center, you
get 34, subtract from 100, and it's only reducing production pollution
by 34%, rather than the 66% it's supposed to.

You could fix this by starting from 100, and adding (100 - amount) for
each effect. You also have to change each -50 to 150 in the
rulesets. This does work, but it's a little confusing to ruleset
maintainers....

If we want these to work like other bonuses, there is a right way to
do it: They're negative bonuses. Recycling Center has a -66 amount,
Hydro Plant has a -50 amount, the compensating factor for Hydro Plant
with Recycling Center has a 50 amount. Then we can start at 100 and
add each amount, just as we do for other bonuses. With just a Hydro
Plant, we end up with 50. With just a Recycling Center, we end up with
34. With both, we end up with 34. Exactly what we want. And multiple
effects that combine will do so the same way they do with other
bonuses.

However, I don't think this really is what any modpack designer will
want. If three 50% bonuses together make a 150% bonus (meaning 250%
adjustment, so 50 science points becomes 125 science points), that's
fine. If three -50% bonuses together make a -150% bonus (meaning -50%
adjustment, so 50 pollution points becomes -25 pollution), that's more
than a little weird.

Think about it as a modpack designer: You want to make two pollution
adjustment buildings that can be used together. You want the first one
to reduce pollution for a city by two thirds, just like a Recycling
Center, and you want the other to reduce pollution for an island by
half. You probably don't expect the two together to reduce pollution
to nothing (or negative!); you want it to reduce it to one sixth. But
there's no way to do that with additive adjustments.

Also, the multiplicative solution means that the amount for Recycling
Center is 34, which seems more intuitive than -66 (34 is, after all,
what the original ruleset writers put there).

But I may be wrong about this.

> This is very good news. I was thinking of looking at this myself,
> but the AI code just looked too scary.
Yes, it was scary, but once I got started, it was more tedious than
difficult.

> If you ever succeed in untangling the code changes, I'd love to get
> this into FreecivAC (and thus, hopefully in the not-too-distant
> future, Freeciv proper).
Here's my current plan: Rather than trying to untangle the massive
patch I created, I'm going to start over.

1. Pull today's CVS
2. Rebuild each of the easy patches (vet-levels, diplomacy battles,
  etc.) from scratch and upload
3. Rebuild the calendar patches as I explained in my other messages
  and upload them
4. Apply all of the other patches I use on a regular basis
5. Rebuild all of my simple patches that depend on other patches and
  upload them
6. Pull your latest impr-gen-combined patch
7. Rebuild all of the foundation-level changes (like the new
  iteration macros) from scratch and upload that as a patch
  dependent on yours
8. Rebuild all of the other non-AI changes as a patch dependent on
  yours and the patch I just mentioned and upload that
9. Rebuild the AI changes as a patch dependent on the last two and
  upload that

For each patch, I'll upload it to the FTP site and the bugtracking
system and post about it here. For the impr-gen-related patches, I'll
also post about it on freecivac-devel (do I have to be subscribed to
the lists to post? Hotmail can't handle the traffic...).

Does this sound like a good plan?



_________________________________________________________________
MSN Photos is the easiest way to share and print your photos: 
http://photos.msn.com/support/worldwide.aspx