Merge pull request #452 from ennorehling/issue-451-destroyfaction

smarter handling of faction.alive

src/bind_faction.c

@@ -343,8 +343,14 @@ static int tolua_faction_get_origin(lua_State * L)
 
 static int tolua_faction_destroy(lua_State * L)
 {
-    faction *f = (faction *)tolua_tousertype(L, 1, 0);
+    faction **fp, *f = (faction *)tolua_tousertype(L, 1, 0);
-    destroyfaction(f);
+    // TODO: this loop is slow af, but what can we do?
+    for (fp = &factions; *fp; fp = &(*fp)->next) {
+        if (*fp == f) {
+            destroyfaction(fp);
+            break;
+        }
+    }
     return 0;
 }
 

src/kernel/alliance.c

@@ -424,13 +424,14 @@ void alliancevictory(void)
     }
     while (al != NULL) {
         if (!fval(al, FFL_MARK)) {
-            int qi;
+            faction **fp;
-            quicklist *flist = al->members;
+            for (fp = &factions; *fp; ) {
-            for (qi = 0; flist; ql_advance(&flist, &qi, 1)) {
+                faction *f = *fp;
-                faction *f = (faction *)ql_get(flist, qi);
                 if (f->alliance == al) {
                     ADDMSG(&f->msgs, msg_message("alliance::lost", "alliance", al));
-                    destroyfaction(f);
+                    destroyfaction(fp);
+                } else {
+                    fp = &f->next;
                 }
             }
         }

src/kernel/faction.c

@@ -252,7 +252,7 @@ faction *addfaction(const char *email, const char *password,
 
     f->alliance_joindate = turn;
     f->lastorders = turn;
-    f->alive = 1;
+    f->_alive = true;
     f->age = 0;
     f->race = frace;
     f->magiegebiet = 0;
@@ -322,18 +322,18 @@ variant read_faction_reference(struct storage * store)
 
 void write_faction_reference(const faction * f, struct storage *store)
 {
-    WRITE_INT(store, (f && f->alive) ? f->no : 0);
+    WRITE_INT(store, (f && f->_alive) ? f->no : 0);
 }
 
-void destroyfaction(faction * f)
+void destroyfaction(faction ** fp)
 {
+    faction * f = *fp;
     unit *u = f->units;
     faction *ff;
 
-    if (!f->alive) {
+    *fp = f->next;
-        return;
-    }
     fset(f, FFL_QUIT);
+    f->_alive = false;
 
     if (f->spellbook) {
         spellbook_clear(f->spellbook);
@@ -389,31 +389,41 @@ void destroyfaction(faction * f)
             u = u->nextF;
         }
     }
-    f->alive = 0;
     /* no way!  f->units = NULL; */
     handle_event(f->attribs, "destroy", f);
     for (ff = factions; ff; ff = ff->next) {
         group *g;
-        ally *sf, *sfn;
+        ally *sf, **sfp;
 
-        /* Alle HELFE für die Partei löschen */
+        for (sfp = &ff->allies; *sfp;) {
-        for (sf = ff->allies; sf; sf = sf->next) {
+            sf = *sfp;
-            if (sf->faction == f) {
+            if (sf->faction == f || sf->faction == NULL) {
-                removelist(&ff->allies, sf);
+                *sfp = sf->next;
-                break;
+                free(sf);
             }
+            else
+                sfp = &(*sfp)->next;
         }
         for (g = ff->groups; g; g = g->next) {
-            for (sf = g->allies; sf;) {
+            for (sfp = &g->allies; *sfp; ) {
-                sfn = sf->next;
+                sf = *sfp;
-                if (sf->faction == f) {
+                if (sf->faction == f || sf->faction == NULL) {
-                    removelist(&g->allies, sf);
+                    *sfp = sf->next;
-                    break;
+                    free(sf);
                 }
-                sf = sfn;
+                else {
+                    sfp = &(*sfp)->next;
                 }
             }
         }
+    }
+
+    if (f->alliance && f->alliance->_leader == f) {
+        setalliance(f, 0);
+    }
+
+    funhash(f);
+    free_faction(f);
 
     /* units of other factions that were disguised as this faction
      * have their disguise replaced by ordinary faction hiding. */
@@ -633,62 +643,25 @@ int skill_limit(faction * f, skill_t sk)
 
 void remove_empty_factions(void)
 {
-    faction **fp, *f3;
+    faction **fp;
 
     for (fp = &factions; *fp;) {
         faction *f = *fp;
-        /* monster (0) werden nicht entfernt. alive kann beim readgame
+
-        * () auf 0 gesetzt werden, wenn monsters keine einheiten mehr
+        if (!(f->_alive && f->units!=NULL) && !fval(f, FFL_NOIDLEOUT)) {
-        * haben. */
-        if ((f->units == NULL || f->alive == 0) && !fval(f, FFL_NOIDLEOUT)) {
-            ursprung *ur = f->ursprung;
-            while (ur && ur->id != 0)
-                ur = ur->next;
             log_debug("dead: %s", factionname(f));
-
+            destroyfaction(fp);
-            /* Einfach in eine Datei schreiben und später vermailen */
-
-            for (f3 = factions; f3; f3 = f3->next) {
-                ally *sf;
-                group *g;
-                ally **sfp = &f3->allies;
-                while (*sfp) {
-                    sf = *sfp;
-                    if (sf->faction == f || sf->faction == NULL) {
-                        *sfp = sf->next;
-                        free(sf);
         }
-                    else
+        else {
-                        sfp = &(*sfp)->next;
-                }
-                for (g = f3->groups; g; g = g->next) {
-                    sfp = &g->allies;
-                    while (*sfp) {
-                        sf = *sfp;
-                        if (sf->faction == f || sf->faction == NULL) {
-                            *sfp = sf->next;
-                            free(sf);
-                        }
-                        else
-                            sfp = &(*sfp)->next;
-                    }
-                }
-            }
-
-            *fp = f->next;
-            funhash(f);
-            free_faction(f);
-            if (f->alliance && f->alliance->_leader == f) {
-                setalliance(f, 0);
-            }
-            destroyfaction(f); // TODO: there was a free() here,
-            // are we duplicating efforts here that also happen
-            // in destroyfaction?
-        }
-        else
             fp = &(*fp)->next;
         }
     }
+}
+
+bool faction_alive(faction *f) {
+    assert(f);
+    return f->_alive || (f->flags&FFL_NPC);
+}
 
 void faction_getorigin(const faction * f, int id, int *x, int *y)
 {

src/kernel/faction.h

@@ -103,7 +103,7 @@ extern "C" {
         struct item *items;         /* items this faction can claim */
         struct seen_region **seen;
         struct quicklist *seen_factions;
-        bool alive;              /* enno: sollte ein flag werden */
+        bool _alive;              /* enno: sollte ein flag werden */
     } faction;
 
     extern struct faction *factions;
@@ -121,7 +121,9 @@ extern "C" {
     struct faction *addfaction(const char *email, const char *password,
         const struct race *frace, const struct locale *loc, int subscription);
     bool checkpasswd(const faction * f, const char *passwd);
-    void destroyfaction(faction * f);
+    void destroyfaction(faction ** f);
+
+    bool faction_alive(struct faction *f);
 
     void set_alliance(struct faction *a, struct faction *b, int status);
     int get_alliance(const struct faction *a, const struct faction *b);

src/kernel/faction.test.c

@@ -75,8 +75,8 @@ static void test_remove_dead_factions(CuTest *tc) {
     remove_empty_factions();
     CuAssertPtrEquals(tc, f, findfaction(f->no));
     CuAssertPtrNotNull(tc, get_monsters());
-    fm->alive = 0;
+    fm->units = 0;
-    f->alive = 0;
+    f->_alive = false;
     fno = f->no;
     remove_empty_factions();
     CuAssertPtrEquals(tc, 0, findfaction(fno));
@@ -107,7 +107,7 @@ static void test_addfaction(CuTest *tc) {
     CuAssertIntEquals(tc, 1234, f->subscription);
     CuAssertIntEquals(tc, 0, f->flags);
     CuAssertIntEquals(tc, 0, f->age);
-    CuAssertIntEquals(tc, 1, f->alive);
+    CuAssertIntEquals(tc, true, faction_alive(f));
     CuAssertIntEquals(tc, M_GRAY, f->magiegebiet);
     CuAssertIntEquals(tc, turn, f->lastorders);
     CuAssertPtrEquals(tc, f, findfaction(f->no));

src/kernel/save.c

@@ -1668,7 +1668,7 @@ int readgame(const char *filename, bool backup)
     log_debug("marking factions as alive.\n");
     for (f = factions; f; f = f->next) {
         if (f->flags & FFL_NPC) {
-            f->alive = 1;
+            f->_alive = true;
             f->magiegebiet = M_GRAY;
             if (f->no == 0) {
                 int no = 666;
@@ -1698,7 +1698,7 @@ int readgame(const char *filename, bool backup)
                     }
                 }
                 if (u->number > 0) {
-                    f->alive = true;
+                    f->_alive = true;
                     if (global.data_version >= SPELL_LEVEL_VERSION) {
                         break;
                     }

src/kernel/unit.c

@@ -1463,7 +1463,7 @@ unit *create_unit(region * r, faction * f, int number, const struct race *urace,
 
     assert(urace);
     if (f) {
-        assert(f->alive);
+        assert(faction_alive(f));
         u_setfaction(u, f);
 
         if (f->locale) {
@@ -1828,7 +1828,7 @@ void remove_empty_units_in_region(region * r)
 
         if (u->number) {
             faction *f = u->faction;
-            if (f == NULL || !f->alive) {
+            if (f == NULL || !faction_alive(f)) {
                 set_number(u, 0);
             }
         }

src/kernel/unit.test.c

@@ -86,7 +86,7 @@ static void test_remove_units_with_dead_faction(CuTest *tc) {
 
     u = test_create_unit(test_create_faction(test_create_race("human")), findregion(0, 0));
     uid = u->no;
-    u->faction->alive = false;
+    u->faction->_alive = false;
     remove_empty_units_in_region(u->region);
     CuAssertPtrEquals(tc, 0, findunit(uid));
     CuAssertIntEquals(tc, 0, u->number);

src/laws.c

@@ -1220,42 +1220,45 @@ static void nmr_death(faction * f)
 
 static void remove_idle_players(void)
 {
-    faction *f;
+    faction **fp;
 
     log_info(" - beseitige Spieler, die sich zu lange nicht mehr gemeldet haben...");
 
-    for (f = factions; f; f = f->next) {
+    for (fp = &factions; *fp;) {
+        faction *f = *fp;
         if (fval(f, FFL_NOIDLEOUT)) {
             f->lastorders = turn;
         }
         if (NMRTimeout() > 0 && turn - f->lastorders >= NMRTimeout()) {
             nmr_death(f);
-            destroyfaction(f);
+            destroyfaction(fp);
-            continue;
+        } else if (turn != f->lastorders) {
-        }
-        if (turn != f->lastorders) {
             char info[256];
             sprintf(info, "%d Einheiten, %d Personen, %d Silber",
                 f->no_units, f->num_total, f->money);
         }
+        fp = &f->next;
     }
     log_info(" - beseitige Spieler, die sich nach der Anmeldung nicht gemeldet haben...");
 
     age = calloc(_max(4, turn + 1), sizeof(int));
-    for (f = factions; f; f = f->next)
+    for (fp = &factions; *fp;) {
+        faction *f = *fp;
         if (!is_monsters(f)) {
             if (RemoveNMRNewbie() && !fval(f, FFL_NOIDLEOUT)) {
                 if (f->age >= 0 && f->age <= turn)
                     ++age[f->age];
                 if (f->age == 2 || f->age == 3) {
                     if (f->lastorders == turn - 2) {
-                        destroyfaction(f);
                         ++dropouts[f->age - 2];
+                        destroyfaction(fp);
                         continue;
                     }
                 }
             }
         }
+        fp = &f->next;
+    }
 }
 
 void quit(void)
@@ -1264,7 +1267,7 @@ void quit(void)
     while (*fptr) {
         faction *f = *fptr;
         if (f->flags & FFL_QUIT) {
-            destroyfaction(f);
+            destroyfaction(fptr);
         }
         else {
             ++f->age;
@@ -1272,8 +1275,6 @@ void quit(void)
                 ADDMSG(&f->msgs, msg_message("newbieimmunity", "turns",
                     NewbieImmunity() - f->age - 1));
             }
-        }
-        if (*fptr == f) {
             fptr = &f->next;
         }
     }
@@ -4263,8 +4264,8 @@ static void maintain_buildings_1(region * r)
  */
 static int warn_password(void)
 {
-    faction *f = factions;
+    faction *f;
-    while (f) {
+    for (f = factions; f; f = f->next) {
         bool pwok = true;
         const char *c = f->passw;
         while (*c && pwok) {
@@ -4277,7 +4278,6 @@ static int warn_password(void)
             f->passw = _strdup(itoa36(rng_int()));
             ADDMSG(&f->msgs, msg_message("illegal_password", "newpass", f->passw));
         }
-        f = f->next;
     }
     return 0;
 }

src/summary.c

@@ -89,7 +89,7 @@ int update_nmrs(void)
         if (fval(f, FFL_ISNEW)) {
             ++newplayers;
         }
-        else if (!fval(f, FFL_NOIDLEOUT) && f->alive) {
+        else if (!fval(f, FFL_NOIDLEOUT)) {
             int nmr = turn - f->lastorders + 1;
             if (nmr < 0 || nmr > NMRTimeout()) {
                 log_error("faction %s has %d NMRS\n", factionid(f), nmr);
@@ -370,7 +370,7 @@ summary *make_summary(void)
         f->nregions = 0;
         f->num_total = 0;
         f->money = 0;
-        if (f->alive && f->units) {
+        if (f->units) {
             s->factions++;
             /* Problem mit Monsterpartei ... */
             if (!is_monsters(f)) {