Merge pull request #870 from ennorehling/develop

refactoring, to go into 3.21

scripts/tests/common.lua

@@ -37,6 +37,18 @@ function setup()
     eressea.settings.set("study.random_progress", "0")
 end
 
+function test_set_faction()
+  local r = region.create(0, 0, "plain")
+  local f1 = create_faction('elf')
+  local f2 = create_faction('elf')
+  local u = one_unit(r, f1)
+  u.group = 'Experten'
+  assert_equal('Experten', u.group)
+  u.faction = f2
+  assert_equal(f2, u.faction)
+  assert_nil(u.group)
+end
+
 function test_locales()
     assert_equal(2, eressea.locale.direction("de", "Ost"))
     assert_equal(5, eressea.locale.direction("de", "westen"))

scripts/tests/e2/e2features.lua

@@ -12,6 +12,23 @@ function setup()
     eressea.settings.set("rules.peasants.growth.factor", "0")
 end
 
+function test_give_unit()
+  local r = region.create(0, 0, "plain")
+  local f1 = faction.create('elf')
+  local f2 = faction.create('elf')
+  local u1 = unit.create(f1, r)
+  local u2 = unit.create(f2, r)
+  assert_equal(f1, u1.faction)
+  assert_equal(f2, u2.faction)
+  u2.group = 'Experten'
+  assert_equal('Experten', u2.group)
+  u1:add_order("HELFE " .. itoa36(f2.id) .. " GIB")
+  u2:add_order("GIB " .. itoa36(u1.id) .. " EINHEIT")
+  process_orders()
+  assert_equal(f1, u2.faction)
+  assert_nil(u2.group)
+end
+
 function test_study_auto()
     local r = region.create(0, 0, "plain")
     local f = faction.create("human")

scripts/tests/e3/rules.lua

@@ -36,6 +36,20 @@ function teardown()
     end
 end
 
+function test_new_faction_cannot_give_unit()
+  local r = region.create(0, 0, "plain")
+  local f1 = faction.create('elf')
+  local f2 = faction.create('elf')
+  local u1 = unit.create(f1, r)
+  local u2 = unit.create(f2, r)
+  assert_equal(f1, u1.faction)
+  assert_equal(f2, u2.faction)
+  u1:add_order("HELFE " .. itoa36(f2.id) .. " GIB")
+  u2:add_order("GIB " .. itoa36(u1.id) .. " EINHEIT")
+  process_orders()
+  assert_equal(f2, u2.faction)
+end
+
 function test_calendar()
     assert_equal("winter", get_season(396))
 end

src/jsonconf.c

