- free_gamedata() is perfect.

- we can now save region uids for references, not x,y

src/common/kernel/border.c

@@ -520,10 +520,8 @@ write_borders(struct storage * store)
         if (b->type->valid && !b->type->valid(b)) continue;
         store->w_tok(store, b->type->__name);
         store->w_int(store, b->id);
-        store->w_int(store, b->from->x);
-        store->w_int(store, b->from->y);
-        store->w_int(store, b->to->x);
-        store->w_int(store, b->to->y);
+        store->w_int(store, b->from->uid);
+        store->w_int(store, b->to->uid);
 
         if (b->type->write) b->type->write(b, store);
         store->w_brk(store);
@@ -537,7 +535,6 @@ int
 read_borders(struct storage * store)
 {
   for (;;) {
-    short fx, fy, tx, ty;
     unsigned int bid = 0;
     char zText[32];
     border * b;
@@ -547,18 +544,19 @@ read_borders(struct storage * store)
     store->r_tok_buf(store, zText, sizeof(zText));
     if (!strcmp(zText, "end")) break;
     bid = store->r_int(store);
-    fx = (short)store->r_int(store);
-    fy = (short)store->r_int(store);
-    tx = (short)store->r_int(store);
-    ty = (short)store->r_int(store);
-
-    from = findregion(fx, fy);
-    if (from==NULL) {
-      log_error(("border for unknown region %d,%d\n", fx, fy));
-    }
-    to = findregion(tx, ty);
-    if (to==NULL)  {
-      log_error(("border for unknown region %d,%d\n", tx, ty));
+    if (store->version<UIDHASH_VERSION) {
+      short fx, fy, tx, ty;
+      fx = (short)store->r_int(store);
+      fy = (short)store->r_int(store);
+      tx = (short)store->r_int(store);
+      ty = (short)store->r_int(store);
+      from = findregion(fx, fy);
+      to = findregion(tx, ty);
+    } else {
+      unsigned int fid = (unsigned int)store->r_int(store);
+      unsigned int tid = (unsigned int)store->r_int(store);
+      from = findregionbyid(fid);
+      to = findregionbyid(tid);
     }
 
     type = find_bordertype(zText);

src/common/kernel/region.c

@@ -389,27 +389,50 @@ attrib_type at_moveblock = {
 static region * regionhash[RMAXHASH];
 static int dummy_data;
 static region * dummy_ptr = (region*)&dummy_data; /* a funny hack */
-static unsigned int uidhash[MAXREGIONS];
+
+typedef struct uidhashentry {
+  unsigned int uid;
+  region * r;
+} uidhashentry;
+static uidhashentry uidhash[MAXREGIONS];
+
+struct region *
+findregionbyid(unsigned int uid)
+{
+  int key = uid % MAXREGIONS;
+  while (uidhash[key].uid!=0 && uidhash[key].uid!=uid) ++key;
+  return uidhash[key].r;
+}
 
 #define DELMARKER dummy_ptr
 
-unsigned int 
-generate_region_id(void)
+static void
+unhash_uid(region * r)
 {
-  unsigned int uid;
+  int key = r->uid % MAXREGIONS;
+  assert(r->uid);
+  while (uidhash[key].uid!=0 && uidhash[key].uid!=r->uid) ++key;
+  assert(uidhash[key].r==r);
+  uidhash[key].r = NULL;
+}
+
+static void
+hash_uid(region * r)
+{
+  unsigned int uid = r->uid;
   for (;;) {
-    int key;
-    do {
-      uid = rng_int();
-    } while (uid==0);
-    key = uid % MAXREGIONS;
-    while (uidhash[key]!=0 && uidhash[key]!=uid) ++key;
-    if (uidhash[key]==0) {
-      uidhash[key] = uid;
-      break;
+    if (uid!=0) {
+      int key = uid % MAXREGIONS;
+      while (uidhash[key].uid!=0 && uidhash[key].uid!=uid) ++key;
+      if (uidhash[key].uid==0) {
+        uidhash[key].uid = uid;
+        uidhash[key].r = r;
+        break;
+      } 
+      assert(uidhash[key].r!=r || !"duplicate registration");
     }
+    uid = rng_int();
   }
-  return uid;
 }
 
 #define HASH_STATISTICS 1
@@ -837,11 +860,11 @@ new_region(short x, short y, unsigned int uid)
   r = calloc(1, sizeof(region));
   r->x = x;
   r->y = y;
-  if (uid==0) uid = generate_region_id();
   r->uid = uid;
   r->age = 1;
   r->planep = findplane(x, y);
   rhash(r);
+  hash_uid(r);
   if (last)
     addlist(&last, r);
   else
@@ -865,6 +888,7 @@ remove_region(region ** rlist, region * r)
   }
 
   runhash(r);
+  unhash_uid(r);
   while (*rlist && *rlist!=r) rlist=&(*rlist)->next;
   assert(*rlist==r);
   *rlist = r->next;
@@ -1217,20 +1241,44 @@ production(const region *r)
   return p;
 }
 
+static void
+resolve_region(variant id, void * address) {
+  region * r = findregion(id.sa[0], id.sa[1]);
+  *(region**)address = r;
+}
+
+static void
+resolve_regionbyid(variant id, void * address) {
+  region * r = findregionbyid((unsigned int)id.i);
+  *(region**)address = r;
+}
+
 int
 read_region_reference(region ** r, struct storage * store)
 {
-  variant coor;
-  coor.sa[0] = (short)store->r_int(store);
-  coor.sa[1] = (short)store->r_int(store);
-  if (coor.sa[0]==SHRT_MAX) {
-    *r = NULL;
-    return AT_READ_FAIL;
-  }
-  *r = findregion(coor.sa[0], coor.sa[1]);
-
-  if (*r==NULL) {
-    ur_add(coor, (void**)r, resolve_region);
+  if (store->version<UIDHASH_VERSION) {
+    variant coor;
+    coor.sa[0] = (short)store->r_int(store);
+    coor.sa[1] = (short)store->r_int(store);
+    if (coor.sa[0]==SHRT_MAX) {
+      *r = NULL;
+      return AT_READ_FAIL;
+    }
+    *r = findregion(coor.sa[0], coor.sa[1]);
+    if (*r==NULL) {
+      ur_add(coor, (void**)r, resolve_region);
+    }
+  } else {
+    variant uid;
+    uid.i = store->r_int(store);
+    if (uid.i==0) {
+      *r = NULL;
+    } else {
+      *r = findregionbyid((unsigned int)uid.i);
+      if (*r==NULL) {
+        ur_add(uid, (void**)r, resolve_regionbyid);
+      }
+    }
   }
   return AT_READ_OK;
 }
@@ -1239,22 +1287,12 @@ void
 write_region_reference(const region * r, struct storage * store)
 {
   if (r) {
-    store->w_int(store, r->x);
-    store->w_int(store, r->y);
-  }
-  else {
-    store->w_int(store, SHRT_MAX);
-    store->w_int(store, SHRT_MAX);
+    store->w_int(store, r->uid);
+  } else {
+    store->w_int(store, 0);
   }
 }
 
-void
-resolve_region(variant id, void * address) {
-  region * r = findregion(id.sa[0], id.sa[1]);
-  *(region**)address = r;
-}
-
-
 struct message_list *
 r_getmessages(const struct region * r, const struct faction * viewer)
 {

src/common/kernel/region.h

@@ -146,6 +146,7 @@ int distance(const struct region*, const struct region*);
 int koor_distance(int ax, int ay, int bx, int by) ;
 extern direction_t reldirection(const struct region * from, const struct region * to);
 extern struct region * findregion(short x, short y);
+extern struct region * findregionbyid(unsigned int uid);
 
 extern struct attrib_type at_direction;
 extern struct attrib_type at_moveblock;
@@ -219,7 +220,6 @@ extern int r_demand(const struct region * r, const struct luxury_type * ltype);
 extern const char * regionname(const struct region * r, const struct faction * f);
 extern const char * write_regionname(const struct region * r, const struct faction * f, char * buffer, size_t size);
 
-extern void resolve_region(variant data, void * address);
 extern struct region * new_region(short x, short y, unsigned int uid);
 extern void remove_region(region ** rlist, region * r);
 extern void terraform(struct region * r, terrain_t terrain);
@@ -242,8 +242,6 @@ extern struct region * r_connect(const struct region *, direction_t dir);
 # define rconnect(r, dir) r_connect(r, dir)
 #endif
 
-extern unsigned int generate_region_id(void);
-
 extern void free_regions(void);
 
 #ifdef __cplusplus

src/common/kernel/save.c

@@ -926,14 +926,12 @@ readregion(struct storage * store, short x, short y)
 
   if (store->version>=UID_VERSION) {
     uid = store->r_int(store);
-  } else {
-    uid = generate_region_id();
   }
 
   if (r==NULL) {
     r = new_region(x, y, uid);
   } else {
-    r->uid = uid;
+    assert(uid==0 || r->uid==uid);
     current_region = r;
     while (r->attribs) a_remove(&r->attribs, r->attribs);
     if (r->land) {
@@ -1336,7 +1334,7 @@ writefaction(struct storage * store, const faction * f)
   store->w_tok(store, "end");
   store->w_brk(store);
   store->w_int(store, listlen(f->ursprung));
-  for(ur = f->ursprung;ur;ur=ur->next) {
+  for (ur = f->ursprung;ur;ur=ur->next) {
     store->w_int(store, ur->id);
     store->w_int(store, ur->x);
     store->w_int(store, ur->y);

src/common/kernel/version.h

@@ -57,6 +57,7 @@
 #define INTPAK_VERSION 329 /* in binary, ints can get packed */
 #define NOZEROIDS_VERSION 330 /* zero is not a valid ID for anything (including factions) */
 #define NOBORDERATTRIBS_VERSION 331 /* border::attribs has been moved to userdata */
+#define UIDHASH_VERSION 332 /* borders use the region.uid to store */
 
 #define MIN_VERSION CURSETYPE_VERSION /* minimal datafile we support */
-#define RELEASE_VERSION NOBORDERATTRIBS_VERSION /* current datafile */
+#define RELEASE_VERSION UIDHASH_VERSION /* current datafile */

src/scripts/run-tests.lua

@@ -56,12 +56,12 @@ run_scripts()
 local now = os.clock()
 -- test_free()
 read_game("571.dat", "binary")
-write_game("571.txt.1", "text")
+write_game("571.dat.1", "binary")
 free_game()
 read_game("570.dat", "binary")
 free_game()
 read_game("571.dat", "binary")
-write_game("571.txt.2", "text")
+write_game("571.dat.2", "binary")
 local elapsed = os.clock() - now
 print(elapsed)
 -- text: 50.574