Merge pull request #47 from badgerman/master

refactoring, tests, old datafile compatibility.

src/bindings.c

@@ -68,6 +68,7 @@ without prior permission by the authors of Eressea.
 #include <quicklist.h>
 #include <util/rand.h>
 #include <util/rng.h>
+#include <util/xml.h>
 
 #include <storage.h>
 
@@ -1001,6 +1002,21 @@ static int tolua_get_spells(lua_State * L)
   return tolua_quicklist_push(L, "spell_list", "spell", spells);
 }
 
+static int init_data(const char *filename, const char *catalog)
+{
+    int l;
+    l = read_xml(filename, catalog);
+    reset_locales();
+    if (l) {
+        return l;
+    }
+    if (turn < 0) {
+        turn = first_turn;
+    }
+    return 0;
+}
+
+
 int tolua_read_xml(lua_State * L)
 {
   const char *filename = tolua_tostring(L, 1, "config.xml");

src/kernel/ally.c

@@ -14,7 +14,7 @@ ally * ally_add(ally **al_p, struct faction *f) {
     ally * al;
     while (*al_p) {
         al = *al_p;
-    if (al->faction==f) return al;
+        if (f && al->faction == f) return al;
         al_p = &al->next;
     }
     al = (ally *)malloc(sizeof(ally));

src/kernel/ally.test.c

@@ -19,10 +19,25 @@ static void test_ally(CuTest * tc)
     CuAssertPtrEquals(tc, 0, ally_find(al, f1));
 }
 
+static void test_ally_null(CuTest * tc)
+{
+    ally *a1 = 0, *a2 = 0;
+
+    a1 = ally_add(&a1, 0);
+    a2 = ally_add(&a1, 0);
+    CuAssertPtrNotNull(tc, a1);
+    CuAssertPtrNotNull(tc, a2);
+    CuAssertPtrEquals(tc, a2, a1->next);
+    CuAssertPtrEquals(tc, 0, a2->next);
+    free(a1);
+    free(a2);
+}
+
 CuSuite *get_ally_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
     SUITE_ADD_TEST(suite, test_ally);
+    SUITE_ADD_TEST(suite, test_ally_null);
     return suite;
 }
 

src/kernel/faction.c

@@ -430,10 +430,8 @@ void set_alliance(faction * a, faction * b, int status)
         sfp = &sf->next;
     }
     if (*sfp == NULL) {
-        ally *sf = *sfp = malloc(sizeof(ally));
+        ally *sf = ally_add(sfp, b);
-        sf->next = NULL;
         sf->status = status;
-        sf->faction = b;
         return;
     }
     (*sfp)->status |= status;

src/kernel/group.c

@@ -73,9 +73,8 @@ static void init_group(faction * f, group * g)
     an = &g->allies;
     for (a = f->allies; a; a = a->next)
         if (a->faction) {
-      ally *ga = calloc(sizeof(ally), 1);
+            ally *ga = ally_add(an, a->faction);
-      *ga = *a;
+            ga->status = a->status;
-      *an = ga;
             an = &ga->next;
         }
 }
@@ -173,7 +172,8 @@ void set_group(struct unit *u, struct group *g)
         }
         a->data.v = g;
         g->members++;
-  } else if (a) {
+    }
+    else if (a) {
         a_remove(&u->attribs, a);
         freset(u, UFL_GROUP);
     }
@@ -232,17 +232,14 @@ void read_groups(struct storage *store, faction * f)
         for (;;) {
             ally *a;
             variant fid;
+
             READ_INT(store, &fid.i);
             if (fid.i <= 0)
                 break;
             if (global.data_version < STORAGE_VERSION && fid.i == 0)
                 break;
-      a = malloc(sizeof(ally));
+            a = ally_add(pa, findfaction(fid.i));
-      *pa = a;
-      pa = &a->next;
             READ_INT(store, &a->status);
-
-      a->faction = findfaction(fid.i);
             if (!a->faction)
                 ur_add(fid, &a->faction, resolve_faction);
         }

