From 7e69149c4092e17fef2d59fd73fe472137d19949 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno@eressea.de>
Date: Thu, 2 Feb 2017 16:52:32 +0100
Subject: [PATCH 1/5] WIP: spellref everywhere

---
 src/creport.c          |  5 +++--
 src/kernel/spell.c     | 13 +++++++++----
 src/kernel/spell.h     | 41 +++++++++++++++++++++--------------------
 src/kernel/spellbook.c | 32 +++++++++++++++++++++++++-------
 src/kernel/spellbook.h |  5 +++--
 src/kernel/xmlreader.c |  2 +-
 src/laws.c             |  4 ++--
 src/magic.c            | 28 ++++++++++++++++------------
 src/report.c           |  6 +++---
 src/reports.c          |  3 ++-
 10 files changed, 85 insertions(+), 54 deletions(-)

diff --git a/src/creport.c b/src/creport.c
index 106f34545..53c542470 100644
--- a/src/creport.c
+++ b/src/creport.c
@@ -718,7 +718,8 @@ 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;
+                // TODO: no need to deref spref here, spref->name == sp->sname
+                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");
@@ -1621,7 +1622,7 @@ 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);
+        cr_reportspell(F, spellref_get(sbe->spref), 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 5e4a57e86..f950918e0 100644
--- a/src/kernel/spell.c
+++ b/src/kernel/spell.c
@@ -167,10 +167,15 @@ struct spell *spellref_get(struct spellref *spref)
 {
     if (!spref->sp) {
         spref->sp = find_spell(spref->name);
-        if (spref->sp) {
-            free(spref->name);
-            spref->name = NULL;
-        }
     }
     return spref->sp;
 }
+
+spellref *spellref_copy(spellref *spref)
+{
+    spellref *result = spellref_create(spref->name);
+    if (spref->sp) {
+        result->sp = spref->sp;
+    }
+    return result;
+}
diff --git a/src/kernel/spell.h b/src/kernel/spell.h
index b5b227b50..116816e26 100644
--- a/src/kernel/spell.h
+++ b/src/kernel/spell.h
@@ -52,6 +52,7 @@ extern "C" {
 
     struct spellref *spellref_create(const char *name);
     void spellref_free(struct spellref *spref);
+    struct spellref *spellref_copy(struct spellref *spref);
     struct spell *spellref_get(struct spellref *spref);
 
     int sp_antimagiczone(struct castorder *co);
@@ -70,7 +71,7 @@ extern "C" {
 }
 #endif
 #endif
-/* ------------------------------------------------------------- *//* Erläuterungen zu den Spruchdefinitionen
+/* ------------------------------------------------------------- *//* Erl�uterungen zu den Spruchdefinitionen
  *
  * Spruchstukturdefinition:
  * spell{
@@ -88,35 +89,35 @@ extern "C" {
  * id:
  * SPL_NOSPELL muss der letzte Spruch in der Liste spelldaten sein,
  * denn nicht auf die Reihenfolge in der Liste sondern auf die id wird
- * geprüft
+ * gepr�ft
  *
  * sptyp:
  * besondere Spruchtypen und Flags
  *    (Regionszauber, Kampfzauber, Farcastbar, Stufe variable, ..)
  *
  * rank:
- * gibt die Priorität und damit die Reihenfolge an, in der der Spruch
+ * gibt die Priorit�t und damit die Reihenfolge an, in der der Spruch
  * gezaubert wird.
- * 1: Aura übertragen
+ * 1: Aura �bertragen
  * 2: Antimagie
- * 3: Magierverändernde Sprüche (Magic Boost, ..)
+ * 3: Magierver�ndernde Spr�che (Magic Boost, ..)
  * 4: Monster erschaffen
  * 5: Standartlevel
  * 7: Teleport
  *
- * Komponenten[Anzahl mögl. Items][Art:Anzahl:Kostentyp]
+ * Komponenten[Anzahl m�gl. Items][Art:Anzahl:Kostentyp]
  *
  * R_AURA:
- * Grundkosten für einen Zauber. Soviel Mp müssen mindestens investiert
- * werden, um den Spruch zu wirken. Zusätzliche Mp können unterschiedliche
+ * Grundkosten f�r einen Zauber. Soviel Mp m�ssen mindestens investiert
+ * werden, um den Spruch zu wirken. Zus�tzliche Mp k�nnen unterschiedliche
  * Auswirkungen haben, die in der Spruchfunktionsroutine definiert werden.
  *
  * R_PERMAURA:
  * Kosten an permantenter Aura
  *
  * Komponenten Kostentyp:
- * SPC_LEVEL == Spruch mit Levelabhängigen Magiekosten. Die angegeben
- * Kosten müssen für Stufe 1 berechnet sein.
+ * SPC_LEVEL == Spruch mit Levelabh�ngigen Magiekosten. Die angegeben
+ * Kosten m�ssen f�r Stufe 1 berechnet sein.
  * SPC_FIX   == Feste Kosten
  *
  * Wenn keine spezielle Syntax angegeben ist, wird die
@@ -137,34 +138,34 @@ extern "C" {
  *
  * u : eine Einheitennummer
  * r : hier kommen zwei Regionskoordinaten x y
- * b : Gebäude- oder Burgnummer
+ * b : Geb�ude- oder Burgnummer
  * s : Schiffsnummer
- * c : String, wird ohne Weiterverarbeitung übergeben
- * i : Zahl (int), wird ohne Weiterverarbeitung übergeben
+ * c : String, wird ohne Weiterverarbeitung �bergeben
+ * i : Zahl (int), wird ohne Weiterverarbeitung �bergeben
  * k : Keywort - dieser String gibt den Paramter an, der folgt. Der
  *     Parameter wird mit findparam() identifiziert.
- *     k muss immer von einem c als Platzhalter für das Objekt gefolgt
+ *     k muss immer von einem c als Platzhalter f�r das Objekt gefolgt
  *     werden.
- *     Ein gutes Beispiel sind hierfür die Sprüche zur Magieanalyse.
+ *     Ein gutes Beispiel sind hierf�r die Spr�che zur Magieanalyse.
  * + : gibt an, das der vorherige Parameter mehrfach vorkommen kann. Da
  *     ein Ende nicht definiert werden kann, muss dies immer am Schluss
  *     kommen.
  *
- * Flags für das Parsing:
+ * Flags f�r das Parsing:
  * TESTRESISTANCE : alle Zielobjekte, also alle Parameter vom Typ Unit,
  *		  Burg, Schiff oder Region, werden auf ihre
- *		  Magieresistenz überprüft
+ *		  Magieresistenz �berpr�ft
  * TESTCANSEE     : jedes Objekt vom Typ Einheit wird auf seine
- *		  Sichtbarkeit überprüft
+ *		  Sichtbarkeit �berpr�ft
  * SEARCHLOCAL   : die Zielobjekte werden nur regional gesucht
  * REGIONSPELL    : Ziel ist die Region, auch wenn kein Zielobjekt
  *		  angegeben wird. Ist TESTRESISTANCE gesetzt, so wird
- *		  die Magieresistenz der Region überprüft
+ *		  die Magieresistenz der Region �berpr�ft
  *
  * Bei fehlendem Ziel oder wenn dieses dem Zauber widersteht, wird die
  * Spruchfunktion nicht aufgerufen.
  * Sind zu wenig Parameter vorhanden, wird der Zauber ebenfalls nicht
- * ausgeführt.
+ * ausgef�hrt.
  * Ist eins von mehreren Zielobjekten resistent, so wird das Flag
  * pa->param[n]->flag == TARGET_RESISTS
  * Ist eins von mehreren Zielobjekten nicht gefunden worden, so ist
diff --git a/src/kernel/spellbook.c b/src/kernel/spellbook.c
index bb060eaee..919142585 100644
--- a/src/kernel/spellbook.c
+++ b/src/kernel/spellbook.c
@@ -50,7 +50,7 @@ void read_spellbook(spellbook **bookp, gamedata *data, int(*get_level)(const spe
                 sb = *bookp;
             }
             if (level > 0 && (data->version >= SPELLBOOK_VERSION || !spellbook_get(sb, sp))) {
-                spellbook_add(sb, sp, level);
+                spellbook_add_spell(sb, sp, level);
             }
         }
     }
@@ -64,25 +64,43 @@ 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, sbe->spref->name);
             WRITE_INT(store, sbe->level);
         }
     }
     WRITE_TOK(store, "end");
 }
 
-void spellbook_add(spellbook *sb, struct spell * sp, int level)
+void spellbook_add(spellbook *sb, spellref *spref, int level)
 {
     spellbook_entry * sbe;
 
-    assert(sb && sp && level > 0);
+    assert(sb && spref && level > 0);
+#ifndef NDEBUG
+    if (spellbook_get(sb, spellref_get(spref))) {
+        log_error("duplicate spell in spellbook '%s': '%s'\n", sb->name, spref->name);
+    }
+#endif  
+    sbe = (spellbook_entry *)malloc(sizeof(spellbook_entry));
+    sbe->spref = spellref_copy(spref);
+    sbe->level = level;
+    selist_push(&sb->spells, sbe);
+}
+
+void spellbook_add_spell(spellbook *sb, spell *sp, int level)
+{
+    spellbook_entry * sbe;
+
+    assert(sb && sp->sname && level > 0);
 #ifndef NDEBUG
     if (spellbook_get(sb, sp)) {
         log_error("duplicate spell in spellbook '%s': '%s'\n", sb->name, sp->sname);
     }
 #endif  
     sbe = (spellbook_entry *)malloc(sizeof(spellbook_entry));
-    sbe->sp = sp;
+    // FIXME: create a better API to create spellref from spell 
+    sbe->spref = spellref_create(sp->sname);
+    sbe->spref->sp = sp;
     sbe->level = level;
     selist_push(&sb->spells, sbe);
 }
@@ -95,6 +113,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_free(sbe->spref);
         free(sbe);
     }
     selist_free(sb->spells);
@@ -124,11 +143,10 @@ 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 (sp == sbe->sp) {
+            if (sp == spellref_get(sbe->spref)) {
                 return sbe;
             }
         }
     }
     return 0;
 }
-
diff --git a/src/kernel/spellbook.h b/src/kernel/spellbook.h
index 75a420c06..858240f54 100644
--- a/src/kernel/spellbook.h
+++ b/src/kernel/spellbook.h
@@ -29,7 +29,7 @@ extern "C" {
     struct selist;
 
     typedef struct spellbook_entry {
-        struct spell * sp;
+        struct spellref * spref;
         int level;
     } spellbook_entry;
 
@@ -44,7 +44,8 @@ extern "C" {
     void read_spellbook(struct spellbook **bookp, struct gamedata *data, int(*get_level)(const struct spell * sp, void *), void * cbdata);
     void write_spellbook(const struct spellbook *book, struct storage *store);
 
-    void spellbook_add(spellbook *sbp, struct spell * sp, int level);
+    void spellbook_add(spellbook *sbp, struct spellref *spref, int level);
+    void spellbook_add_spell(spellbook *sb, struct spell *sp, 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/xmlreader.c b/src/kernel/xmlreader.c
index 959872920..f9d0912da 100644
--- a/src/kernel/xmlreader.c
+++ b/src/kernel/xmlreader.c
@@ -2076,9 +2076,9 @@ void register_xmlreader(void)
     xml_register_callback(parse_buildings);       /* requires resources */
     xml_register_callback(parse_ships);   /* requires terrains */
     xml_register_callback(parse_races);   /* requires spells */
-    xml_register_callback(parse_spells);  /* requires resources */
     xml_register_callback(parse_spellbooks);  /* requires spells */
     xml_register_callback(parse_equipment);       /* requires spells */
+    xml_register_callback(parse_spells);  /* requires resources */
     xml_register_callback(parse_calendar);
 }
 #endif
diff --git a/src/laws.c b/src/laws.c
index 6b0bfda8d..f543fc78f 100644
--- a/src/laws.c
+++ b/src/laws.c
@@ -3458,8 +3458,8 @@ 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);
+                if (!spellbook_get(dst, spellref_get(sbe->spref))) {
+                    spellbook_add(dst, sbe->spref, sbe->level);
                 }
             }
         }
