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

["Gregory Berkolaiko" <Gregory.Berkolaiko@xxxxxxxxxxxx>]

On Thu, 15 May 2003, Raimar Falke wrote:

> On Thu, May 15, 2003 at 01:20:53PM -0700, Per I. Mathisen wrote:
> > >  - 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).
> > 
> > This is far, probably too much. But nevertheless, don't assert this
> > without capping it somewhere.
> > 
> > +  assert(src->length < MAX_LEN_ROUTE);
> 
> Silent capping is not what we want. We want something like:
> 
>   freelog(LOG_ERROR, "freeciv only support paths of %d length. The
>   path you tried to use is %d. Please contact freeciv-dev@... to let
>   this limit be increased.");

Yes please.

> > +  pf_print_path(LOG_DEBUG, src);
> > 
> > I hope you remove this for the final version :-)
> 
> Is it really this much runtime overhead?

It's not executed and it doesn't get optimized away.  So it should be 
removed.

> > +    for (i = 0; i < packet->length; i++) {
> > +      dio_put_uint8(&dout, packet->path[i].x);
> > +      dio_put_uint8(&dout, packet->path[i].y);
> > +      dio_put_bool8(&dout, packet->path[i].wait);
> >      }
> > 
> > Hmm. Instead of sending a hard 'wait', why not mark dangers and let the
> > server sort out the waiting? This way we don't screw up if the server sits
> > on some information we don't have. Probably harder to code, though.
> 
> Uhhh. You want that the client sends the server a goal and a set of
> dangerous tiles and the server finds a path? Or do you want that the
> client sends the server a path and a set of dangerous tiles and the
> server figures out the waiting?

I think Per is asking too much.  Keep it simple.  If the goto screws up, 
oh well, such things happen.

I have a complaint on the "wait" bit too, though.  Why are you reinventing
the wheel?  Path Finding implements the waiting by reinserting the
position into the path.  We should keep it throughout the code, instead of
converting from one format to another.  Just make goto_route_execute
recognize this convention, instead of doing it in compact_path.

Also, please re-read the CodingStyle where it talks about function 
header comments.  

> > As for savegames, I say just nuke the punit->pgr info from old savegames.
> > It is not important information unless we want the savegame to debug. And
> > then we want new savegames anyway.
> 
> Possible.

I think it should be done.

G.