Merge pull request #396 from stm2/spell_parameters

fixed treatment of spell parameters (bug 1867)

res/e3a/spells.xml

@@ -585,7 +585,7 @@
     <!-- Schutzzauber -->
     <resource name="aura" amount="5" cost="level"/>
   </spell>
-  <spell name="protective_runes" rank="2" index="99" parameters="kc" ship="true">
+  <spell name="protective_runes" rank="2" index="99" parameters="kc" regiontarget="false" unittarget="false" buildingtarget="true" shiptarget="true" ship="true">
     <resource name="aura" amount="20" cost="fixed"/>
   </spell>
   <spell name="analyze_magic" rank="5" index="102" parameters="kc?" los="true" ship="true" variable="true">

res/eressea/spells.xml

@@ -313,7 +313,7 @@
   <spell name="keeploot" rank="5" index="98" variable="true" combat="3">
     <resource name="aura" amount="1" cost="level"/>
   </spell>
-  <spell name="protective_runes" rank="2" index="99" parameters="kc" ship="true">
+  <spell name="protective_runes" rank="2" index="99" parameters="kc" regiontarget="false" unittarget="false" buildingtarget="true" shiptarget="true" ship="true">
     <resource name="aura" amount="20" cost="fixed"/>
   </spell>
   <spell name="song_resist_magic" rank="2" index="100" far="true" variable="true">
@@ -332,7 +332,7 @@
     <resource name="aura" amount="10" cost="fixed"/>
   </spell>
   <spell name="analyse_object" rank="5" index="105" parameters="kc+" ship="true" variable="true">
-    <resource name="aura" amount="3" cost="level"/>
+    <resource name="aura" amount="3" cost="level" regiontarget="true" unittarget="false" buildingtarget="true" shiptarget="true"/>
   </spell>
   <spell name="destroy_magic" rank="2" index="106" parameters="kc+" los="true" ship="true" far="true" variable="true">
     <resource name="aura" amount="4" cost="level"/>

src/kernel/spell.test.c

@@ -8,7 +8,7 @@
 
 #include <stdlib.h>
 
-static void test_create_spell(CuTest * tc)
+static void test_create_a_spell(CuTest * tc)
 {
     spell * sp;
 
@@ -48,7 +48,7 @@ static void test_create_spell_with_id(CuTest * tc)
 CuSuite *get_spell_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
-    SUITE_ADD_TEST(suite, test_create_spell);
+    SUITE_ADD_TEST(suite, test_create_a_spell);
     SUITE_ADD_TEST(suite, test_create_duplicate_spell);
     SUITE_ADD_TEST(suite, test_create_spell_with_id);
     return suite;

src/kernel/xmlreader.c

@@ -1484,6 +1484,16 @@ static int parse_spells(xmlDocPtr doc)
                 sp->sptyp |= FARCASTING;
             if (xml_bvalue(node, "variable", false))
                 sp->sptyp |= SPELLLEVEL;
+
+            if (xml_bvalue(node, "buildingtarget", false))
+                sp->sptyp |= BUILDINGSPELL;
+            if (xml_bvalue(node, "shiptarget", false))
+                sp->sptyp |= SHIPSPELL;
+            if (xml_bvalue(node, "unittarget", false))
+                sp->sptyp |= UNITSPELL;
+            if (xml_bvalue(node, "regiontarget", false))
+                sp->sptyp |= REGIONSPELL;
+
             k = xml_ivalue(node, "combat", 0);
             if (k >= 0 && k <= 3)
                 sp->sptyp |= modes[k];

src/magic.test.c

@@ -74,24 +74,6 @@ void test_spellbooks(CuTest * tc)
     test_cleanup();
 }
 