diff --git a/src/magic.c b/src/magic.c
index 7969253e0..bdac11df8 100644
--- a/src/magic.c
+++ b/src/magic.c
@@ -430,14 +430,14 @@ void show_new_spells(faction * f, int level, const spellbook *book)
         for (qi = 0; ql; selist_advance(&ql, &qi, 1)) {
             spellbook_entry *sbe = (spellbook_entry *)selist_get(ql, qi);
             if (sbe->level <= level) {
-
-                if (!already_seen(f, sbe->sp)) {
+                spell *sp = spellref_get(sbe->spref);
+                if (!already_seen(f, sp)) {
                     attrib * a = a_new(&at_reportspell);
                     spellbook_entry * entry = (spellbook_entry *)a->data.v;
                     entry->level = sbe->level;
-                    entry->sp = sbe->sp;
+                    entry->spref = spellref_copy(sbe->spref);
                     a_add(&f->attribs, a);
-                    a_add(&f->attribs, a_new(&at_seenspell))->data.v = sbe->sp;
+                    a_add(&f->attribs, a_new(&at_seenspell))->data.v = sp;
                 }
             }
         }
@@ -481,13 +481,16 @@ void pick_random_spells(faction * f, int level, spellbook * book, int num_spells
                     commonspells[maxspell] = sbe;
                     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;
+                else {
+                    spell *sp = spellref_get(sbe->spref);
+                    if (f->spellbook && 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;
                 }
             }
 
@@ -495,7 +498,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, sbe->spref, sbe->level);
                 commonspells[spellno] = commonspells[--numspells];
             }
         }
