[Freeciv-Dev] Re: (PR#2521) general effects framework

["Mike Kaufman via RT" <rt@xxxxxxxxxxxxxx>]

> So there are non-immediate effects (persistent) which stay and effect
> other objects. And there are immediate effects which are one-shot and
> get removed immediately.

that's correct. most effects will fall into the former category.
EFT_GIVE_IMM_ADV, EFT_IMPROVE_REP, EFT_REVEAL_MAP, and possibly
EFT_REVEAL_CITIES are the only immediate effects for now.

> > cond_bldg is special because given an effect in an effect list (Island,
> > Player, and World) you don't know right away if the effect will be active.
> > cond_bldg means: this effect is _only_ active in a city that has this
> > building. So for an effect in the Player range, it could be active or not
> > depending which city we're looking at (if everything else is sufficient for
> > it to emit, then it is "pending") (thanks for asking, it's made me realize
> > there's a bug in the current code.)
> 
> If I follow this logic wouldn't it also be nice to have a pending2
> condition which is true if the effect has Player or World range and is
> "almost finished" but only the island is the wrong one/undefined.
> 
> Basically I don't like the special case.

I'm not sure you follow the logic. If the effects has only Local or City
range, then it will always know if a the required building is present. For
example the city walls effects -50 Spy Resist depends on Palace, but since
this is a local effect it should [there's a bug right now] never be
pending. Now say I have a wonder that does the same thing but is Player
ranged. Then if a spy attacks a city, then the effect is pending. If the
city has the palace, then you get -50, if the city doesn't then the effect
isn't active. This really isn't a special case, as these are necessary all
over the default ruleset. If you can think of a better method that doesn't
required a pending status, I'm all ears. 

> > If EFR_NONE is used, then the effect is not added to any lists. This is
> > needed (I think) for equiv_range not for effects.
> 
> Can you explain it more?

Ok, based on the framework I've laid out, effects whose definition gives a
range of None are useless effects. They affect nothing. They are not added
to any effects list. Why should they?

On the other hand, for equiv range, you have all sorts of buildings that
aren't equivalent to anything else. You need to give them a range so that
fill_improv_lists and its ilk work. That's why I need it to stay around.
I've not thought a great deal about this however. There may be a way to
eliminate it.

> > that's right. That's because we can't include improvement.h, tech.h at the
> > top of effects.h because they must include effects.h at the top of them :(
> 
> I expected this answer ;) This is another possibility to point you at
> PR#2350. The same is the solution here.

this will not work (with any sort of elegance). Think about it a bit more 
and notice where in you'd have to put the #includes to do so. I personally
could care less about being completely type safe if the alternative is
butt-ugly code.

> Isn't it possible to design something like this:
>  effect A depends that no effect B is active
>  effect B depends on A

No. see Ben's answer. (Note to Ben: this one of the reasons why I had to
discard the idea of the suppress_bldg condition as an answer to equiv)

> I agree with you that it is a complex problem. However the worst case
> I can think of goes as followes: we add this patch and also all the
> others and than see that even with caching and some performance
> kludges we end up performing 5 times slower than now. The AI doesn't
> cope with it and to tech it would make it slower by factor of 10. At
> this point we can't back out. I want to prevent this by having some
> numbers now.

This is fear-mongering. Not in my worst dreams do I predict a slowdown of
more than a factor of two. Without the AI, I predict a slowdown of less
than 10%, if that. Less after some clever cacheing as Ben as shown. 
The clincher is the AI. Per might have some thoughts and predictions on this.
We can always back out. Please remember though that the end result is
configurability that exceeds the present by an enormous factor.

Ben's suggestions on safe-lists and such are excellent, and for the default
ruleset this should speed up updating by quite a bit.

-mike