[Freeciv-Dev] (PR#4242) Clean up the goto route network protocol and route execution

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

[rfalke - Thu May 15 20:00:45 2003]:

> 
> The attached patch:
>  - cleans up the network protocol (struct packet_goto_route)
>  - rewrites goto_route_execute
>  - minor cleanups (destroy_unit_goto_route, move of struct goto_route
>  to unit.h, maybe_cancel_patrol_due_to_enemy)
> 
> Notes:
>  - since I'm not sure what should be returned in case of waiting I
>  just added another constant: GR_WAIT. It doesn't seems that any
>  caller checks the result of goto_route_execute strictly.

Indeed.

>  - the savegame load support of older savegames is fishy. It may
>  work. It may not. It works with a new savegame.

Old savegames have to be compatible.  I'm not sure if they are or not.

>  - a lot of the cruft of the old protocol was about handling
>  unlimited routes by splitting these into chunks. I removed this and
>  set a limit (MAX_LEN_ROUTE). MAX_LEN_ROUTE is 1200 steps. This is ~5
>  times the number to go around the largest world (255x255).

A worthy goal, although it can be separated from the "waiting"
functionality.

>  - a unit which is waiting is blinking with a "G" at the client. This
>  may be irritating.

Indeed, that's no good.  Looks like the client's focus rules need to be
changed to support this.

>  - the old code creates a patrol route by reversing the path to the
>  "reverse point". This doesn't work with waiting (trireme). If this is
>  done properly it can happen that the way back to the starting point
>  is different from the way to the "reverse point".

Hmm, that's a problem for PR#4683, and should be changed.  Although this
can also be separated from the other changes.

>  - only the human player ever uses punit->pgr. No other code sets it.
>  - in total the patch removes some lines :-)
>  11 files changed, 323 insertions(+), 358 deletions(-)
> 
> It works with my test cases. But this is rather large change so I'm
> sure that there are errors.

I don't think the extra 'wait' bit is needed for the network, and
changing the savegame also seems unnecessary.  The logic is clear enough
as it is, although it may be easier to debug with the extra information.

Aside from that, I like all the changes.  But I would rather see them be
made one at a time rather than in one big patch.

jason