@@ -2948,6 +2951,7 @@ const char *spell_info(const spell * sp, const struct locale *lang)
     return LOC(lang, mkname("spellinfo", sp->sname));
 }
 
+// TODO: should take the name, not the spell (spellref optimizations)
 const char *spell_name(const spell * sp, const struct locale *lang)
 {
     return LOC(lang, mkname("spell", sp->sname));
@@ -2967,7 +2971,7 @@ 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);
-        spell *sp = sbe->sp;
+        spell *sp = spellref_get(sbe->spref);
 
         const char *n = spell_name(sp, lang);
         if (!n) {
diff --git a/src/report.c b/src/report.c
index 1282789fa..fa1222f90 100644
--- a/src/report.c
+++ b/src/report.c
@@ -234,7 +234,7 @@ paragraph(struct stream *out, const char *str, ptrdiff_t indent, int hanging_ind
     } while (*begin);
 }
 
-static size_t write_spell_modifier(spell * sp, int flag, const char * str, bool cont, char * bufp, size_t size) {
+static size_t write_spell_modifier(const spell * sp, int flag, const char * str, bool cont, char * bufp, size_t size) {
     if (sp->sptyp & flag) {
         size_t bytes = 0;
         if (cont) {
@@ -255,7 +255,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;
-    spell * sp = sbe->sp;
+    const spell * sp = spellref_get(sbe->spref);
     const char *params = sp->parameter;
 
     if (sp->sptyp & ISCOMBATSPELL) {
@@ -445,7 +445,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;
-    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 5a47acfed..bb727083e 100644
--- a/src/reports.c
+++ b/src/reports.c
@@ -680,7 +680,8 @@ size_t size)
                     if (wrptr(&bufp, &size, result) != 0) {
                         WARN_STATIC_BUFFER();
                     }
-                    bufp = STRLCPY(bufp, spell_name(sbe->sp, f->locale), size);
+                    // TODO: no need to deref the spellref here (spref->name is good)
+                    bufp = STRLCPY(bufp, spell_name(spellref_get(sbe->spref), f->locale), size);
                 }
             }
 

From 0c08a644950fbead4a917798e6237cf20776efb4 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Thu, 2 Feb 2017 17:10:53 +0100
Subject: [PATCH 2/5] new clibs module, fixed MSVC settings

---
 clibs | 2 +-
 cmake | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/clibs b/clibs
index f91ef37f0..27c8b3202 160000
--- a/clibs
+++ b/clibs
@@ -1 +1 @@
-Subproject commit f91ef37f08c5244bf616f1836c0aa9caaf36805c
+Subproject commit 27c8b3202b52766465743c3324fc0b52c5ba4b11
diff --git a/cmake b/cmake
index f1fb3943a..d88983c7f 160000
--- a/cmake
+++ b/cmake
@@ -1 +1 @@
-Subproject commit f1fb3943ace59994d90d71a891b80033dc2700a2
+Subproject commit d88983c7ff4bc3a4884a7c3f74e8190bac5eab23

From 456d1bd1967fb66d0e354d45df57019489501461 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Thu, 2 Feb 2017 20:08:04 +0100
Subject: [PATCH 3/5] spellbooks should not use spellref, it's unnecessary.

---
 src/creport.c           |  4 ++--
 src/kernel/spell.c      | 26 ++++++++++++++------------
 src/kernel/spell.h      |  3 +--
 src/kernel/spell.test.c |  5 +++--
 src/kernel/spellbook.c  | 33 +++++++--------------------------
 src/kernel/spellbook.h  |  7 +++----
 src/kernel/xmlreader.c  | 10 +++++-----
 src/laws.c              |  4 ++--
 src/magic.c             | 19 ++++++++-----------
 src/report.c            |  4 ++--
 src/reports.c           |  2 +-
 11 files changed, 48 insertions(+), 69 deletions(-)

diff --git a/src/creport.c b/src/creport.c
index 53c542470..345aaf0bc 100644
--- a/src/creport.c
+++ b/src/creport.c
@@ -719,7 +719,7 @@ static void cr_output_spells(stream *out, const unit * u, int maxlevel)
             spellbook_entry * sbe = (spellbook_entry *)selist_get(ql, qi);
             if (sbe->level <= maxlevel) {
                 // TODO: no need to deref spref here, spref->name == sp->sname
-                spell * sp = spellref_get(sbe->spref);
+                spell * sp = sbe->sp;
                 const char *name = translate(mkname("spell", sp->sname), spell_name(sp, f->locale));
                 if (!header) {
                     stream_printf(out, "SPRUECHE\n");
@@ -1622,7 +1622,7 @@ 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, spellref_get(sbe->spref), sbe->level, f->locale);
+        cr_reportspell(F, sbe->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 f950918e0..212b0a7d2 100644
--- a/src/kernel/spell.c
+++ b/src/kernel/spell.c
@@ -146,12 +146,18 @@ spell *find_spellbyid(unsigned int id)
     return NULL;
 }
 
-struct spellref *spellref_create(const char *name)
+struct spellref *spellref_create(spell *sp, const char *name)
 {
     spellref *spref = malloc(sizeof(spellref));
 
-    spref->name = strdup(name);
-    spref->sp = NULL;
+    if (sp) {
+        spref->sp = sp;
+        spref->name = strdup(sp->sname);
+    }
+    else if (name) {
+        spref->name = strdup(name);
+        spref->sp = NULL;
+    }
     return spref;
 }
 
@@ -166,16 +172,12 @@ void spellref_free(spellref *spref)
 struct spell *spellref_get(struct spellref *spref)
 {
     if (!spref->sp) {
+        assert(spref->name);
         spref->sp = find_spell(spref->name);
+        if (spref->sp) {
+            free(spref->name);
+            spref->name = NULL;
+        }
     }
     return spref->sp;
 }
-
-spellref *spellref_copy(spellref *spref)
-{
-    spellref *result = spellref_create(spref->name);
-    if (spref->sp) {
-        result->sp = spref->sp;
-    }
-    return result;
-}
diff --git a/src/kernel/spell.h b/src/kernel/spell.h
index 116816e26..e7592f082 100644
--- a/src/kernel/spell.h
+++ b/src/kernel/spell.h
@@ -50,9 +50,8 @@ extern "C" {
         struct spell *sp;
     } spellref;
 
-    struct spellref *spellref_create(const char *name);
+    struct spellref *spellref_create(struct spell *sp, const char *name);
     void spellref_free(struct spellref *spref);
-    struct spellref *spellref_copy(struct spellref *spref);
     struct spell *spellref_get(struct spellref *spref);
 
     int sp_antimagiczone(struct castorder *co);
diff --git a/src/kernel/spell.test.c b/src/kernel/spell.test.c
index 4a8cd617a..b500f94cc 100644
--- a/src/kernel/spell.test.c
+++ b/src/kernel/spell.test.c
@@ -74,13 +74,14 @@ static void test_spellref(CuTest *tc)
     spellref *ref;
     spell *sp;
     test_setup();
-    ref = spellref_create("hodor");
+    ref = spellref_create(NULL, "hodor");
     CuAssertPtrNotNull(tc, ref);
+    CuAssertPtrEquals(tc, NULL, ref->sp);
+    CuAssertStrEquals(tc, "hodor", ref->name);
     CuAssertPtrEquals(tc, NULL, spellref_get(ref));
     sp = create_spell("hodor", 0);
     CuAssertPtrNotNull(tc, sp);
     CuAssertPtrEquals(tc, sp, spellref_get(ref));
-    CuAssertPtrEquals(tc, NULL, ref->name);
     spellref_free(ref);
     test_cleanup();
 }
diff --git a/src/kernel/spellbook.c b/src/kernel/spellbook.c
index 919142585..b7f65420c 100644
--- a/src/kernel/spellbook.c
+++ b/src/kernel/spellbook.c
@@ -50,7 +50,7 @@ void read_spellbook(spellbook **bookp, gamedata *data, int(*get_level)(const spe
                 sb = *bookp;
             }
             if (level > 0 && (data->version >= SPELLBOOK_VERSION || !spellbook_get(sb, sp))) {
-                spellbook_add_spell(sb, sp, level);
+                spellbook_add(sb, sp, level);
             }
         }
     }
@@ -64,43 +64,25 @@ 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->spref->name);
+            WRITE_TOK(store, sbe->sp->sname);
             WRITE_INT(store, sbe->level);
         }
     }
     WRITE_TOK(store, "end");
 }
 
