more strictly define what it means to be the owner of the building, and when it is transferred.

test coverage is a winderful thing.
UFL_OWNER is probably entirely useless now, must fix

src/gamecode/laws.c

@@ -3362,8 +3362,7 @@ static void new_units(void)
             if (token && token[0]) {
               name = strdup(token);
             }
-            u2 =
-              create_unit(r, u->faction, 0, u->faction->race, alias, name, u);
+            u2 = create_unit(r, u->faction, 0, u->faction->race, alias, name, u);
             if (name != NULL)
               free(name);
             fset(u2, UFL_ISNEW);

src/kernel/building.c

@@ -634,10 +634,8 @@ void building_set_owner(struct building *b, struct unit * owner)
   fset(owner, UFL_OWNER);
 }
 
-unit *building_owner(const building * bld)
+static unit *building_owner_ex(const building * bld, const faction * last_owner)
 {
-  unit *owner = bld->_owner;
-  if (owner && (owner->building!=bld || owner->number<=0)) {
   unit *u, *heir = 0;
 
   /* Eigentümer tot oder kein Eigentümer vorhanden. Erste lebende Einheit
@@ -645,9 +643,9 @@ unit *building_owner(const building * bld)
   for (u = bld->region->units; u; u = u->next) {
     if (u->building == bld) {
       if (u->number > 0) {
-          if (u->faction==owner->faction) {
+        if (heir && last_owner && heir->faction!=last_owner && u->faction==last_owner) {
           heir = u;
-            break;
+          break; /* we found someone from the same faction who is not dead. let's take this guy */
         }
         else if (!heir) {
           heir = u; /* you'll do in an emergency */
@@ -656,16 +654,24 @@ unit *building_owner(const building * bld)
       freset(u, UFL_OWNER);
     }
   }
-    freset(owner, UFL_OWNER);
-    owner->building = 0;
-    owner = heir;
-    if (owner) {
-      fset(owner, UFL_OWNER);
-    }
+  return heir;
+}
+
+unit *building_owner(const building * bld)
+{
+  unit *owner = bld->_owner;
+  if (!owner || (owner->building!=bld || owner->number<=0)) {
+    unit * heir = building_owner_ex(bld, owner?owner->faction:0);
+    return (heir && heir->number>0) ? heir : 0;
   }
   return owner;
 }
 