src/kernel/pool.c

@@ -167,7 +167,7 @@ int count)
 
     if ((mode & GET_SLACK) && (mode & GET_RESERVE))
         use = have;
-    else {
+    else if (mode & (GET_SLACK|GET_RESERVE)) {
         int reserve = get_reservation(u, rtype);
         int slack = _max(0, have - reserve);
         if (mode & GET_RESERVE)
@@ -177,11 +177,9 @@ int count)
     }
     if (rtype->flags & RTF_POOLED && mode & ~(GET_SLACK | GET_RESERVE)) {
         for (v = r->units; v && use < count; v = v->next)
-            if (u != v) {
+            if (u != v && (u->items || rtype->uget)) {
             int mask;
 
-            if (v->items == NULL && rtype->uget == NULL)
-                continue;
             if ((urace(v)->ec_flags & GIVEITEM) == 0)
                 continue;
 

src/kernel/pool.h

@@ -29,8 +29,8 @@ extern "C" {
 #define GET_POOLED_SLACK    0x08
 #define GET_POOLED_RESERVE  0x10
 #define GET_POOLED_FORCE    0x20        /* ignore f->options pools */
-#define GET_ALLIED_SLACK    0x30
+#define GET_ALLIED_SLACK    0x40
-#define GET_ALLIED_RESERVE  0x40
+#define GET_ALLIED_RESERVE  0x80
 
 /* for convenience: */
 #define GET_DEFAULT (GET_RESERVE|GET_SLACK|GET_POOLED_SLACK)

src/kernel/pool.test.c

@@ -1,15 +1,64 @@
 #include <platform.h>
 #include <kernel/types.h>
 
+#include "ally.h"
 #include "pool.h"
 #include "magic.h"
 #include "unit.h"
 #include "item.h"
+#include "faction.h"
 #include "region.h"
 #include "skill.h"
 
 #include <CuTest.h>
 #include <tests.h>
+#include <assert.h>
+#include <limits.h>
+
+void test_pool(CuTest *tc) {
+    unit *u1, *u2, *u3;
+    faction *f;
+    region *r;
+    struct resource_type *rtype;
+    ally *al;
+
+    test_cleanup();
+    test_create_world();
+    rtype = rt_get_or_create("money");
+    it_get_or_create(rtype);
+    f = test_create_faction(0);
+    r = findregion(0, 0);
+    assert(r && f && rtype && rtype->itype);
+    u1 = test_create_unit(f, r);
+    u2 = test_create_unit(f, r);
+    u3 = test_create_unit(test_create_faction(0), r);
+    assert(u1 && u2);
+    i_change(&u1->items, rtype->itype, 100);
+    set_resvalue(u1, rtype, 50);
+    i_change(&u2->items, rtype->itype, 200);
+    set_resvalue(u2, rtype, 100);
+    i_change(&u3->items, rtype->itype, 400);
+    set_resvalue(u3, rtype, 200);
+
+    CuAssertIntEquals(tc, 50, get_pooled(u1, rtype, GET_SLACK, 40));
+    CuAssertIntEquals(tc, 50, get_pooled(u1, rtype, GET_SLACK, INT_MAX));
+    CuAssertIntEquals(tc, 100, get_pooled(u1, rtype, GET_SLACK | GET_RESERVE, INT_MAX));
+    CuAssertIntEquals(tc, 150, get_pooled(u1, rtype, GET_SLACK | GET_POOLED_SLACK, INT_MAX));
+    CuAssertIntEquals(tc, 100, get_pooled(u1, rtype, GET_POOLED_SLACK, INT_MAX));
+    CuAssertIntEquals(tc, 200, get_pooled(u1, rtype, GET_POOLED_SLACK | GET_POOLED_RESERVE, INT_MAX));
+
+    al = ally_add(&u3->faction->allies, f);
+    al->status = HELP_GUARD;
+    CuAssertIntEquals(tc, 0, get_pooled(u1, rtype, GET_ALLIED_SLACK | GET_ALLIED_RESERVE, INT_MAX));
+    al->status = HELP_MONEY;
+    CuAssertIntEquals(tc, 200, get_pooled(u1, rtype, GET_ALLIED_SLACK, INT_MAX));
+    CuAssertIntEquals(tc, 200, get_pooled(u1, rtype, GET_ALLIED_RESERVE, INT_MAX));
+    CuAssertIntEquals(tc, 400, get_pooled(u1, rtype, GET_ALLIED_SLACK | GET_ALLIED_RESERVE, INT_MAX));
+
+    CuAssertIntEquals(tc, 100, get_pooled(u1, rtype, GET_ALL, 50));
+    CuAssertIntEquals(tc, 300, get_pooled(u1, rtype, GET_ALL, 150));
+    CuAssertIntEquals(tc, 300, get_pooled(u1, rtype, GET_ALL, INT_MAX));
+}
 
 void test_change_resource(CuTest * tc)
 {
@@ -41,7 +90,7 @@ void test_change_resource(CuTest * tc)
 CuSuite *get_pool_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
-/*  SUITE_ADD_TEST(suite, test_pool); */
+    SUITE_ADD_TEST(suite, test_pool);
     SUITE_ADD_TEST(suite, test_change_resource);
     return suite;
 }

src/kernel/save.c

@@ -1119,8 +1119,11 @@ static ally **addally(const faction * f, ally ** sfp, int aid, int state)
     if (state == 0)
         return sfp;
 
-    sf = calloc(1, sizeof(ally));
+    while (*sfp) {
-    sf->faction = af;
+        sfp = &(*sfp)->next;
+    }
+
+    sf = ally_add(sfp, af);
     if (!sf->faction) {
         variant id;
         id.i = aid;
@@ -1128,9 +1131,6 @@ static ally **addally(const faction * f, ally ** sfp, int aid, int state)
     }
     sf->status = state & HELP_ALL;
 
-    while (*sfp)
-        sfp = &(*sfp)->next;
-    *sfp = sf;
     return &sf->next;
 }
 

src/kernel/unit.c

@@ -1161,10 +1161,13 @@ skill *add_skill(unit * u, skill_t id)
     ++u->skill_size;
     u->skills = realloc(u->skills, u->skill_size * sizeof(skill));
     sv = (u->skills + u->skill_size - 1);
-    sv->level = (unsigned char)0;
+    sv->level = 0;
-    sv->weeks = (unsigned char)1;
+    sv->weeks = 1;
-    sv->old = (unsigned char)0;
+    sv->old = 0;
-    sv->id = (unsigned char)id;
+    sv->id = id;
+    if (id == SK_MAGIC && u->faction) {
+        assert(max_magicians(u->faction) >= u->number);
+    }
     return sv;
 }
 

src/laws.c

@@ -83,7 +83,6 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 #include <util/umlaut.h>
 #include <util/message.h>
 #include <util/rng.h>
-#include <util/xml.h>
 
 #include <attributes/otherfaction.h>
 
@@ -1349,10 +1348,8 @@ int ally_cmd(unit * u, struct order *ord)
             return 0;
         }
         else {
-            sf = calloc(1, sizeof(ally));
+            sf = ally_add(sfp, f);
-            sf->faction = f;
             sf->status = 0;
-            addlist(sfp, sf);
         }
     }
     switch (keyword) {
@@ -4557,17 +4554,3 @@ void update_subscriptions(void)
     }
     fclose(F);
 }