-void spellbook_add(spellbook *sb, spellref *spref, int level)
+void spellbook_add(spellbook *sb, spell *sp, int level)
 {
     spellbook_entry * sbe;
 
-    assert(sb && spref && level > 0);
-#ifndef NDEBUG
-    if (spellbook_get(sb, spellref_get(spref))) {
-        log_error("duplicate spell in spellbook '%s': '%s'\n", sb->name, spref->name);
-    }
-#endif  
-    sbe = (spellbook_entry *)malloc(sizeof(spellbook_entry));
-    sbe->spref = spellref_copy(spref);
-    sbe->level = level;
-    selist_push(&sb->spells, sbe);
-}
-
-void spellbook_add_spell(spellbook *sb, spell *sp, int level)
-{
-    spellbook_entry * sbe;
-
-    assert(sb && sp->sname && level > 0);
+    assert(sb && sp && level > 0);
 #ifndef NDEBUG
     if (spellbook_get(sb, sp)) {
         log_error("duplicate spell in spellbook '%s': '%s'\n", sb->name, sp->sname);
     }
 #endif  
     sbe = (spellbook_entry *)malloc(sizeof(spellbook_entry));
-    // FIXME: create a better API to create spellref from spell 
-    sbe->spref = spellref_create(sp->sname);
-    sbe->spref->sp = sp;
+    sbe->sp = sp;
     sbe->level = level;
     selist_push(&sb->spells, sbe);
 }
