[Freeciv-Dev] Re: (PR#4196) Enemies in allied transport assert.

["Per I. Mathisen" <per@xxxxxxxxxxx>]

On Thu, 8 May 2003, Gregory Berkolaiko wrote:
> If there are enemies in a transport of your ally and you attempt to move
> onto the tile, you get an assert from get_defender.

A bug.

> Even if we decide that ally of an ally cannot be enemy, this is still a
> bug

No. It is an impossibility.

>, as the lacks robustness and the error message comes from a wrong
> place.

There would be no error message. The error message is there to catch silly
bugs like the one above. It succeeded...

> I therefore set off to remove the assert.

... hence removing it is bad unless we have good reason. (Which we may
have, see below.)

> Instead, I went through get_defender hitting C-k in a seemingly random
> fashion.

Yay!

> The function will now return the best defender available on the tile
> against given unit. It might be an enemy, might be an ally, might even
> be one of the attacker's relatives. I decided that get_defender is no
> place to make such checks.

Err... but...

You're changing the rules. Maybe this would be a good thing, but at least
then we have to aware of what is changing.

Previously, if we attack a tile containing units with multiple owners, you
were guaranteed to be picking one of those units that were at war with you
as defender. Units not at war with you would survive the attack.

Now you get the situation where you may attack a tile containing such
units, and get a friendly unit as defender, which will be protecting (as
human shield) the enemy units on the tile. Whether or not this human
shield thing works, will be rather arbitrary, which is itself bad.

Of course, we may rule that you can't attack such stacks at all.

Anyway, there you have a fine way to confuse the AI - cooperate with
someone not at war with it to render your units totally immune to attack.

The correct solution is to go the other way, the way that the layers patch
went: Returning NULL is not an indication that the tile is empty. It is
just an indication that you cannot attack anything there. You got to lose
the assumption that just because you can't attack a tile, you can move
there.

> The step after that is a thorough cleaning of handle_unit_move_request, to
> ensure that it issues right warnings.

Well, that is necessary in any case. The function is a mess. I started
cleaning it up a while ago, moving more stuff to the client, but left it
unfinished, the patch is in RT.

> Also it would be helpful if you can tell me what happens when something
> (not Fighter) attacks a tile with a Bomber and fortified tank.

It is not allowed. The check to can_unit_attack_unit_on_tile() fails.

  - Per