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

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

At 08:36 AM 03/01/21 +0100, Raimar Falke wrote:
>On Mon, Jan 20, 2003 at 10:45:36PM -0500, Jason Dorje Short wrote:
>> Mike Kaufman wrote:
>> >On Sat, Jan 18, 2003 at 12:05:39PM -0500, Ross W. Wetmore wrote:
>> >
>> >>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. 
>> >
>> >
>> >yes, either of these is far better than (-1,-1) which I've never ever
had a
>> >good feeling about. Take your pick really, but I prefer the the 
>> >map_position
>> >pointer because it's cleaner...
>> 
>> I agree.  I remember Raimar vetoing this idea a *long* time ago, but 
>> that was for (IIRC) using pointers to positions for events (e.g., the 
>> vnotify_conn_ex argument), as opposed to goto destinations.  I believe 
>> his reasons were (1) (-1,-1) should be used over the network anyway to 
>> preserve backward-compatability, and (2) using a pointer is slower.
>
>(3) memory leaks you may get with this. So I'm for exactly two places
>where this struct of the position is allocated and freed.
>
>       Raimar
>-- 

The map_position pointer (to independently allocated storage) is a bit
of a maintenance headache as Raimar suggests. Go the step further and
just don't do it that way.

Instead, add the pointer to the punit struct, and point it back at the
goto_dest_x, goto_dest_y elements which you can convert to an unnamed
map_position, or a union of the existing elements and a map_position
if you need to preserve backwards compatibility. There is no need to
worry about managing memory - ever as that is done when the punit struct
is created and destroyed, and it is the only one to use this memory.

If the pointer is null the goto_dest locations are not valid, if it is
non-null it is pointed correctly and can be used to access the values,
i.e. to set a goto_dest, fill in the values (just like now) and point
to the &punit->goto_dest_x or however you reference it. To mark it as
invalid, just null out the pointer.

Cheers,
RossW
=====