@@ -113,7 +95,6 @@ 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_free(sbe->spref);
         free(sbe);
     }
     selist_free(sb->spells);
@@ -135,7 +116,7 @@ int spellbook_foreach(spellbook *sb, int(*callback)(spellbook_entry *, void *),
     return 0;
 }
 
-spellbook_entry * spellbook_get(spellbook *sb, const struct spell * sp)
+spellbook_entry * spellbook_get(spellbook *sb, const struct spell *sp)
 {
     if (sb) {
         selist *ql;
@@ -143,7 +124,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 (sp == spellref_get(sbe->spref)) {
+            if (sbe->sp==sp) {
                 return sbe;
             }
         }
diff --git a/src/kernel/spellbook.h b/src/kernel/spellbook.h
index 858240f54..fb3438a8b 100644
--- a/src/kernel/spellbook.h
+++ b/src/kernel/spellbook.h
@@ -29,7 +29,7 @@ extern "C" {
     struct selist;
 
     typedef struct spellbook_entry {
-        struct spellref * spref;
+        struct spell *sp;
         int level;
     } spellbook_entry;
 
@@ -44,11 +44,10 @@ extern "C" {
     void read_spellbook(struct spellbook **bookp, struct gamedata *data, int(*get_level)(const struct spell * sp, void *), void * cbdata);
     void write_spellbook(const struct spellbook *book, struct storage *store);
 
-    void spellbook_add(spellbook *sbp, struct spellref *spref, int level);
-    void spellbook_add_spell(spellbook *sb, struct spell *sp, int level);
+    void spellbook_add(spellbook *sbp, struct spell *sp, 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);
+    spellbook_entry * spellbook_get(spellbook *sb, const struct spell *sp);
 
 #ifdef __cplusplus
 }
diff --git a/src/kernel/xmlreader.c b/src/kernel/xmlreader.c
index f9d0912da..4225858ee 100644
--- a/src/kernel/xmlreader.c
+++ b/src/kernel/xmlreader.c
@@ -79,7 +79,7 @@ static spellref *xml_spellref(xmlNode * node, const char *name)
 {
     xmlChar *propValue = xmlGetProp(node, BAD_CAST name);
     if (propValue != NULL) {
-        spellref *ref = spellref_create((const char *)propValue);
+        spellref *ref = spellref_create(NULL, (const char *)propValue);
         xmlFree(propValue);
         return ref;
     }
@@ -2075,10 +2075,10 @@ void register_xmlreader(void)
 
     xml_register_callback(parse_buildings);       /* requires resources */
     xml_register_callback(parse_ships);   /* requires terrains */
-    xml_register_callback(parse_races);   /* requires spells */
-    xml_register_callback(parse_spellbooks);  /* requires spells */
-    xml_register_callback(parse_equipment);       /* requires spells */
-    xml_register_callback(parse_spells);  /* requires resources */
+    xml_register_callback(parse_races);
     xml_register_callback(parse_calendar);
+    xml_register_callback(parse_spells);  /* requires resources */
+    xml_register_callback(parse_equipment);       /* requires spells */
+    xml_register_callback(parse_spellbooks);  /* requires spells */
 }
 #endif
diff --git a/src/laws.c b/src/laws.c
index f543fc78f..6b0bfda8d 100644
--- a/src/laws.c
+++ b/src/laws.c
@@ -3458,8 +3458,8 @@ 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, spellref_get(sbe->spref))) {
-                    spellbook_add(dst, sbe->spref, sbe->level);
+                if (!spellbook_get(dst, sbe->sp)) {
+                    spellbook_add(dst, sbe->sp, sbe->level);
                 }
             }
         }