-static spell * test_magic_create_spell(void)
-{
-    spell *sp;
-    sp = create_spell("testspell", 0);
-
-    sp->components = (spell_component *)calloc(4, sizeof(spell_component));
-    sp->components[0].amount = 1;
-    sp->components[0].type = get_resourcetype(R_SILVER);
-    sp->components[0].cost = SPC_FIX;
-    sp->components[1].amount = 1;
-    sp->components[1].type = get_resourcetype(R_AURA);
-    sp->components[1].cost = SPC_LEVEL;
-    sp->components[2].amount = 1;
-    sp->components[2].type = get_resourcetype(R_HORSE);
-    sp->components[2].cost = SPC_LINEAR;
-    return sp;
-}
-
 void test_pay_spell(CuTest * tc)
 {
     spell *sp;
@@ -107,7 +89,7 @@ void test_pay_spell(CuTest * tc)
     u = test_create_unit(f, r);
     CuAssertPtrNotNull(tc, u);
 
-    sp = test_magic_create_spell();
+    sp = test_create_spell();
     CuAssertPtrNotNull(tc, sp);
 
     set_level(u, SK_MAGIC, 5);
@@ -141,7 +123,7 @@ void test_pay_spell_failure(CuTest * tc)
     u = test_create_unit(f, r);
     CuAssertPtrNotNull(tc, u);
 
-    sp = test_magic_create_spell();
+    sp = test_create_spell();
     CuAssertPtrNotNull(tc, sp);
 
     set_level(u, SK_MAGIC, 5);

src/report.c

@@ -248,14 +248,15 @@ static size_t write_spell_modifier(spell * sp, int flag, const char * str, bool
     return 0;
 }
 
-static void nr_spell(stream *out, spellbook_entry * sbe, const struct locale *lang)
+void nr_spell_syntax(struct stream *out, struct spellbook_entry * sbe, const struct locale *lang);
+
+void nr_spell(stream *out, spellbook_entry * sbe, const struct locale *lang)
 {
     int bytes, k, itemanz, costtyp;
     char buf[4096];
     char *startp, *bufp = buf;
     size_t size = sizeof(buf) - 1;
     spell * sp = sbe->sp;
-    const char *params = sp->parameter;
 
     newline(out);
     centre(out, spell_name(sp, lang), true);
@@ -361,6 +362,20 @@ static void nr_spell(stream *out, spellbook_entry * sbe, const struct locale *la
     bufp = buf;
     size = sizeof(buf) - 1;
 
+    nr_spell_syntax(out, sbe, lang);
+
+    newline(out);
+}
+
+void nr_spell_syntax(stream *out, spellbook_entry * sbe, const struct locale *lang)
+{
+    int bytes;
+    char buf[4096];
+    char *bufp = buf;
+    size_t size = sizeof(buf) - 1;
+    spell * sp = sbe->sp;
+    const char *params = sp->parameter;
+
     if (sp->sptyp & ISCOMBATSPELL) {
         bytes = (int)strlcpy(bufp, LOC(lang, keyword(K_COMBATSPELL)), size);
     }
@@ -456,34 +471,38 @@ static void nr_spell(stream *out, spellbook_entry * sbe, const struct locale *la
                 WARN_STATIC_BUFFER();
         }
         else if (cp == 'k') {
-            if (*params == 'c') {
+            bool multi = false;
+            if (params && *params == 'c') {
                 /* skip over a potential id */
                 ++params;
             }
+            if (params && *params == '+') {
+                ++params;
+                multi = true;
+            }
             for (targetp = targets; targetp->flag; ++targetp) {
                 if (sp->sptyp & targetp->flag)
                     ++maxparam;
             }
-            if (maxparam > 1) {
+            if (!maxparam || maxparam > 1) {
                 bytes = (int)strlcpy(bufp, " (", size);
                 if (wrptr(&bufp, &size, bytes) != 0)
                     WARN_STATIC_BUFFER();
             }
             i = 0;
             for (targetp = targets; targetp->flag; ++targetp) {
-                if (sp->sptyp & targetp->flag) {
+                if (!maxparam || sp->sptyp & targetp->flag) {
                     if (i++ != 0) {
                         bytes = (int)strlcpy(bufp, " |", size);
                         if (wrptr(&bufp, &size, bytes) != 0)
                             WARN_STATIC_BUFFER();
                     }
-                    if (targetp->param) {
+                    if (targetp->param && targetp->vars) {
                         locp = LOC(lang, targetp->vars);
                         bytes =
                             (int)_snprintf(bufp, size, " %s <%s>", parameters[targetp->param],
                                 locp);
-                        if (*params == '+') {
+                        if (multi) {
-                            ++params;
                             if (wrptr(&bufp, &size, bytes) != 0)
                                 WARN_STATIC_BUFFER();
                             bytes = (int)_snprintf(bufp, size, " [<%s> ...]", locp);
@@ -497,7 +516,7 @@ static void nr_spell(stream *out, spellbook_entry * sbe, const struct locale *la
                         WARN_STATIC_BUFFER();
                 }
             }
-            if (maxparam > 1) {
+            if (!maxparam || maxparam > 1) {
                 bytes = (int)strlcpy(bufp, " )", size);
                 if (wrptr(&bufp, &size, bytes) != 0)
                     WARN_STATIC_BUFFER();
@@ -520,11 +539,13 @@ static void nr_spell(stream *out, spellbook_entry * sbe, const struct locale *la
             bytes = (int)_snprintf(bufp, size, " <%s>", locp);
             if (wrptr(&bufp, &size, bytes) != 0)
                 WARN_STATIC_BUFFER();
+        } else {
+           log_error("unknown spell parameter %c for spell", cp, sp->sname);
         }
     }
     *bufp = 0;
     paragraph(out, buf, 2, 0, 0);
-    newline(out);
+
 }
 
 static void

src/report.h

@@ -20,12 +20,17 @@ extern "C" {
 #endif
 
     struct stream;
+    struct spellbook_entry;
     struct region;
     struct faction;
     void register_nr(void);
     void report_cleanup(void);
     void write_spaces(struct stream *out, size_t num);
     void write_travelthru(struct stream *out, const struct region * r, const struct faction * f);
+
+    void nr_spell_syntax(struct stream *out, struct spellbook_entry * sbe, const struct locale *lang);
+    void nr_spell(struct stream *out, struct spellbook_entry * sbe, const struct locale *lang);
+
 #ifdef __cplusplus
 }
 #endif

src/reports.test.c

@@ -6,6 +6,7 @@
 #include "move.h"
 #include "seen.h"
 #include "travelthru.h"
+#include "keyword.h"
 
 #include <kernel/building.h>
 #include <kernel/faction.h>
@@ -13,6 +14,8 @@
 #include <kernel/region.h>
 #include <kernel/ship.h>
 #include <kernel/unit.h>
+#include <kernel/spell.h>
+#include <kernel/spellbook.h>
 
 #include <util/language.h>
 
@@ -284,6 +287,130 @@ static void test_write_unit(CuTest *tc) {
     test_cleanup();
 }
 
+typedef struct {
+    struct locale *lang;
+    spell *sp;
+    spellbook *spb;
+    spellbook_entry * sbe;
+} spell_fixture;
+
+static void setup_spell_fixture(spell_fixture * spf) {
+    spf->lang = get_or_create_locale("de");
+    locale_setstring(spf->lang, mkname("spell", "testspell"), "Testzauber");
+    locale_setstring(spf->lang, "nr_spell_type", "Typ:");
+    locale_setstring(spf->lang, "sptype_normal", "Normal");
+    locale_setstring(spf->lang, "nr_spell_modifiers", "Modifier:");
+    locale_setstring(spf->lang, "smod_none", "Keine");
+    locale_setstring(spf->lang, keyword(K_CAST), "ZAUBERE");
+    locale_setstring(spf->lang, parameters[P_REGION], "REGION");
+    locale_setstring(spf->lang, parameters[P_LEVEL], "STUFE");
+    locale_setstring(spf->lang, "par_unit", "enr");
+    locale_setstring(spf->lang, "par_ship", "snr");
+    locale_setstring(spf->lang, "par_building", "bnr");
+    locale_setstring(spf->lang, "spellpar::hodor", "Hodor");
+
+    spf->spb = create_spellbook("testbook");
+    spf->sp = test_create_spell();
+    spellbook_add(spf->spb, spf->sp, 1);
+    spf->sbe = spellbook_get(spf->spb, spf->sp);
+
+}
+
+static void test_spell_syntax(CuTest *tc, char *msg, spell_fixture *spell, char *syntax) {
+    stream strm;
+    char buf[1024];
+    char *linestart, *newline;
+    size_t len;
+
+    mstream_init(&strm);
+
+
+    nr_spell_syntax(&strm, spell->sbe, spell->lang);
+
+    strm.api->rewind(strm.handle);
+
+    len = strm.api->read(strm.handle, buf, sizeof(buf));
+    buf[len] = '\0';
+
+    linestart = strtok(buf, "\n");
+    while (linestart && !strstr(linestart, "ZAUBERE"))
+        linestart = strtok(NULL, "\n") ;
+
+    CuAssertTrue(tc, (bool) linestart);
+
+    while ((newline = strtok(NULL, "\n")))
+        *(newline-1) = '\n';
+
+    CuAssertStrEquals_Msg(tc, msg, syntax, linestart);
+
+    mstream_done(&strm);
+}
+
+static void set_parameter(spell_fixture spell, char *value) {
+    free(spell.sp->parameter);
+    spell.sp->parameter = _strdup(value);
+}
+
+static void test_write_spell_syntax(CuTest *tc) {
+    spell_fixture spell;
+
+    test_cleanup();
+    setup_spell_fixture(&spell);
+
+    test_spell_syntax(tc, "vanilla",  &spell,   "  ZAUBERE \"Testzauber\"");
+
+    spell.sp->sptyp |= FARCASTING;
+    test_spell_syntax(tc, "far",  &spell,   "  ZAUBERE [REGION x y] \"Testzauber\"");
+
+    spell.sp->sptyp |= SPELLLEVEL;
+    test_spell_syntax(tc, "farlevel",  &spell,   "  ZAUBERE [REGION x y] [STUFE n] \"Testzauber\"");
+    spell.sp->sptyp = 0;
+
+    set_parameter(spell, "kc");
+    test_spell_syntax(tc, "kc", &spell,   "  ZAUBERE \"Testzauber\" ( REGION | EINHEIT <enr> | SCHIFF <snr> | BURG <bnr> )");
+
+    spell.sp->sptyp |= BUILDINGSPELL;
+    test_spell_syntax(tc, "kc typed", &spell,   "  ZAUBERE \"Testzauber\" BURG <bnr>");
+    spell.sp->sptyp = 0;
+
+    set_parameter(spell, "b");
+    test_spell_syntax(tc, "b", &spell,   "  ZAUBERE \"Testzauber\" <bnr>");
+
+    set_parameter(spell, "s");
+    test_spell_syntax(tc, "s", &spell,   "  ZAUBERE \"Testzauber\" <snr>");
+
+    set_parameter(spell, "s+");
+    test_spell_syntax(tc, "s+", &spell,   "  ZAUBERE \"Testzauber\" <snr> [<snr> ...]");
+
+    set_parameter(spell, "u");
+    test_spell_syntax(tc, "u", &spell,   "  ZAUBERE \"Testzauber\" <enr>");
+
+    set_parameter(spell, "r");
+    test_spell_syntax(tc, "r", &spell,   "  ZAUBERE \"Testzauber\" <x> <y>");
+
+    set_parameter(spell, "bc");
+    free(spell.sp->syntax);
+    spell.sp->syntax = _strdup("hodor");
+    test_spell_syntax(tc, "bc hodor", &spell,   "  ZAUBERE \"Testzauber\" <bnr> <Hodor>");
+    free(spell.sp->syntax);
+    spell.sp->syntax = 0;
+
+    /* There are no spells with optional parameters, so we don't force this, for now
+    set_parameter(spell, "c?");
+    free(spell.sp->syntax);
+    spell.sp->syntax = _strdup("hodor");
+    test_spell_syntax(tc, "c?", &spell,   "  ZAUBERE \"Testzauber\" [<Hodor>]");
+    free(spell.sp->syntax);
+    spell.sp->syntax = 0;
+    */
+
+    set_parameter(spell, "kc+");
+    test_spell_syntax(tc, "kc+", &spell,
+        "  ZAUBERE \"Testzauber\" ( REGION | EINHEIT <enr> [<enr> ...] | SCHIFF <snr>\n  [<snr> ...] | BURG <bnr> [<bnr> ...] )");
+
+    test_cleanup();
+}
+
 CuSuite *get_reports_suite(void)
 {
     CuSuite *suite = CuSuiteNew();
@@ -296,5 +423,6 @@ CuSuite *get_reports_suite(void)
     SUITE_ADD_TEST(suite, test_sparagraph);
     SUITE_ADD_TEST(suite, test_write_travelthru);
     SUITE_ADD_TEST(suite, test_write_unit);
+    SUITE_ADD_TEST(suite, test_write_spell_syntax);
     return suite;
 }

src/tests.c

@@ -196,6 +196,26 @@ void test_create_castorder(castorder *co, unit *u, int level, float force, int r
     free_order(ord);
 }
 
+spell * test_create_spell(void)
+{
+    spell *sp;
+    sp = create_spell("testspell", 0);
+
+    sp->components = (spell_component *)calloc(4, sizeof(spell_component));
+    sp->components[0].amount = 1;
+    sp->components[0].type = get_resourcetype(R_SILVER);
+    sp->components[0].cost = SPC_FIX;
+    sp->components[1].amount = 1;
+    sp->components[1].type = get_resourcetype(R_AURA);
+    sp->components[1].cost = SPC_LEVEL;
+    sp->components[2].amount = 1;
+    sp->components[2].type = get_resourcetype(R_HORSE);
+    sp->components[2].cost = SPC_LINEAR;
+    sp->syntax = 0;
+    sp->parameter = 0;
+    return sp;
+}
+
 void test_translate_param(const struct locale *lang, param_t param, const char *text) {
     struct critbit_tree **cb;
 

src/tests.h

@@ -24,6 +24,7 @@ extern "C" {
     struct terrain_type;
     struct castorder;
     struct spellparameter;
+    struct spell;
 
     struct CuTest;
 
@@ -43,6 +44,7 @@ extern "C" {
     struct ship_type *test_create_shiptype(const char * name);
     struct building_type *test_create_buildingtype(const char *name);
     void test_create_castorder(struct castorder *co, struct unit *u, int level, float force, int range, struct spellparameter *par);
+    struct spell * test_create_spell(void);
 
     int RunAllTests(void);
     void test_translate_param(const struct locale *lang, param_t param, const char *text);