make destroyfaction remove the faction from the list (slightly scary change)

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,17 +322,16 @@ 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);
 
     if (f->spellbook) {
@@ -389,32 +388,43 @@ void destroyfaction(faction * f)
             u = u->nextF;
         }
     }
-    f->alive = 0;
+    f->alive = false;
     /* 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);
+                }
+                else {
+                    sfp = &(*sfp)->next;
                 }
-                sf = sfn;
             }
         }
     }
 
+    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. */
     if (rule_stealth_other()) {
@@ -633,60 +643,18 @@ 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) && !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
-                        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
+        else {
             fp = &(*fp)->next;
+        }
     }
 }
 

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,7 @@ 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);
 
     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,7 +75,7 @@ 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;
     fno = f->no;
     remove_empty_factions();

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;) {
-        if (!is_monsters(f)) {
+        faction *f = *fp;
+        if (f->alive && !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,21 +4264,22 @@ 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;
+        if (f->alive) {
-        while (*c && pwok) {
+            const char *c = f->passw;
-            if (!isalnum((unsigned char)*c))
+            while (*c && pwok) {
-                pwok = false;
+                if (!isalnum((unsigned char)*c))
-            c++;
+                    pwok = false;
+                c++;
+            }
+            if (!pwok) {
+                free(f->passw);
+                f->passw = _strdup(itoa36(rng_int()));
+                ADDMSG(&f->msgs, msg_message("illegal_password", "newpass", f->passw));
+            }
         }
-        if (!pwok) {
-            free(f->passw);
-            f->passw = _strdup(itoa36(rng_int()));
-            ADDMSG(&f->msgs, msg_message("illegal_password", "newpass", f->passw));
-        }
-        f = f->next;
     }
     return 0;
 }