diff --git a/src/magic.c b/src/magic.c
index bdac11df8..7e820f9c3 100644
--- a/src/magic.c
+++ b/src/magic.c
@@ -430,14 +430,13 @@ void show_new_spells(faction * f, int level, const spellbook *book)
         for (qi = 0; ql; selist_advance(&ql, &qi, 1)) {
             spellbook_entry *sbe = (spellbook_entry *)selist_get(ql, qi);
             if (sbe->level <= level) {
-                spell *sp = spellref_get(sbe->spref);
-                if (!already_seen(f, sp)) {
+                if (!already_seen(f, sbe->sp)) {
                     attrib * a = a_new(&at_reportspell);
                     spellbook_entry * entry = (spellbook_entry *)a->data.v;
                     entry->level = sbe->level;
-                    entry->spref = spellref_copy(sbe->spref);
+                    entry->sp = sbe->sp;
                     a_add(&f->attribs, a);
-                    a_add(&f->attribs, a_new(&at_seenspell))->data.v = sp;
+                    a_add(&f->attribs, a_new(&at_seenspell))->data.v = sbe->sp;
                 }
             }
         }
@@ -482,8 +481,7 @@ void pick_random_spells(faction * f, int level, spellbook * book, int num_spells
                     sbe = 0;
                 }
                 else {
-                    spell *sp = spellref_get(sbe->spref);
-                    if (f->spellbook && spellbook_get(f->spellbook, sp)) {
+                    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) {
@@ -498,7 +496,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->spref, sbe->level);
+                spellbook_add(f->spellbook, sbe->sp, sbe->level);
                 commonspells[spellno] = commonspells[--numspells];
             }
         }