+void building_update_owner(building * bld) {
+  unit * owner = bld->_owner;
+  bld->_owner = building_owner_ex(bld, owner?owner->faction:0);
+}
+
 const char *building_getname(const building * self)
 {
   return self->name;

src/kernel/building.h

@@ -153,6 +153,7 @@ extern "C" {
 
   extern struct unit *building_owner(const struct building *b);
   extern void building_set_owner(struct building *b, struct unit * u);
+  extern void building_update_owner(struct building * bld);
 
   extern struct attrib_type at_building_action;
   void building_addaction(struct building *b, const char *fname,

src/kernel/building_test.c

@@ -54,7 +54,7 @@ static void test_building_set_owner(CuTest * tc)
   CuAssertPtrEquals(tc, u2, building_owner(bld));
 }
 
-static void test_buildingowner_goes_to_next_after_death(CuTest * tc)
+static void test_buildingowner_goes_to_next_when_empty(CuTest * tc)
 {
   struct region *r;
   struct building *bld;
@@ -88,7 +88,7 @@ static void test_buildingowner_goes_to_next_after_death(CuTest * tc)
   CuAssertPtrEquals(tc, u2, building_owner(bld));
 }
 
-static void test_buildingowner_goes_to_other_after_death(CuTest * tc)
+static void test_buildingowner_goes_to_other_when_empty(CuTest * tc)
 {
   struct region *r;
   struct building *bld;
@@ -123,7 +123,7 @@ static void test_buildingowner_goes_to_other_after_death(CuTest * tc)
   CuAssertPtrEquals(tc, u2, building_owner(bld));
 }
 
-static void test_buildingowner_goes_to_same_faction_after_death(CuTest * tc)
+static void test_buildingowner_goes_to_same_faction_when_empty(CuTest * tc)
 {
   struct region *r;
   struct building *bld;
@@ -159,12 +159,8 @@ static void test_buildingowner_goes_to_same_faction_after_death(CuTest * tc)
   CuAssertTrue(tc, fval(u, UFL_OWNER));
   u->number = 0;
   CuAssertPtrEquals(tc, u3, building_owner(bld));
-  CuAssertTrue(tc, !fval(u, UFL_OWNER));
-  CuAssertTrue(tc, fval(u3, UFL_OWNER));
   u3->number = 0;
   CuAssertPtrEquals(tc, u2, building_owner(bld));
-  CuAssertTrue(tc, !fval(u3, UFL_OWNER));
-  CuAssertTrue(tc, fval(u2, UFL_OWNER));
 }
 
 static void test_buildingowner_goes_to_next_after_leave(CuTest * tc)
@@ -276,7 +272,7 @@ static void test_buildingowner_goes_to_same_faction_after_leave(CuTest * tc)
   CuAssertPtrEquals(tc, 0, building_owner(bld));
 }
 
-static void test_buildingowner_resets_when_dead(CuTest * tc)
+static void test_buildingowner_resets_when_empty(CuTest * tc)
 {
   struct region *r;
   struct building *bld;
@@ -306,10 +302,49 @@ static void test_buildingowner_resets_when_dead(CuTest * tc)
   CuAssertPtrEquals(tc, u, building_owner(bld));
   u->number = 0;
   CuAssertPtrEquals(tc, 0, building_owner(bld));
+  u->number = 1;
+  CuAssertPtrEquals(tc, u, building_owner(bld));
 }
 
-void test_buildingowner_goes_to_empty_unit_if_no_other(CuTest * tc)
+void test_buildingowner_goes_to_empty_unit_after_leave(CuTest * tc)
 {
+  struct region *r;
+  struct building *bld;
+  struct unit *u1, *u2, *u3;
+  struct faction *f1;
+  const struct building_type *btype;
+  const struct race *human;
+
+  test_cleanup();
+  test_create_world();
+
+  human = rc_find("human");
+  CuAssertPtrNotNull(tc, human);
+
+  btype = bt_find("castle");
+  CuAssertPtrNotNull(tc, btype);
+
+  f1 = test_create_faction(human);
+  r = findregion(0, 0);
+
+  bld = test_create_building(r, btype);
+  CuAssertPtrNotNull(tc, bld);
+
+  u1 = test_create_unit(f1, r);
+  u2 = test_create_unit(f1, r);
+  u3 = test_create_unit(f1, r);
+  u_set_building(u1, bld);
+  u_set_building(u2, bld);
+  u_set_building(u3, bld);
+
+  CuAssertPtrEquals(tc, u1, building_owner(bld));
+  u2->number = 0;
+  leave_building(u1);
+  CuAssertPtrEquals(tc, u3, building_owner(bld));
+  leave_building(u3);
+  CuAssertPtrEquals(tc, 0, building_owner(bld));
+  u2->number = 1;
+  CuAssertPtrEquals(tc, u2, building_owner(bld));
 }
 
 CuSuite *get_building_suite(void)
@@ -317,13 +352,13 @@ CuSuite *get_building_suite(void)
   CuSuite *suite = CuSuiteNew();
   SUITE_ADD_TEST(suite, test_register_building);
   SUITE_ADD_TEST(suite, test_building_set_owner);
-  SUITE_ADD_TEST(suite, test_buildingowner_resets_when_dead);
-  SUITE_ADD_TEST(suite, test_buildingowner_goes_to_next_after_death);
-  SUITE_ADD_TEST(suite, test_buildingowner_goes_to_other_after_death);
-  SUITE_ADD_TEST(suite, test_buildingowner_goes_to_same_faction_after_death);
+  SUITE_ADD_TEST(suite, test_buildingowner_resets_when_empty);
+  SUITE_ADD_TEST(suite, test_buildingowner_goes_to_next_when_empty);
+  SUITE_ADD_TEST(suite, test_buildingowner_goes_to_other_when_empty);
+  SUITE_ADD_TEST(suite, test_buildingowner_goes_to_same_faction_when_empty);
   SUITE_ADD_TEST(suite, test_buildingowner_goes_to_next_after_leave);
   SUITE_ADD_TEST(suite, test_buildingowner_goes_to_other_after_leave);
   SUITE_ADD_TEST(suite, test_buildingowner_goes_to_same_faction_after_leave);
-  SUITE_ADD_TEST(suite, test_buildingowner_goes_to_empty_unit_if_no_other);
+  SUITE_ADD_TEST(suite, test_buildingowner_goes_to_empty_unit_after_leave);
   return suite;
 }

src/kernel/unit.c

@@ -809,7 +809,8 @@ void leave_building(unit * u)
 
   u->building = 0;
   if (b->_owner==u) {
-    b->_owner = building_owner(b);
+    building_update_owner(b);
+    assert(b->_owner!=u);
   }
 }