From d568e7949033ba87da920d5b136dbf420ba444b5 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Sat, 5 May 2018 07:38:11 +0200
Subject: [PATCH] spellbooks contain spellref, not spell *. for
 order-independent load from XML.

---
 src/attributes/seenspell.c  | 13 ++++++------
 src/attributes/seenspell.h  |  2 +-
 src/battle.c                |  2 +-
 src/bindings.c              |  2 +-
 src/creport.c               |  7 ++++---
 src/kernel/spell.c          | 41 ++++++++++++++++++++++++++++++-------
 src/kernel/spell.h          |  6 +++++-
 src/kernel/spell.test.c     |  3 ++-
 src/kernel/spellbook.c      | 17 ++++++++++++---
 src/kernel/spellbook.h      |  6 ++++--
 src/kernel/spellbook.test.c |  2 +-
 src/laws.c                  |  5 +++--
 src/magic.c                 | 25 ++++++++++++----------
 src/magic.test.c            |  2 +-
 src/report.c                |  4 ++--
 src/reports.c               |  3 ++-
 src/xmlreader.c             | 14 +++++--------
 17 files changed, 101 insertions(+), 53 deletions(-)

diff --git a/src/attributes/seenspell.c b/src/attributes/seenspell.c
index c2b9c7c75..82bdf22ca 100644
--- a/src/attributes/seenspell.c
+++ b/src/attributes/seenspell.c
@@ -107,14 +107,14 @@ static int cmp_spell(const void *a, const void *b) {
     return strcmp(spa->sname, spb->sname);
 }
 