@@ -2971,15 +2969,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);
-        spell *sp = spellref_get(sbe->spref);
 
-        const char *n = spell_name(sp, lang);
+        const char *n = spell_name(sbe->sp, lang);
         if (!n) {
-            log_error("no translation in locale %s for spell %s\n", locale_name(lang), sp->sname);
+            log_error("no translation in locale %s for spell %s\n", locale_name(lang), sbe->sp->sname);
         }
         else {
             variant token;
-            token.v = sp;
+            token.v = sbe->sp;
             addtoken((struct tnode **)tokens, n, token);
         }
     }
diff --git a/src/report.c b/src/report.c
index fa1222f90..06244c70c 100644
--- a/src/report.c
+++ b/src/report.c
@@ -255,7 +255,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 = spellref_get(sbe->spref);
+    const spell * sp = sbe->sp;
     const char *params = sp->parameter;
 
     if (sp->sptyp & ISCOMBATSPELL) {
@@ -445,7 +445,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 = spellref_get(sbe->spref);
+    const spell * sp = sbe->sp;
 
     newline(out);
     centre(out, spell_name(sp, lang), true);
diff --git a/src/reports.c b/src/reports.c
index bb727083e..318660024 100644
--- a/src/reports.c
+++ b/src/reports.c
@@ -681,7 +681,7 @@ size_t size)
                         WARN_STATIC_BUFFER();
                     }
                     // TODO: no need to deref the spellref here (spref->name is good)
-                    bufp = STRLCPY(bufp, spell_name(spellref_get(sbe->spref), f->locale), size);
+                    bufp = STRLCPY(bufp, spell_name(sbe->sp, f->locale), size);
                 }
             }
 

From 0224620ca332048a11d05c3d12197bdcb7efb822 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Thu, 2 Feb 2017 20:12:52 +0100
Subject: [PATCH 4/5] code duplication, lazy_spell was almost the same as
 spellref.

---
 src/kernel/equipment.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/src/kernel/equipment.c b/src/kernel/equipment.c
index 00f8c2f95..6b74121ff 100644
--- a/src/kernel/equipment.c
+++ b/src/kernel/equipment.c
@@ -87,8 +87,7 @@ void equipment_setskill(equipment * eq, skill_t sk, const char *value)
 }
 
 typedef struct lazy_spell {
-    char *name;
-    struct spell *sp;
+    struct spellref *spref;
     int level;
 } lazy_spell;
 
