[Freeciv-Dev] (PR#8754) effects patch

["Vasco Alexandre da Silva Costa" <vasc@xxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8754 >

> [kauf - Thu Jul 08 20:43:08 2004]:
> 
> I must disagree here. This needs to be done before the majority of the
> code is committed mainly because I foresee that a bunch of committed
> code will then have to be changed all over again. Then again I could
> be convinced otherwise, perhaps Vasco has a detailed plan on how to
> do this...

With the latest version, the only code in effects.c that would
require changes to support a generalized sources cache is
target_in_range(). Even the buildings.ruleset format will
remain the same.

The sources cache is pretty much separate from the effects code. You
could implement it any way you desire as long as it provides the
required API functionality.

Of course, new code would be necessary elsewhere to create and update
it, but this is a separate facility.

> > client/packhand.c: uses B_PALACE and B_CITY in
> handle_city_short_info.
> 
> yeah, this is nasty. There is also a ticket somwhere that deals with
> the client's problems with not getting rid of stale palaces.

I think I will just kludge these for now like Ben did, but this code
needs to be re-thought. It is indeed very nasty. Do you know that
ticket's number? It could be interesting.

> > If there are any keywords you would like to rename, better rename
> > them sooner rather than later.
> 
> I have only skimmed the patch, so take these with a grain of salt.
> I'll look harder after the middle of next week.
> 
> leonardo and marco_polo functions should be renamed.

s/marco_polo_make_contact/do_have_embassies_effect
s/handle_leonardo/do_upgrade_effect
s/do_apollo_program/do_reveal_effects
s/great_library/do_adv_parasite_effect

?

> city_gan_grow_to() has two function prototypes.

Fixed this in my local tree, will be in the next version.

> I am so not a fan of game.palace_improvement (though I'm not sure yet
> what you're attempting to accomplish here).

B_PALACE has many strange interactions, some of which are not even
effect related. A player can only have one B_PALACE or none at all.
B_PALACE is free for the first city you build, despite it
having Masonry as a tech_req.
To build another B_PALACE you do need Masonry, but the old B_PALACE is
removed when you do this.

The city with a B_PALACE is called player capital and is displayed as
such in several dialogs, like the intel dialog.

If someone captures your capital it always destroys that B_PALACE.
Your spaceship is destroyed, and your empire may split into civil war.

You cannot give your capital city away via a treaty, you cannot lose
it via civil war (this one is weird).

These are currently handled as effects: city spy resistance (sabotage,
your own spy defense is boosted), bribe immunity, reduction of
corruption and waste in city by half.

Then handle_city_short_info passes it to the client so you can see
which is that player's capital in the intel dialog...

> The above in is_captial() worries me.
> I see there's been no movement on {Sea|Land|Etc}_Defend...

I didn't do it because I don't know which is the best way of making the
API.

In lots of places that check these effects, you don't have a unit
to compare with. So if I make an interface that requires a unit
pointer, to check the effect, that isn't very convenient.

Does anyone intend to add more movement types anyway? The whole
pathfinding and movement code is extremely hardwired, it is
not easy to add new movement types. Terrain types or specials on
the other hand are easy to modify via terrain.ruleset.

> The use of the variable name "elt" is rather vague and means nothing
> to me.

It is short for "element". It is like using "i", "it", "iterator" or
"node" as variable names.

> Why Island->Continent?

I thought about this for quite some time. I never liked giving multiple
names to the same thing. It just confuses someone reading the code.
I did a "grep -i" of the source code and the term continent beats
island nearly 2:1. So I renamed it to continent in this patch.