additional testing for largestbuilding and comparisons.

src/economy.c

@@ -575,7 +575,7 @@ void give_control(unit * u, unit * u2)
 
             assert(u->building == u2->building);
             if (f == u->faction) {
-                building *b = largestbuilding(r, &cmp_current_owner, false);
+                building *b = largestbuilding(r, cmp_current_owner, false);
                 if (b == u->building) {
                     friendly_takeover(r, u2->faction);
                 }

src/guard.c

@@ -112,7 +112,7 @@ static bool is_guardian_r(const unit * guard)
     if (guard->building && rule_region_owners() && guard == building_owner(guard->building)) {
         faction *owner = region_get_owner(guard->region);
         if (owner == guard->faction) {
-            building *bowner = largestbuilding(guard->region, &cmp_taxes, false);
+            building *bowner = largestbuilding(guard->region, cmp_taxes, false);
             if (bowner == guard->building) {
                 return true;
             }

src/kernel/building.c

@@ -523,10 +523,10 @@ static unit *building_owner_ex(const building * bld, const struct faction * last
     }
     if (!heir && config_token("rules.region_owner_pay_building", bld->type->_name)) {
         if (rule_region_owners()) {
-            u = building_owner(largestbuilding(bld->region, &cmp_taxes, false));
+            u = building_owner(largestbuilding(bld->region, cmp_taxes, false));
         }
         else {
-            u = building_owner(largestbuilding(bld->region, &cmp_wage, false));
+            u = building_owner(largestbuilding(bld->region, cmp_wage, false));
         }
         if (u) {
             heir = u;
@@ -643,9 +643,12 @@ bool is_building_type(const struct building_type *btype, const char *name) {
 building *largestbuilding(const region * r, cmp_building_cb cmp_gt,
     bool imaginary)
 {
-    building *b, *best = NULL;
+    building *b, *best = r->buildings;
 
-    for (b = rbuildings(r); b; b = b->next) {
+    if (!best) {
+        return NULL;
+    }
+    for (b = best->next; b; b = b->next) {
         if (cmp_gt(b, best) <= 0)
             continue;
         if (!imaginary) {
@@ -673,7 +676,7 @@ static const int wagetable[7][4] = {
 static int
 default_wage(const region * r, const faction * f, const race * rc, int in_turn)
 {
-    building *b = largestbuilding(r, &cmp_wage, false);
+    building *b = largestbuilding(r, cmp_wage, false);
     int esize = 0;
     double wage;
     static int ct_cache;
@@ -786,11 +789,14 @@ bool is_owner_building(const struct building * b)
 
 int building_taxes(const building *b, int bsize) {
     assert(b);
-    if (!b->type->taxes) return 0;
+    if (b->type->taxes) {
-    else {
         int level = buildingeffsize(b, false);
-        return (int)(0.5+1/b->type->taxes(b, level));
+        double tax = b->type->taxes(b, level);
+        if (tax > 0) {
+            return (int)(0.5 + 1 / tax);
         }
+    }
+    return 0;
 }
 
 
@@ -817,8 +823,9 @@ int cmp_taxes(const building * b, const building * a)
             else {
                 if (u && u->faction == f) {
                     u = building_owner(a);
-                    if (u && u->faction == f)
+                    if (u && u->faction == f) {
-                        return -1;
+                        return 0;
+                    }
                     return 1;
                 }
             }
@@ -827,7 +834,7 @@ int cmp_taxes(const building * b, const building * a)
             return 1;
         }
     }
-    return -1;
+    return 0;
 }
 
 int cmp_current_owner(const building * b, const building * a)
@@ -843,10 +850,10 @@ int cmp_current_owner(const building * b, const building * a)
             int newtaxes = building_taxes(b, b->size);
             int oldtaxes = building_taxes(a, a->size);
 
-            if (newtaxes < oldtaxes) {
+            if (newtaxes > oldtaxes) {
                 return 1;
             }
-            if (newtaxes > oldtaxes) {
+            if (newtaxes < oldtaxes) {
                 return -1;
             }
             //if (newsize != oldsize) {
@@ -858,5 +865,5 @@ int cmp_current_owner(const building * b, const building * a)
             return 1;
         }
     }
-    return -1;
+    return 0;
 }

src/kernel/building.test.c

@@ -427,10 +427,86 @@ static void test_cmp_castle_size(CuTest *tc) {
     u_set_building(u2, b2);
     b1->size = 5;
     b2->size = 10;
-    CuAssertTrue(tc, cmp_castle_size(b1, b2)<0);
+    CuAssertTrue(tc, cmp_castle_size(b1, b2) < 0);
-    CuAssertTrue(tc, cmp_castle_size(b2, b1)>0);
+    CuAssertTrue(tc, cmp_castle_size(b2, b1) > 0);
-    CuAssertTrue(tc, cmp_castle_size(b1, NULL)>0);
+    CuAssertTrue(tc, cmp_castle_size(b1, b1) == 0);
-    CuAssertTrue(tc, cmp_castle_size(NULL, b1)<0);
+    test_cleanup();
+}
+
+static double tax_cb(const building *b, int level) {
+    UNUSED_ARG(level);
+    return b->size * 0.01;
+}
+
+static void test_cmp_wage(CuTest *tc) {
+    region *r;
+    building *b1, *b2;
+    building_type *btype;
+
+    test_setup();
+    btype = test_create_buildingtype("castle");
+    btype->taxes = tax_cb;
+    r = test_create_region(0, 0, 0);
+    b1 = test_create_building(r, btype);
+    b2 = test_create_building(r, btype);
+    b1->size = 5;
+    b2->size = 10;
+    CuAssertPtrEquals(tc, b2, largestbuilding(r, cmp_wage, false));
+    CuAssertTrue(tc, cmp_wage(b1, b2) < 0);
+    CuAssertTrue(tc, cmp_wage(b2, b1) > 0);
+    CuAssertTrue(tc, cmp_wage(b1, b1) == 0);
+    test_cleanup();
+}
+
+static void test_cmp_taxes(CuTest *tc) {
+    region *r;
+    building *b1, *b2;
+    building_type *btype;
+    unit *u1, *u2;
+
+    test_setup();
+    btype = test_create_buildingtype("castle");
+    btype->taxes = tax_cb;
+    r = test_create_region(0, 0, 0);
+    b1 = test_create_building(r, btype);
+    b2 = test_create_building(r, btype);
+    b1->size = 5;
+    b2->size = 10;
+    u1 = test_create_unit(test_create_faction(0), r);
+    u_set_building(u1, b1);
+    u2 = test_create_unit(test_create_faction(0), r);
+    u_set_building(u2, b2);
+    CuAssertPtrEquals(tc, b2, largestbuilding(r, cmp_taxes, false));
+    CuAssertTrue(tc, cmp_taxes(b1, b2) < 0);
+    CuAssertTrue(tc, cmp_taxes(b2, b1) > 0);
+    CuAssertTrue(tc, cmp_taxes(b1, b1) == 0);
+    test_cleanup();
+}
+
+static void test_cmp_current_owner(CuTest *tc) {
+    region *r;
+    building *b1, *b2;
+    building_type *btype;
+    unit *u1, *u2;
+
+    test_setup();
+    config_set("rules.region_owners", "1");
+    btype = test_create_buildingtype("castle");
+    btype->taxes = tax_cb;
+    r = test_create_region(0, 0, 0);
+    b1 = test_create_building(r, btype);
+    b2 = test_create_building(r, btype);
+    b1->size = 5;
+    b2->size = 10;
+    u1 = test_create_unit(test_create_faction(0), r);
+    u_set_building(u1, b1);
+    u2 = test_create_unit(test_create_faction(0), r);
+    u_set_building(u2, b2);
+    region_set_owner(r, u1->faction, turn);
+    CuAssertPtrEquals(tc, b1, largestbuilding(r, cmp_current_owner, false));
+    CuAssertTrue(tc, cmp_current_owner(b2, b1) < 0);
+    CuAssertTrue(tc, cmp_current_owner(b1, b2) > 0);
+    CuAssertTrue(tc, cmp_current_owner(b1, b1) == 0);
     test_cleanup();
 }
 
@@ -459,10 +535,36 @@ static void test_building_effsize(CuTest *tc) {
     test_cleanup();
 }
 
+static int cmp_size(const building *lhs, const building *rhs) {
+    return lhs->size - rhs->size;
+}
+
+static void test_largestbuilding(CuTest *tc) {
+    region *r;
+    building *b1, *b2;
+    test_setup();
+    r = test_create_region(0, 0, NULL);
+    CuAssertPtrEquals(tc, NULL, largestbuilding(r, cmp_size, false));
+    b1 = test_create_building(r, NULL);
+    b2 = test_create_building(r, NULL);
+    b1->size = 1;
+    b2->size = 1;
+    CuAssertPtrEquals(tc, b1, largestbuilding(r, cmp_size, false));
+    b1->size = 2;
+    CuAssertPtrEquals(tc, b1, largestbuilding(r, cmp_size, false));
+    b2->size = 3;
+    CuAssertPtrEquals(tc, b2, largestbuilding(r, cmp_size, false));
+    test_cleanup();
+}
+
 CuSuite *get_building_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
+    SUITE_ADD_TEST(suite, test_largestbuilding);
     SUITE_ADD_TEST(suite, test_cmp_castle_size);
+    SUITE_ADD_TEST(suite, test_cmp_taxes);
+    SUITE_ADD_TEST(suite, test_cmp_wage);
+    SUITE_ADD_TEST(suite, test_cmp_current_owner);
     SUITE_ADD_TEST(suite, test_register_building);
     SUITE_ADD_TEST(suite, test_btype_defaults);
     SUITE_ADD_TEST(suite, test_building_set_owner);

src/kernel/region.c

@@ -1398,8 +1398,8 @@ faction *update_owners(region * r)
     faction *f = NULL;
     assert(rule_region_owners());
     if (r->land) {
-        building *bowner = largestbuilding(r, &cmp_current_owner, false);
+        building *bowner = largestbuilding(r, cmp_current_owner, false);
-        building *blargest = largestbuilding(r, &cmp_taxes, false);
+        building *blargest = largestbuilding(r, cmp_taxes, false);
         if (blargest) {
             if (!bowner || bowner->size < blargest->size) {
                 /* region owners update? */

src/morale.c

@@ -43,7 +43,7 @@ void morale_update(region *r) {
     if (r->land->ownership && r->land->ownership->owner) {
         int stability = turn - r->land->ownership->morale_turn;
         int maxmorale = MORALE_DEFAULT;
-        building *b = largestbuilding(r, &cmp_taxes, false);
+        building *b = largestbuilding(r, cmp_taxes, false);
         if (b) {
             int bsize = buildingeffsize(b, false);
             maxmorale = (int)(0.5 + b->type->taxes(b, bsize + 1) * MORALE_TAX_FACTOR);