@@ -96,9 +95,8 @@ void equipment_addspell(equipment * eq, const char * name, int level)
 {
     if (eq) {
         lazy_spell *ls = malloc(sizeof(lazy_spell));
-        ls->sp = NULL;
+        ls->spref = spellref_create(NULL, name);
         ls->level = level;
-        ls->name = strdup(name);
         selist_push(&eq->spells, ls);
     }
 }
@@ -162,12 +160,8 @@ void equip_unit_mask(struct unit *u, const struct equipment *eq, int mask)
 
                 for (qi = 0; ql; selist_advance(&ql, &qi, 1)) {
                     lazy_spell *sbe = (lazy_spell *)selist_get(ql, qi);
-                    if (!sbe->sp) {
-                        sbe->sp = find_spell(sbe->name);
-                        free(sbe->name);
-                        sbe->name = NULL;
-                    }
-                    unit_add_spell(u, mage, sbe->sp, sbe->level);
+                    spell *sp = spellref_get(sbe->spref);
+                    unit_add_spell(u, mage, sp, sbe->level);
                 }
             }
         }
@@ -238,7 +232,7 @@ void equip_items(struct item **items, const struct equipment *eq)
 
 void free_ls(void *arg) {
     lazy_spell *ls = (lazy_spell*)arg;
-    free(ls->name);
+    spellref_free(ls->spref);
     free(ls);
 }
 

From 7009f26ee51652eb352574d7455162a68d8554e0 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Thu, 2 Feb 2017 20:12:52 +0100
Subject: [PATCH 5/5] code duplication, lazy_spell was almost the same as
 spellref.

---
 src/kernel/equipment.c | 16 +++++-----------
 src/kernel/xmlreader.c | 17 ++++++++++-------
 2 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/src/kernel/equipment.c b/src/kernel/equipment.c
index 00f8c2f95..6b74121ff 100644
--- a/src/kernel/equipment.c
+++ b/src/kernel/equipment.c
@@ -87,8 +87,7 @@ void equipment_setskill(equipment * eq, skill_t sk, const char *value)
 }
 
 typedef struct lazy_spell {
-    char *name;
-    struct spell *sp;
+    struct spellref *spref;
     int level;
 } lazy_spell;
 
@@ -96,9 +95,8 @@ void equipment_addspell(equipment * eq, const char * name, int level)
 {
     if (eq) {
         lazy_spell *ls = malloc(sizeof(lazy_spell));
-        ls->sp = NULL;
+        ls->spref = spellref_create(NULL, name);
         ls->level = level;
-        ls->name = strdup(name);
         selist_push(&eq->spells, ls);
     }
 }
@@ -162,12 +160,8 @@ void equip_unit_mask(struct unit *u, const struct equipment *eq, int mask)
 
                 for (qi = 0; ql; selist_advance(&ql, &qi, 1)) {
                     lazy_spell *sbe = (lazy_spell *)selist_get(ql, qi);
-                    if (!sbe->sp) {
-                        sbe->sp = find_spell(sbe->name);
-                        free(sbe->name);
-                        sbe->name = NULL;
-                    }
-                    unit_add_spell(u, mage, sbe->sp, sbe->level);
+                    spell *sp = spellref_get(sbe->spref);
+                    unit_add_spell(u, mage, sp, sbe->level);
                 }
             }
         }
@@ -238,7 +232,7 @@ void equip_items(struct item **items, const struct equipment *eq)
 
 void free_ls(void *arg) {
     lazy_spell *ls = (lazy_spell*)arg;
-    free(ls->name);
+    spellref_free(ls->spref);
     free(ls);
 }
 
diff --git a/src/kernel/xmlreader.c b/src/kernel/xmlreader.c
index 4225858ee..854a6dd4c 100644
--- a/src/kernel/xmlreader.c
+++ b/src/kernel/xmlreader.c
@@ -2068,17 +2068,20 @@ static int parse_strings(xmlDocPtr doc)
 
 void register_xmlreader(void)
 {
-    xml_register_callback(parse_strings);
-    xml_register_callback(parse_messages);
-    xml_register_callback(parse_resources);
     xml_register_callback(parse_rules);
 
-    xml_register_callback(parse_buildings);       /* requires resources */
-    xml_register_callback(parse_ships);   /* requires terrains */
     xml_register_callback(parse_races);
     xml_register_callback(parse_calendar);
-    xml_register_callback(parse_spells);  /* requires resources */
-    xml_register_callback(parse_equipment);       /* requires spells */
+    xml_register_callback(parse_resources);
+
+    xml_register_callback(parse_buildings); /* requires resources */
+    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_strings);
+    xml_register_callback(parse_messages);
 }
 #endif