-
-int init_data(const char *filename, const char *catalog)
-{
-    int l;
-    l = read_xml(filename, catalog);
-    reset_locales();
-    if (l) {
-        return l;
-    }
-    if (turn < 0) {
-        turn = first_turn;
-    }
-    return 0;
-}

src/laws.h

@@ -24,13 +24,10 @@ extern "C" {
 
   extern int writepasswd(void);
   void demographics(void);
-  void last_orders(void);
-  void find_address(void);
   void update_guards(void);
   void update_subscriptions(void);
   void deliverMail(struct faction *f, struct region *r, struct unit *u,
     const char *s, struct unit *receiver);
-  int init_data(const char *filename, const char *catalog);
 
   bool renamed_building(const struct building * b);
   int rename_building(struct unit * u, struct order * ord, struct building * b, const char *name);

src/laws.test.c

@@ -2,6 +2,7 @@
 #include <kernel/types.h>
 #include "laws.h"
 
+#include <kernel/ally.h>
 #include <kernel/config.h>
 #include <kernel/building.h>
 #include <kernel/faction.h>
@@ -57,14 +58,15 @@ static void test_rename_building(CuTest * tc)
 
     rename_building(u, NULL, b, "Villa Nagel");
     CuAssertStrEquals(tc, "Villa Nagel", b->name);
+    CuAssertTrue(tc, renamed_building(b));
 }
 
 static void test_rename_building_twice(CuTest * tc)
 {
     region *r;
-    building *b;
     unit *u;
     faction *f;
+    building *b;
     building_type *btype;
 
     test_cleanup();
@@ -85,6 +87,37 @@ static void test_rename_building_twice(CuTest * tc)
     CuAssertStrEquals(tc, "Villa Kunterbunt", b->name);
 }
 
