[Freeciv-Dev] Re: [Patch] Make do_unit_goto return a meaningful value (PR#985)

[Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx>]

 --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote: 
> On Sat, Sep 29, 2001 at 02:16:32PM -0700, Gregory Berkolaiko wrote:
> > 
> > The testgames are identical with/without the patch.
> 
> Looks ok.

good

> > @@ -1099,7 +1104,7 @@
> >        /* this is a special case for air units who do not always want
> to move. */
> >        if (same_pos(waypoint_x, waypoint_y, punit->x, punit->y)) {
> >     advance_unit_focus(punit);
> > -   return;
> > +   return GR_ARRIVED; /* well, somewhere */
> 
> Than split GR_ARRIVED into GR_ARRIVED_AT_DEST and GR_ARRIVED_SOMEWHERE

I have a problem with this: nowhere in the code such differentiation is
needed yet, but I do not know what will come in the future.  I think the
best solution is to go for something now, comment it well, explaining
what case it is really (it's already explained in this particular
instance) so that it will be easy to change later when needed.  

Actually, thinking about it, this place should return
GR_OUT_OF_MOVEPOINTS, which is a lie, but the unit does not want to be
disturbed so it serves the purpose.

So here I suggest
+  return GR_OUT_OF_MOVEPOINTS;  /* it's a lie, but the unit wants to */
+                                /* rest for a while (refuel) */

> > -   return;
> > +   return GR_FAILED;  /* Isn't this line redundant? */
> 
> Either it is or it isn't.

Looking at the code: yes it is.
Put an abort() here: runs as normal.
Decision: remove it.

> >        /* Don't attack more than once per goto */
> >        if (penemy && !pplayer->ai.control) { /* Should I cancel for
> ai's too? */
> >     punit->activity = ACTIVITY_IDLE;
> >     send_unit_info(0, punit);
> > -   return;
> > +   return GR_FAILED; /* well, nothing better comes to mind... */
> 
> ????

Actually I would be surprised if this case is ever encountered but it's
interpretation is as follows: human-owned unit was on eXplore,
encountered something and attacked it.  Wants to rest now.  How about
  return GR_OUT_OF_MOVEPOINTS
with a comment?  Alternatively, can return something like
GR_GOT_IN_TROUBLE and let the superior code decide whether to cancel
eXplore.

Best,
G.

____________________________________________________________
Do You Yahoo!?
Get your free @yahoo.co.uk address at http://mail.yahoo.co.uk
or your free @yahoo.ie address at http://mail.yahoo.ie