@@ -792,7 +792,7 @@ static void json_calendar(cJSON *json) {
             for (i = 0, jmonth = child->child; jmonth; jmonth = jmonth->next, ++i) {
                 if (jmonth->type == cJSON_Object) {
                     storms[i] = cJSON_GetObjectItem(jmonth, "storm")->valueint;
-                    month_season[i] = cJSON_GetObjectItem(jmonth, "season")->valueint;
+                    month_season[i] = (season_t) cJSON_GetObjectItem(jmonth, "season")->valueint;
                 }
                 else {
                     log_error("calendar.months[%d] is not an object: %d", i, json->type);

src/jsonconf.test.c

@@ -160,8 +160,8 @@ static void test_calendar(CuTest * tc)
     CuAssertIntEquals(tc, 99, storms[0]);
     CuAssertIntEquals(tc, 22, storms[1]);
     CuAssertPtrNotNull(tc, month_season);
-    CuAssertIntEquals(tc, 1, month_season[0]);
-    CuAssertIntEquals(tc, 2, month_season[1]);
+    CuAssertIntEquals(tc, SEASON_SPRING, month_season[0]);
+    CuAssertIntEquals(tc, SEASON_SUMMER, month_season[1]);
     cJSON_Delete(json);
     test_teardown();
 }

src/kernel/calendar.h

@@ -15,7 +15,7 @@ extern "C" {
     extern const char *seasonnames[CALENDAR_SEASONS];
 
     extern int months_per_year;
-    extern int *month_season;
+    extern season_t *month_season;
     extern int first_month;
     extern int turn;
 

src/kernel/calendar.test.c

@@ -65,13 +65,13 @@ static void test_calendar(CuTest * tc)
     test_teardown();
 }
 
-static void setup_calendar() {
+static void setup_calendar(void) {
     int i;
 
     months_per_year = 4;
     weeks_per_month = 2;
     free(month_season);
-    month_season = calloc(months_per_year, sizeof(int));
+    month_season = calloc(months_per_year, sizeof(season_t));
     for (i = 0; i != 4; ++i) {
         month_season[i] = (season_t)i;
     }

src/kernel/group.c

@@ -49,7 +49,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 static group *ghash[GMAXHASH];
 static int maxgid;
 
-group *new_group(faction * f, const char *name, int gid)
+group *create_group(faction * f, const char *name, int gid)
 {
     group **gp = &f->groups;
     int index = gid % GMAXHASH;
@@ -63,6 +63,7 @@ group *new_group(faction * f, const char *name, int gid)
     if (gid > maxgid) maxgid = gid;
     g->name = str_strdup(name);
     g->gid = gid;
+    g->f = f;
 
     g->nexthash = ghash[index];
     return ghash[index] = g;
@@ -93,11 +94,15 @@ static int read_group(variant *var, void *owner, gamedata *data)
 {
     struct storage *store = data->store;
     group *g;
+    unit * u = (unit *)owner;
     int gid;
 
     READ_INT(store, &gid);
     var->v = g = find_group(gid);
-    if (g != 0) {
+    if (g != NULL) {
+        if (g->f != u->faction) {
+            return AT_READ_FAIL;
+        }
         g->members++;
         return AT_READ_OK;
     }
@@ -184,7 +189,7 @@ group *join_group(unit * u, const char *name)
     if (name && name[0]) {
         g = find_groupbyname(u->faction->groups, name);
         if (g == NULL) {
-            g = new_group(u->faction, name, ++maxgid);
+            g = create_group(u->faction, name, ++maxgid);
             init_group(u->faction, g);
         }
     }
@@ -219,7 +224,7 @@ void read_groups(gamedata *data, faction * f)
         if (gid == 0)
             break;
         READ_STR(store, buf, sizeof(buf));
-        g = new_group(f, buf, gid);
+        g = create_group(f, buf, gid);
         read_allies(data, &g->allies);
         read_attribs(data, &g->attribs, g);
     }

src/kernel/group.h

@@ -40,7 +40,7 @@ extern "C" {
     extern void set_group(struct unit *u, struct group *g);
     extern struct group * get_group(const struct unit *u);
     extern void free_group(struct group *g);
-    struct group *new_group(struct faction * f, const char *name, int gid);
+    struct group *create_group(struct faction * f, const char *name, int gid);
 
     extern void write_groups(struct gamedata *data, const struct faction *f);
     extern void read_groups(struct gamedata *data, struct faction *f);

src/kernel/group.test.c

@@ -80,8 +80,10 @@ static void test_group_readwrite(CuTest * tc)
     mstream_init(&data.strm);
     gamedata_init(&data, &store, RELEASE_VERSION);
     f = test_create_faction(NULL);
-    new_group(f, "NW", 42);
-    g = new_group(f, "Egoisten", 43);
+    create_group(f, "NW", 42);
+    g = create_group(f, "Egoisten", 43);
+    CuAssertPtrEquals(tc, f, g->f);
+    CuAssertStrEquals(tc, "Egoisten", g->name);
     key_set(&g->attribs, 44, 44);
     ally_set(&g->allies, f, HELP_GIVE);
     write_groups(&data, f);

src/kernel/save.c

@@ -1,3 +1,4 @@
+#include "save.h"
 /*
 Copyright (c) 1998-2019, Enno Rehling <enno@eressea.de>
 Katja Zedel <katze@felidae.kn-bremen.de
@@ -1473,61 +1474,15 @@ static void fix_familiars(void (*callback)(unit *)) {
     }
 }
 
-int read_game(gamedata *data)
-{
-    int p, nread;
-    faction *f, **fp;
-    region *r;
-    unit *u;
+static void read_regions(gamedata *data) {
     storage * store = data->store;
     const struct building_type *bt_lighthouse = bt_find("lighthouse");
     const struct race *rc_spell = rc_find("spell");
-
-    if (data->version >= SAVEGAMEID_VERSION) {
-        int gameid;
-
-        READ_INT(store, &gameid);
-        if (gameid != game_id()) {
-            log_warning("game mismatch: datafile contains game %d, but config is for %d", gameid, game_id());
-        }
-    }
-    else {
-        READ_STR(store, NULL, 0);
-    }
-
-    if (data->version < FIXATKEYS_VERSION) {
-        attrib *a = NULL;
-        read_attribs(data, &a, NULL);
-        a_removeall(&a, NULL);
-    }
-
-    READ_INT(store, &turn);
-    log_debug(" - reading turn %d", turn);
-    rng_init(turn + config_get_int("game.seed", 0));
-    READ_INT(store, NULL);          /* max_unique_id = ignore */
-    READ_INT(store, &nextborder);
-
-    read_planes(data);
-    read_alliances(data);
-    READ_INT(store, &nread);
-    log_debug(" - Einzulesende Parteien: %d\n", nread);
-    fp = &factions;
-    while (*fp) {
-        fp = &(*fp)->next;
-    }
-
-    while (--nread >= 0) {
-        faction *f = read_faction(data);
-
-        *fp = f;
-        fp = &f->next;
-    }
-    *fp = 0;
-
-    /* Regionen */
+    region *r;
+    int nread;
 
     READ_INT(store, &nread);
-    assert(nread < MAXREGIONS && nread >=0);
+    assert(nread < MAXREGIONS && nread >= 0);
 
     log_debug(" - Einzulesende Regionen: %d", nread);
 
@@ -1535,13 +1490,14 @@ int read_game(gamedata *data)
         unit **up;
         building **bp;
         ship **shp;
+        int p;
 
         r = read_region(data);
 
         /* Burgen */
         READ_INT(store, &p);
         if (p > 0 && !r->land) {
-            log_debug("%s, uid=%d has %d %s", regionname(r, NULL), r->uid, p, (p==1) ? "building" : "buildings");
+            log_debug("%s, uid=%d has %d %s", regionname(r, NULL), r->uid, p, (p == 1) ? "building" : "buildings");
         }
         bp = &r->buildings;
 
@@ -1595,7 +1551,6 @@ int read_game(gamedata *data)
             }
         }
     }
-    read_borders(data);
 
     log_debug("updating area information for lighthouses.");
     for (r = regions; r; r = r->next) {
@@ -1608,8 +1563,12 @@ int read_game(gamedata *data)
             }
         }
     }
+}
+
+static void init_factions(int data_version)
+{
     log_debug("marking factions as alive.");
-    for (f = factions; f; f = f->next) {
+    for (faction *f = factions; f; f = f->next) {
         if (f->flags & FFL_NPC) {
             f->_alive = true;
             f->magiegebiet = M_GRAY;
@@ -1623,8 +1582,8 @@ int read_game(gamedata *data)
         }
         else {
             assert(f->units);
-            for (u = f->units; u; u = u->nextF) {
-                if (data->version < SPELL_LEVEL_VERSION) {
+            for (unit *u = f->units; u; u = u->nextF) {
+                if (data_version < SPELL_LEVEL_VERSION) {
                     struct sc_mage *mage = get_mage(u);
                     if (mage) {
                         faction *f = u->faction;
@@ -1641,16 +1600,76 @@ int read_game(gamedata *data)
                 }
                 if (u->number > 0) {
                     f->_alive = true;
-                    if (data->version >= SPELL_LEVEL_VERSION) {
+                    if (data_version >= SPELL_LEVEL_VERSION) {
                         break;
                     }
                 }
             }
-            if (data->version < SPELL_LEVEL_VERSION && f->spellbook) {
+            if (data_version < SPELL_LEVEL_VERSION && f->spellbook) {
                 spellbook_foreach(f->spellbook, cb_sb_maxlevel, f);
             }
         }
     }
+}
+
+static void read_factions(gamedata * data)
+{
+    storage * store = data->store;
+    int nread;
+    faction **fp;
+    READ_INT(store, &nread);
+    log_debug(" - Einzulesende Parteien: %d\n", nread);
+    fp = &factions;
+    while (*fp) {
+        fp = &(*fp)->next;
+    }
+
+    while (--nread >= 0) {
+        faction *f = read_faction(data);
+
+        *fp = f;
+        fp = &f->next;
+    }
+}
+
+int read_game(gamedata *data)
+{
+    storage * store = data->store;
+
+    if (data->version >= SAVEGAMEID_VERSION) {
+        int gameid;
+
+        READ_INT(store, &gameid);
+        if (gameid != game_id()) {
+            log_warning("game mismatch: datafile contains game %d, but config is for %d", gameid, game_id());
+        }
+    }
+    else {
+        READ_STR(store, NULL, 0);
+    }
+
+    if (data->version < FIXATKEYS_VERSION) {
+        attrib *a = NULL;
+        read_attribs(data, &a, NULL);
+        a_removeall(&a, NULL);
+    }
+
+    READ_INT(store, &turn);
+    log_debug(" - reading turn %d", turn);
+    rng_init(turn + config_get_int("game.seed", 0));
+    READ_INT(store, NULL);          /* max_unique_id = ignore */
+    READ_INT(store, &nextborder);
+
+    read_planes(data);
+    read_alliances(data);
+
+    read_factions(data);
+
+    /* Regionen */
+
+    read_regions(data);
+    read_borders(data);
+    init_factions(data->version);
     if (data->version < FIX_CLONES_VERSION) {
         fix_clones();
     }

src/kernel/unit.c

@@ -913,7 +913,7 @@ void u_setfaction(unit * u, faction * f)
             --u->faction->num_units;
             u->faction->num_people -= u->number;
         }
-        join_group(u, NULL);
+        set_group(u, NULL);
         free_orders(&u->orders);
         set_order(&u->thisorder, NULL);