+static void test_contact(CuTest * tc)
+{
+    region *r;
+    unit *u1, *u2, *u3;
+    building *b;
+    building_type *btype;
+    ally *al;
+
+    test_cleanup();
+    test_create_world();
+
+    btype = bt_get_or_create("castle");
+    r = findregion(0, 0);
+    b = new_building(btype, r, default_locale);
+    u1 = test_create_unit(test_create_faction(0), r);
+    u2 = test_create_unit(test_create_faction(0), r);
+    u3 = test_create_unit(test_create_faction(0), r);
+    set_level(u3, SK_PERCEPTION, 2);
+    usetsiege(u3, b);
+    b->besieged = 1;
+    CuAssertIntEquals(tc, 1, can_contact(r, u1, u2));
+
+    u_set_building(u1, b);
+    CuAssertIntEquals(tc, 0, can_contact(r, u1, u2));
+    al = ally_add(&u1->faction->allies, u2->faction);
+    al->status = HELP_ALL;
+    CuAssertIntEquals(tc, HELP_GIVE, can_contact(r, u1, u2));
+    u_set_building(u2, b);
+    CuAssertIntEquals(tc, 1, can_contact(r, u1, u2));
+}
+
 static void test_fishing_feeds_2_people(CuTest * tc)
 {
     const resource_type *rtype;
@@ -412,5 +445,6 @@ CuSuite *get_laws_suite(void)
     SUITE_ADD_TEST(suite, test_reserve_cmd);
     SUITE_ADD_TEST(suite, test_new_units);
     SUITE_ADD_TEST(suite, test_cannot_create_unit_above_limit);
+    SUITE_ADD_TEST(suite, test_contact);
     return suite;
 }

src/modules/gmcmd.c

@@ -45,9 +45,9 @@
 
 static int read_permissions(attrib * a, void *owner, struct storage *store)
 {
-  attrib *attr = NULL;
+  assert(!a);
-  a_read(store, &attr, owner);
+  a_read(store, &a, owner);
-  a_remove(&attr, a);
+  a_remove(&a, a);
   return AT_READ_OK;
 }
 

src/reports.c

@@ -1183,7 +1183,7 @@ link_seen(seen_region * seehash[], const region * first, const region * last)
         }
         r = r->next;
     }
-    sr->next = 0;
+    if (sr) sr->next = 0;
 }
 
 seen_region *find_seen(struct seen_region *seehash[], const region * r)