[freeciv-ai] Re: Goto coordinates and ai roles

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

If you cannot rely on an arbitrary map_position to represent an invalid
position (i.e. all map positions are potentially valid), then you need an
extra bit of information somewhere. There are two common ways to do this.

Set a goto_active guard bit and do things like 
  if (punit->goto_active) {
    /* punit->goto_dest_x, punit->goto_dest_y accesses */
  }

Make the goto_dest a map_position pointer and use null as the guard test.
  if (punit->goto_dest) {
    /* punit->goto_dest->x, punit->goto_dest->y accesses */
  }

Note, the pointer can point back into the punit structure, so you don't
need to track extra memory allocations, and can in fact directly access 
the punit goto(x,y) values if necessary, bypassing the guard check. The
pointer acts very much as a guard test that allows you to turn the goto
map_position on and off.

Both of these mean breaking backwards compatibility for the (-1,-1)
error assumptions including over network communications. But the tests
are much simpler than a same_pos() or some unnormalized version to
detect the special case. 

As Jason pointed out in a subsequent post, is_normalized_map_pos() is 
just plain wrong as it does not test for the special case, but rather 
an arbitrarily large number of cases some of which might be real and 
some unreal.

Cheers,
RossW
=====

At 06:42 PM 03/01/16 +0000, Per I. Mathisen wrote:
>In the massiveai patch (most of which is now in cvs, but not this), I had
>unset goto coodinates set to (-1, -1) and a macro valid_goto() which
>checked for this to avoid segfaults.
>
>The good part of it is that it stopped use of unset goto coordinates,
>which could lead to unpredictable results. (Yes, this is actually a
>problem in the current code.)
>
>The bad part is that it led to a bunch of new, stupid tests to avoid
>segfaults, and wasn't very clean API-wise.
>
>Now the need for fixing this problem arises with greater urgency, since
>generalised topology will no longer allow us to use (0, 0) with impunity.
>
>So what I've been thinking of doing, is as follows:
>
>Whether a unit has a valid goto destination or not depends on its ai_role.
>Certain roles require a null destination, others require a valid
>destination. A null goto destination is always (-1, -1).
>
>Goto destinations in the higher level code are _only_ used for gotos that
>have more than one turn horizon. This corresponds to the use of the
>ai_gothere() function. One turn horizon goto should use ai_unit_goto()
>which preserves any existing goto destination or a null coord set.
>
>Furthermore, any use of goto should be gated through goto_is_sane(), which
>should be expanded to check for the normality of the goto coordinates.
>
>This schema requires some moderately big changes to the way
>ai_manage_units() and below are organized, since it first sets roles, and
>then sets targets, which can't be allowed - in the above schema roles and
>targets must be set simultaneously.
>
>Comments?
>
>  - Per