This is the mail archive of the xconq7@sources.redhat.com mailing list for the Xconq project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Major improvement to the Xconq kernel


On Thu, Nov 13, 2003 at 09:48:37PM -0500, Jim Kingdon wrote:
> > A new path-finding code by Peter Garrone has just been checked in. It uses
> > the Astar algorithm and lives in the new source file path.c.
> 
> Cool.
> 
> So far I found:
> 
> * One case of a suboptimal path (went around an independent town the
>   wrong way, for one hex more than needed).

I think its an optimal algorithm, provided 1 mp = 1 hex. 
I would require more information to investigate this though,
at least enough to repeat it.

> 
> * One case where an infantry just sat there in one my my towns, about
>   3-5 hexes short of its destination.  It was apparently using up its
>   ACP on each turn.  Won't be able to do anything about this unless I
>   notice it happening again (as in, I'm not even 100% sure it was
>   really a bug I saw).

Again I would need enough information to repeat this.

> 
> * One SIGSEGV.  Most interesting part of the stack trace follows.
>   Because the default compilation uses -O, it is hard to quickly
>   look at the variables at that point.
> 
> #5  0x080af4da in point_in_dir (x=6, y=0, dir=135856599, xp=0xbfffde44, yp=0x0)
>     at world.c:1459
> #6  0x08076069 in attack_blockage (side=0x8381a98, unit=0x8189dcc,
>     dirp1=1108574732) at ai.c:2483
> #7  0x0807144b in ai_react_to_task_result (side=0x6, unit=0x8189dcc,
>     task=0x8416394, rslt=TASK_FAILED) at ai.c:553

The y=0 looks highly suspicious. I did get this once myself in the coral
sea battle, but have never been able to repeat it. If anyone does get
this, could they tell us the game, and even perhaps sumit a checkpoint
file.

I have gone through
all the code I submitted and identified areas where I could be ignoring
the boundary and added extra assertion checks. I dont think
any of it is too onerous for production code.

Here is the patch.

-----------------------------------------------------------------------
diff -r -U 5 -p kernel_current/ai.c kernel/ai.c
--- kernel_current/ai.c	Fri Nov 14 18:15:41 2003
+++ kernel/ai.c	Fri Nov 14 22:13:38 2003
@@ -12,10 +12,11 @@ any later version.  See the file COPYING
    must include it. */
 
 #include "conq.h"
 #include "kpublic.h"
 #include "ai.h"
+#include <assert.h>
 
 extern void register_mplayer(AI_ops *ops);
 extern void register_iplayer(AI_ops *ops);
 
 extern int taskexecs;
@@ -2459,11 +2460,12 @@ blocked_by_enemy(Unit *unit, int tx, int
 		{
 			/* unusual situation. No previous path. Try one. */
 			dir = choose_move_direction(unit, tx, ty, 0, CMD_NONE);
 			if(dir < 0)return 0; /* blocked, but not by enemy. */
 		}
-		point_in_dir(unit->x, unit->y, dir, &x, &y);
+		if(!point_in_dir(unit->x, unit->y, dir, &x, &y))
+			return 0;
 	}
 
 
    	unit2 = unit_at(x, y);
    	if (in_play(unit2) && !trusted_side(unit->side, unit2->side))
