Complete.Org: Mailing Lists: Archives: freeciv-dev: February 2003:
[Freeciv-Dev] Re: (PR#2370) Path finding
Home

[Freeciv-Dev] Re: (PR#2370) Path finding

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: undisclosed-recipients:;
Subject: [Freeciv-Dev] Re: (PR#2370) Path finding
From: "Raimar Falke via RT" <rt@xxxxxxxxxxxxxx>
Date: Tue, 11 Feb 2003 12:22:15 -0800
Reply-to: rt.freeciv.org@xxxxxxxxxxxxxx

On Mon, Feb 10, 2003 at 04:13:02PM -0800, Per I. Mathisen via RT wrote:
> 
> On Mon, 10 Feb 2003, Raimar Falke via RT wrote:
> > Per wants to commit PF. Per: you have to puzzle the PF together with
> > Gregorys code from the last mail above and the attached diff which
> > contains the common part and the test case. You want to merge the
> > common part with Gregorys code.
> 
> I've merged the code you sent and moved it into aicore. Stripped the

> _identical_ pqueue implementation from airgoto.c and path_finding_gb.c,

Greg just copied it.

> and moved it into a separate file. Removed path_finding_pf, and made the
> gb code default.  Stripped away dead code. Fixed bad style.

> Now I'm wondering, didn't you ever write any code which actually made path
> finding work inside the game?

No. AFAIK Greg and I only used plain timing measures or used the
warmap call to also trigger a pf call.

> There's a lot of duplicate functions, those got to go. Both old client and
> server goto should be replaced with the new path finding as it is
> committed; I don't want to keep around duplicate goto data structures and
> functions.

Ack.

> The client code barfs with this error: civclient: attribute.c:64:
> attribute_init: Assertion `attribute_hash == ((void *)0)' failed.

This 
> +  attribute_init();
> +  agents_init();
> +  pf_init();

is wrong. Looks like a merge error. only pf_init is required. The
place for this call is pretty much a non-issue.

> I removed the test code, and didn't test result, other than that the
> server still works. I did not attempt to fix anything, nor ensure that
> this result is working. I am also unsure if the path_tools.c (was
> path_finding_tools.c) part is any longer necessary.

path_finding_tools.c is necessary and may expand. Please leave the
name. Or rename it pf_tools if you want a short name.

> That is because it was at this point I pretty much gave up. Sorry, the two
> of you got to sort out this mess. It will take me at least 10x the time to
> do so, since I don't know the code, and I don't know _whose_ code this or
> that function belongs to.

Problems:
 - quality of path_finding.c (later)
 - name of path_tools
 - pqueue:
   * no comment at all what this is (also bad filename IMHO)
   * PQDATUM* suggests a flexibility which isn't there
   * INIT_ITEMS, *tiny* shouldn't be public
 - get_move_costs2 and update_move_costs2 were used by my
 implmentation. remove it.
 - data/nation/Makefile.am is in the diff
 - noise in server/stdinhand.c

Besides this you have done a good job. So please let use fix the
public interface of the queue and than you can commit this.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 checking for the vaidity of the Maxwell laws on this machine... ok
 checking if e=mc^2... ok
 checking if we can safely swap on /dev/fd0... yes
    -- kvirc 2.0.0's configure 




[Prev in Thread] Current Thread [Next in Thread]