-static bool set_seen(attrib **alist, struct spell *sp) {
+static bool set_seen(attrib **alist, const struct spell *sp) {
     attrib *a = a_find(*alist, &at_seenspells);
     selist **sl;
     if (!a) {
         a = a_add(alist, a_new(&at_seenspells));
     }
     sl = (selist **)&a->data.v;
-    return selist_set_insert(sl, sp, cmp_spell);
+    return selist_set_insert(sl, (void *)sp, cmp_spell);
 }
 
 static void upgrade_seenspell(attrib **alist, attrib *abegin) {
@@ -123,7 +123,7 @@ static void upgrade_seenspell(attrib **alist, attrib *abegin) {
     ak = a_find(*alist, &at_seenspells);
     if (ak) alist = &ak;
     for (a = abegin; a && a->type == abegin->type; a = a->next) {
-        set_seen(alist, (struct spell *)a->data.v);
+        set_seen(alist, (const struct spell *)a->data.v);
     }
 }
 
@@ -156,11 +156,12 @@ attrib_type at_reportspell = {
     "reportspell", NULL
 };
 
-void show_spell(faction *f, const spellbook_entry *sbe)
+void show_spell(faction *f, spellbook_entry *sbe)
 {
-    if (!already_seen(f, sbe->sp)) {
+    const spell *sp = spellref_get(&sbe->spref);
+    if (!already_seen(f, sp)) {
         /* mark the spell as seen by this faction: */
-        if (set_seen(&f->attribs, sbe->sp)) {
+        if (set_seen(&f->attribs, sp)) {
             /* add the spell to the report: */
             attrib * a = a_new(&at_reportspell);
             a->data.v = (void *)sbe;
diff --git a/src/attributes/seenspell.h b/src/attributes/seenspell.h
index cdfb36e48..a8bb35c4d 100644
--- a/src/attributes/seenspell.h
+++ b/src/attributes/seenspell.h
@@ -6,7 +6,7 @@ struct spellbook_entry;
 struct faction;
 struct spell;
 
-void show_spell(struct faction * f, const struct spellbook_entry *sbe);
+void show_spell(struct faction * f, struct spellbook_entry *sbe);
 void reset_seen_spells(struct faction * f, const struct spell *sp);
 
 extern struct attrib_type at_reportspell;
diff --git a/src/battle.c b/src/battle.c
index 01b865d61..1e6692a68 100644
--- a/src/battle.c
+++ b/src/battle.c
@@ -1870,7 +1870,7 @@ static void do_extra_spell(troop at, const att * a)
     const spell *sp = spellref_get(a->data.sp);
 
     if (!sp) {
-        log_error("no such spell: '%s'", a->data.sp->name);
+        log_error("no such spell: '%s'", a->data.sp->_name);
     }
     else {
         assert(a->level > 0);
diff --git a/src/bindings.c b/src/bindings.c
index 5797f7cb5..f9f51ded3 100755
--- a/src/bindings.c
+++ b/src/bindings.c
@@ -860,7 +860,7 @@ static int tolua_get_spell_name(lua_State * L)
 static int tolua_get_spell_entry_name(lua_State * L)
 {
     spellbook_entry *self = (spellbook_entry*)tolua_tousertype(L, 1, 0);
-    lua_pushstring(L, self->sp->sname);
+    lua_pushstring(L, spellref_name(&self->spref));
     return 1;
 }
 
diff --git a/src/creport.c b/src/creport.c
index bba824538..4fc1cdd99 100644
--- a/src/creport.c
+++ b/src/creport.c
@@ -758,7 +758,7 @@ static void cr_output_spells(stream *out, const unit * u, int maxlevel)
         for (ql = book->spells, qi = 0; ql; selist_advance(&ql, &qi, 1)) {
             spellbook_entry * sbe = (spellbook_entry *)selist_get(ql, qi);
             if (sbe->level <= maxlevel) {
-                spell * sp = sbe->sp;
+                const spell *sp = spellref_get(&sbe->spref);
                 const char *name = translate(mkname("spell", sp->sname), spell_name(sp, f->locale));
                 if (!header) {
                     stream_printf(out, "SPRUECHE\n");
@@ -1083,7 +1083,7 @@ static void cr_find_address(FILE * F, const faction * uf, selist * addresses)
 
 /* = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = =  */
 
-static void cr_reportspell(FILE * F, spell * sp, int level, const struct locale *lang)
+static void cr_reportspell(FILE * F, const spell * sp, int level, const struct locale *lang)
 {
     int k;
     const char *name =
@@ -1673,7 +1673,8 @@ report_computer(const char *filename, report_context * ctx, const char *bom)
     a = a_find(f->attribs, &at_reportspell);
     while (a && a->type == &at_reportspell) {
         spellbook_entry *sbe = (spellbook_entry *)a->data.v;
-        cr_reportspell(F, sbe->sp, sbe->level, f->locale);
+        const spell *sp = spellref_get(&sbe->spref);
+        cr_reportspell(F, sp, sbe->level, f->locale);
         a = a->next;
     }
     for (a = a_find(f->attribs, &at_showitem); a && a->type == &at_showitem;
diff --git a/src/kernel/spell.c b/src/kernel/spell.c
index 785f270b3..41e56babd 100644
--- a/src/kernel/spell.c
+++ b/src/kernel/spell.c
@@ -180,19 +180,37 @@ struct spellref *spellref_create(spell *sp, const char *name)
 
     if (sp) {
         spref->sp = sp;
-        spref->name = str_strdup(sp->sname);
+        spref->_name = str_strdup(sp->sname);
     }
     else if (name) {
-        spref->name = str_strdup(name);
+        spref->_name = str_strdup(name);
         spref->sp = NULL;
     }
     return spref;
 }
 
+void spellref_init(spellref *spref, spell *sp, const char *name)
+{
+    if (sp) {
+        spref->sp = sp;
+        spref->_name = str_strdup(sp->sname);
+    }
+    else if (name) {
+        spref->_name = str_strdup(name);
+        spref->sp = NULL;
+    }
+}
+
+void spellref_done(spellref *spref) {
+    if (spref) {
+        free(spref->_name);
+    }
+}
+
 void spellref_free(spellref *spref)
 {
     if (spref) {
-        free(spref->name);
+        spellref_done(spref);
         free(spref);
     }
 }
@@ -200,12 +218,21 @@ void spellref_free(spellref *spref)
 struct spell *spellref_get(struct spellref *spref)
 {
     if (!spref->sp) {
-        assert(spref->name);
-        spref->sp = find_spell(spref->name);
+        assert(spref->_name);
+        spref->sp = find_spell(spref->_name);
         if (spref->sp) {
-            free(spref->name);
-            spref->name = NULL;
+            free(spref->_name);
+            spref->_name = NULL;
         }
     }
     return spref->sp;
 }
+
+const char *spellref_name(const struct spellref *spref)
+{
+    if (spref->_name) {
+        return spref->_name;
+    }
+    assert(spref->sp);
+    return spref->sp->sname;
+}
diff --git a/src/kernel/spell.h b/src/kernel/spell.h
index 17a565fa6..8b155f89b 100644
--- a/src/kernel/spell.h
+++ b/src/kernel/spell.h
@@ -43,7 +43,7 @@ extern "C" {
     } spell;
 
     typedef struct spellref {
-        char * name;
+        char * _name;
         struct spell *sp;
     } spellref;
 
@@ -56,6 +56,10 @@ extern "C" {
     struct spellref *spellref_create(struct spell *sp, const char *name);
     void spellref_free(struct spellref *spref);
     struct spell *spellref_get(struct spellref *spref);
+    const char *spellref_name(const struct spellref *spref);
+
+    void spellref_init(struct spellref *spref, struct spell *sp, const char *name);
+    void spellref_done(struct spellref *spref);
 
     int sp_antimagiczone(struct castorder *co);
 
diff --git a/src/kernel/spell.test.c b/src/kernel/spell.test.c
index 5502069d1..622780e20 100644
--- a/src/kernel/spell.test.c
+++ b/src/kernel/spell.test.c
@@ -58,8 +58,9 @@ static void test_spellref(CuTest *tc)
     ref = spellref_create(NULL, "hodor");
     CuAssertPtrNotNull(tc, ref);
     CuAssertPtrEquals(tc, NULL, ref->sp);
-    CuAssertStrEquals(tc, "hodor", ref->name);
+    CuAssertStrEquals(tc, "hodor", ref->_name);
     CuAssertPtrEquals(tc, NULL, spellref_get(ref));
+    CuAssertStrEquals(tc, "hodor", spellref_name(ref));
     sp = create_spell("hodor");
     CuAssertPtrNotNull(tc, sp);
     CuAssertPtrEquals(tc, sp, spellref_get(ref));
diff --git a/src/kernel/spellbook.c b/src/kernel/spellbook.c
index ae5387f11..868f9757c 100644
--- a/src/kernel/spellbook.c
+++ b/src/kernel/spellbook.c
@@ -67,13 +67,23 @@ void write_spellbook(const struct spellbook *book, struct storage *store)
     if (book) {
         for (ql = book->spells, qi = 0; ql; selist_advance(&ql, &qi, 1)) {
             spellbook_entry *sbe = (spellbook_entry *)selist_get(ql, qi);
-            WRITE_TOK(store, sbe->sp->sname);
+            WRITE_TOK(store, spellref_name(&sbe->spref));
             WRITE_INT(store, sbe->level);
         }
     }
     WRITE_TOK(store, "end");
 }
 
+void spellbook_addref(spellbook *sb, const char *name, int level) {
+    spellbook_entry * sbe;
+
+    assert(sb && name && level > 0);
+    sbe = (spellbook_entry *)malloc(sizeof(spellbook_entry));
+    spellref_init(&sbe->spref, NULL, name);
+    sbe->level = level;
+    selist_push(&sb->spells, sbe);
+}
+
 void spellbook_add(spellbook *sb, spell *sp, int level)
 {
     spellbook_entry * sbe;
@@ -85,7 +95,7 @@ void spellbook_add(spellbook *sb, spell *sp, int level)
     }
 #endif  
     sbe = (spellbook_entry *)malloc(sizeof(spellbook_entry));
-    sbe->sp = sp;
+    spellref_init(&sbe->spref, sp, NULL);
     sbe->level = level;
     selist_push(&sb->spells, sbe);
 }
@@ -98,6 +108,7 @@ void spellbook_clear(spellbook *sb)
     assert(sb);
     for (qi = 0, ql = sb->spells; ql; selist_advance(&ql, &qi, 1)) {
         spellbook_entry *sbe = (spellbook_entry *)selist_get(ql, qi);
+        spellref_done(&sbe->spref);
         free(sbe);
     }
     selist_free(sb->spells);
@@ -127,7 +138,7 @@ spellbook_entry * spellbook_get(spellbook *sb, const struct spell *sp)
 
         for (qi = 0, ql = sb->spells; ql; selist_advance(&ql, &qi, 1)) {
             spellbook_entry *sbe = (spellbook_entry *)selist_get(ql, qi);
-            if (sbe->sp==sp) {
+            if (spellref_get(&sbe->spref) == sp) {
                 return sbe;
             }
         }
diff --git a/src/kernel/spellbook.h b/src/kernel/spellbook.h
index fb3438a8b..3894f9b44 100644
--- a/src/kernel/spellbook.h
+++ b/src/kernel/spellbook.h
@@ -19,17 +19,18 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 #ifndef H_KRNL_SPELLBOOK_H
 #define H_KRNL_SPELLBOOK_H
 
+#include "spell.h"
+
 #ifdef __cplusplus
 extern "C" {
 #endif
 
-    struct spell;
     struct storage;
     struct gamedata;
     struct selist;
 
     typedef struct spellbook_entry {
-        struct spell *sp;
+        spellref spref;
         int level;
     } spellbook_entry;
 
@@ -45,6 +46,7 @@ extern "C" {
     void write_spellbook(const struct spellbook *book, struct storage *store);
 
     void spellbook_add(spellbook *sbp, struct spell *sp, int level);
+    void spellbook_addref(spellbook *sb, const char *name, int level);
     int spellbook_foreach(spellbook *sb, int(*callback)(spellbook_entry *, void *), void * data);
     void spellbook_clear(spellbook *sb);
     spellbook_entry * spellbook_get(spellbook *sb, const struct spell *sp);
diff --git a/src/kernel/spellbook.test.c b/src/kernel/spellbook.test.c
index 37f20cced..22ea7c491 100644
--- a/src/kernel/spellbook.test.c
+++ b/src/kernel/spellbook.test.c
@@ -40,7 +40,7 @@ void test_named_spellbooks(CuTest * tc)
     sbe = spellbook_get(sb, sp);
     CuAssertPtrNotNull(tc, sbe);
     CuAssertIntEquals(tc, 1, sbe->level);
-    CuAssertPtrEquals(tc, sp, sbe->sp);
+    CuAssertPtrEquals(tc, sp, spellref_get(&sbe->spref));
 
     spellbook_foreach(sb, count_spell_cb, &counter);
     CuAssertIntEquals(tc, 1, counter);
diff --git a/src/laws.c b/src/laws.c
index 1887d8657..b24fa5b3b 100644
--- a/src/laws.c
+++ b/src/laws.c
@@ -3298,8 +3298,9 @@ static void copy_spells(const spellbook * src, spellbook * dst, int maxlevel)
         for (qi = 0, ql = src->spells; ql; selist_advance(&ql, &qi, 1)) {
             spellbook_entry * sbe = (spellbook_entry *)selist_get(ql, qi);
             if (sbe->level <= maxlevel) {
-                if (!spellbook_get(dst, sbe->sp)) {
-                    spellbook_add(dst, sbe->sp, sbe->level);
+                spell *sp = spellref_get(&sbe->spref);
+                if (!spellbook_get(dst, sp)) {
+                    spellbook_add(dst, sp, sbe->level);
                 }
             }
         }
diff --git a/src/magic.c b/src/magic.c
index 1865b4f9e..a00a29527 100644
--- a/src/magic.c
+++ b/src/magic.c
@@ -375,13 +375,16 @@ void pick_random_spells(faction * f, int level, spellbook * book, int num_spells
                     sbe = 0;
                 }
                 else {
-                    if (f->spellbook && spellbook_get(f->spellbook, sbe->sp)) {
-                        /* already have this spell, remove it from the list of candidates */
-                        commonspells[spellno] = commonspells[--numspells];
-                        if (maxspell > numspells) {
-                            maxspell = numspells;
+                    if (f->spellbook) {
+                        const spell *sp = spellref_get(&sbe->spref);
+                        if (sp && spellbook_get(f->spellbook, sp)) {
+                            /* already have this spell, remove it from the list of candidates */
+                            commonspells[spellno] = commonspells[--numspells];
+                            if (maxspell > numspells) {
+                                maxspell = numspells;
+                            }
+                            sbe = 0;
                         }
-                        sbe = 0;
                     }
                 }
             }
@@ -390,7 +393,7 @@ void pick_random_spells(faction * f, int level, spellbook * book, int num_spells
                 if (!f->spellbook) {
                     f->spellbook = create_spellbook(0);
                 }
-                spellbook_add(f->spellbook, sbe->sp, sbe->level);
+                spellbook_add(f->spellbook, spellref_get(&sbe->spref), sbe->level);
                 commonspells[spellno] = commonspells[--numspells];
             }
         }
@@ -2840,14 +2843,14 @@ static void select_spellbook(void **tokens, spellbook *sb, const struct locale *
 
     for (qi = 0, ql = sb->spells; ql; selist_advance(&ql, &qi, 1)) {
         spellbook_entry *sbe = (spellbook_entry *)selist_get(ql, qi);
-
-        const char *n = spell_name(sbe->sp, lang);
+        const spell *sp = spellref_get(&sbe->spref);
+        const char *n = spell_name(sp, lang);
         if (!n) {
-            log_error("no translation in locale %s for spell %s\n", locale_name(lang), sbe->sp->sname);
+            log_error("no translation in locale %s for spell %s\n", locale_name(lang), sp->sname);
         }
         else {
             variant token;
-            token.v = sbe->sp;
+            token.v = (void *)sp;
             addtoken((struct tnode **)tokens, n, token);
         }
     }
diff --git a/src/magic.test.c b/src/magic.test.c
index e68f3ca21..5533d93ec 100644
--- a/src/magic.test.c
+++ b/src/magic.test.c
@@ -86,7 +86,7 @@ static void test_spellbooks(CuTest * tc)
     CuAssertPtrNotNull(tc, sp);
     entry = spellbook_get(herp, sp);
     CuAssertPtrNotNull(tc, entry);
-    CuAssertPtrEquals(tc, sp, entry->sp);
+    CuAssertPtrEquals(tc, sp, spellref_get(&entry->spref));
 
     test_teardown();
     test_setup();
diff --git a/src/report.c b/src/report.c
index adea79257..d894f1566 100644
--- a/src/report.c
+++ b/src/report.c
@@ -249,7 +249,7 @@ void nr_spell_syntax(struct stream *out, spellbook_entry * sbe, const struct loc
     char buf[4096];
     char *bufp = buf;
     size_t size = sizeof(buf) - 1;
-    const spell * sp = sbe->sp;
+    const spell *sp = spellref_get(&sbe->spref);
     const char *params = sp->parameter;
 
     if (sp->sptyp & ISCOMBATSPELL) {
@@ -439,7 +439,7 @@ void nr_spell(struct stream *out, spellbook_entry * sbe, const struct locale *la
     char buf[4096];
     char *startp, *bufp = buf;
     size_t size = sizeof(buf) - 1;
-    const spell * sp = sbe->sp;
+    const spell *sp = spellref_get(&sbe->spref);
 
     newline(out);
     centre(out, spell_name(sp, lang), true);
diff --git a/src/reports.c b/src/reports.c
index f5b7f5531..da1c88bdc 100644
--- a/src/reports.c
+++ b/src/reports.c
@@ -870,6 +870,7 @@ bufunit(const faction * f, const unit * u, seen_mode mode, char *buf,
 
             for (header = 0, qi = 0; ql; selist_advance(&ql, &qi, 1)) {
                 spellbook_entry * sbe = (spellbook_entry *)selist_get(ql, qi);
+                const spell *sp = spellref_get(&sbe->spref);
                 if (sbe->level <= maxlevel) {
                     if (!header) {
                         n = snprintf(bufp, size, ", %s: ", LOC(lang, "nr_spells"));
@@ -882,7 +883,7 @@ bufunit(const faction * f, const unit * u, seen_mode mode, char *buf,
                         WARN_STATIC_BUFFER();
                     }
                     /* TODO: no need to deref the spellref here (spref->name is good) */
-                    bufp = STRLCPY(bufp, spell_name(sbe->sp, lang), size);
+                    bufp = STRLCPY(bufp, spell_name(sp, lang), size);
                 }
             }
 
diff --git a/src/xmlreader.c b/src/xmlreader.c
index 2108d1841..cdaf6b809 100644
--- a/src/xmlreader.c
+++ b/src/xmlreader.c
@@ -1093,17 +1093,12 @@ static int parse_spellbooks(xmlDocPtr doc)
             if (result->nodesetval->nodeNr > 0) {
                 for (k = 0; k != result->nodesetval->nodeNr; ++k) {
                     xmlNodePtr node = result->nodesetval->nodeTab[k];
-                    spell * sp = 0;
                     int level = xml_ivalue(node, "level", -1);
 
-                    propValue = xmlGetProp(node, BAD_CAST "spell");
-                    if (propValue) {
-                        sp = find_spell((const char *)propValue);
+                    if (level > 0 && (propValue = xmlGetProp(node, BAD_CAST "spell")) != NULL) {
+                        spellbook_addref(sb, (const char *)propValue, level);
                         xmlFree(propValue);
                     }
-                    if (sp && level > 0) {
-                        spellbook_add(sb, sp, level);
-                    }
                     else {
                         log_error("invalid entry at index '%d' in spellbook '%s'\n", k, sb->name);
                     }
@@ -1510,7 +1505,8 @@ static int parse_races(xmlDocPtr doc)
                     if (attack->data.sp) {
                         attack->level = xml_ivalue(node, "level", 0);
                         if (attack->level <= 0) {
-                            log_error("magical attack '%s' for race '%s' needs a level: %d\n", attack->data.sp->name, rc->_name, attack->level);
+                            log_error("magical attack '%s' for race '%s' needs a level: %d\n",
+                                spellref_name(attack->data.sp), rc->_name, attack->level);
                         }
                     }
                 }
@@ -1709,8 +1705,8 @@ void register_xmlreader(void)
     xml_register_callback(parse_ships);     /* requires resources, terrains */
     xml_register_callback(parse_equipment); /* requires resources */
 
-    xml_register_callback(parse_spells); /* requires resources */
     xml_register_callback(parse_spellbooks);  /* requires spells */
+    xml_register_callback(parse_spells); /* requires resources */
 
     xml_register_callback(parse_strings);
     xml_register_callback(parse_messages);