@@ -2477,12 +2479,17 @@ blocked_by_enemy(Unit *unit, int tx, int
 int
 attack_blockage(Side *side, Unit *unit, int dirp1)
 {
     int x1,y1;
     Unit *unit2;
-	if(!dirp1)return FALSE;
-    point_in_dir(unit->x, unit->y, dirp1-1, &x1, &y1);
+    if(!dirp1)
+	return FALSE;
+    assert(dirp1 >= 1 && dirp1 <= NUMDIRS);
+    assert(in_area(unit->x,unit->y));
+    if(!point_in_dir(unit->x, unit->y, dirp1-1, &x1, &y1))
+	return FALSE;
+    assert(in_area(x1,y1));
     unit2 = unit_at(x1, y1);
     if (in_play(unit2)
         && !trusted_side(unit->side, unit2->side)
         && uu_zz_bhw(unit->type, unit2->type) > 0
         ) {
diff -r -U 5 -p kernel_current/move.c kernel/move.c
--- kernel_current/move.c	Fri Nov 14 18:15:42 2003
+++ kernel/move.c	Fri Nov 14 22:00:29 2003
@@ -6,10 +6,11 @@ it under the terms of the GNU General Pu
 the Free Software Foundation; either version 2, or (at your option)
 any later version.  See the file COPYING.  */
 
 #include "conq.h"
 #include "kernel.h"
+#include <assert.h>
 
 enum {
     over_nothing = 0,
     over_own = 1,
     over_border = 2,
@@ -223,13 +224,15 @@ do_move_action(Unit *unit, Unit *unit2, 
 	     * is only used for extraordinary circumstances, such as
 	     * retreating. In that case cached path is useless.
 	     */
 	    int dir = choose_move_direction(unit2, x, y, 0, CMD_NONE);
 	    if(dir < 0)break;
+	    assert(dir >= 0 && dir < NUMDIRS);
 
 	    /* Should be unit2 ? */
-	    interior_point_in_dir(unit->x, unit->y, dir, &ix, &iy);
+	    if(!interior_point_in_dir(unit->x, unit->y, dir, &ix, &iy))
+		break;
     	    if (type_can_occupy_cell(unit2->type, ix, iy)) {
 	    	ox = unit2->x;  
     	        oy = unit2->y;  
 		oz = unit2->z;
 	        speed = unit_speed(unit2, ix, iy);
@@ -1458,11 +1461,12 @@ pathable_point_for_unit_type(Side * side
     int fullness;
     int mpcost;
     int required_size;
     int capacity;
     if (dir >= 0) {
-	point_in_dir(tx, ty, opposite_dir(dir), &x, &y);
+	if(!point_in_dir(tx, ty, opposite_dir(dir), &x, &y))
+	    return 0;
     }
 
     /* check it the terrain itself is poisonous */
     if (terrain_always_impassable(u, t)) {
 	int c;
@@ -1657,11 +1661,12 @@ pathable_point(Unit * unit, int dir, str
 
     if (!terrain_visible(unit->side, tx, ty))
 	return 0;
 
     if (dir >= 0) {
-	point_in_dir(tx, ty, opposite_dir(dir), &x, &y);
+	if(!point_in_dir(tx, ty, opposite_dir(dir), &x, &y))
+	    return 0;
     } else {
 	x = unit->x;
 	y = unit->y;
     }
 
@@ -1796,11 +1801,13 @@ choose_move_direction(Unit * unit, int t
 	struct path_node_data *pnd;
 	int flags;
 	pnd = path_get_next_cached_node(unit, &goal_data, &flags);
 	if (pnd && flags == cmdflags) {
 	    /* assume the point is next. Do immediate call. */
-	    return path_get_next_move(unit, &goal_data, cmdflags);
+	    int dir = path_get_next_move(unit, &goal_data, cmdflags);
+	    assert(dir >= -1 && dir < NUMDIRS);
+	    return dir;
 	}
     }
 
     /* set this flag to speed things up. */
     can_be_carried = 0;
@@ -1859,10 +1866,11 @@ choose_move_direction(Unit * unit, int t
 	       unit->id,
 	       unit->type, u_type_name(unit->type),
 	       unit->x, unit->y, tx, ty, distance(unit->x, unit->y, tx,
 						  ty), path_cb_cnt, dir);
 
+    assert(dir >= -1 && dir < NUMDIRS);
     return dir;
 }
 
 /*
  * Return the transport unit if a transport change on an
@@ -1889,10 +1897,11 @@ get_cached_move_direction(Unit * unit, i
 {
     struct path_node_data goal_data;
     int dir;
     set_node_data(&goal_data, x, y, 0);
     dir = path_get_next_cached_move(unit, &goal_data);
+    assert(dir >= -1 && dir < NUMDIRS);
     return dir;
 }
 
 /*
  * Return a hash index based on node data coordinates.
@@ -1976,10 +1985,12 @@ cost_to_move_on_path(void *pkey, const s
 
     /* points should be adjacent. */
     dir = closest_dir(tx - x, ty - y);
     if (dir < 0)
 	return -1;
+
+    assert(dir < NUMDIRS);
 
     if (!pathable_point(unit, dir, p2, cmdflags, &mp_cost))
 	return -1;
 
     return mp_cost;
diff -r -U 5 -p kernel_current/task.c kernel/task.c
--- kernel_current/task.c	Fri Nov 14 18:15:43 2003
+++ kernel/task.c	Fri Nov 14 21:59:52 2003
@@ -6,10 +6,11 @@ it under the terms of the GNU General Pu
 the Free Software Foundation; either version 2, or (at your option)
 any later version.  See the file COPYING.  */
 
 #include "conq.h"
 #include "kernel.h"
+#include <assert.h>
 
 /* This is the number of tasks to allocate initially.  More will always be
    allocated as needed, so this should be a "reasonable" value. */
 
 #ifndef INITMAXTASKS
@@ -956,12 +957,14 @@ do_approach_subtask(Unit *unit, int tx, 
     if(s == 0 && !terrain_visible(unit->side, tx, ty))s++;
 
     dir = choose_move_direction(unit, tx, ty, s, cmdflags);
 
     if(dir < 0)return TASK_FAILED;
+    assert(dir >= 0 && dir < NUMDIRS);
 
-    point_in_dir(unit->x, unit->y, dir, &nx, &ny);
+    if(!point_in_dir(unit->x, unit->y, dir, &nx, &ny))
+	return TASK_FAILED;
 
     /* first, check any enemy on route. */
     for_all_stack(nx, ny, unit2) {
 	if (!trusted_side(unit->side, unit2->side)) {
             if (unit->occupant) {
@@ -1011,15 +1014,17 @@ do_approach_subtask(Unit *unit, int tx, 
 
 	if(!u_trans)
 	{
 	    /* try to hijack a ferry on any adjacent hex. */
 	    int i;
-	    for(i=0;i<6;i++)
+	    for(i=0;i<NUMDIRS;i++)
 	    {
 		int x,y;
-		if(i == dir)continue;
-                point_in_dir(unit->x, unit->y, i, &x, &y);
+		if(i == dir)
+		    continue;
+                if(!point_in_dir(unit->x, unit->y, i, &x, &y))
+		    continue;
 	        for_all_stack(x, y, unit2) {
 	            if(could_directly_board_ferry(unit,unit2))
 		    {
 		        u_trans = unit2;
 		        break;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]