From f3972a23901ee64f192984a385957099fabc393a Mon Sep 17 00:00:00 2001 From: Steffen Mecke Date: Thu, 12 Nov 2015 19:34:25 +0100 Subject: [PATCH 1/4] prepared test for nr_spell --- src/kernel/spell.test.c | 4 ++-- src/magic.test.c | 22 ++----------------- src/report.c | 2 +- src/report.h | 4 ++++ src/reports.test.c | 48 +++++++++++++++++++++++++++++++++++++++++ src/tests.c | 18 ++++++++++++++++ src/tests.h | 2 ++ 7 files changed, 77 insertions(+), 23 deletions(-) diff --git a/src/kernel/spell.test.c b/src/kernel/spell.test.c index becc93d6f..23e227ebe 100644 --- a/src/kernel/spell.test.c +++ b/src/kernel/spell.test.c @@ -8,7 +8,7 @@ #include -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; diff --git a/src/magic.test.c b/src/magic.test.c index b208836f9..6ed4d81a9 100644 --- a/src/magic.test.c +++ b/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); diff --git a/src/report.c b/src/report.c index f157a75f4..42a9e8cdf 100644 --- a/src/report.c +++ b/src/report.c @@ -248,7 +248,7 @@ 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(stream *out, spellbook_entry * sbe, const struct locale *lang) { int bytes, k, itemanz, costtyp; char buf[4096]; diff --git a/src/report.h b/src/report.h index 657bef36b..6379dbeeb 100644 --- a/src/report.h +++ b/src/report.h @@ -20,12 +20,16 @@ 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(struct stream *out, struct spellbook_entry * sbe, const struct locale *lang); + #ifdef __cplusplus } #endif diff --git a/src/reports.test.c b/src/reports.test.c index 1dd9f5928..7b14680a5 100644 --- a/src/reports.test.c +++ b/src/reports.test.c @@ -6,6 +6,7 @@ #include "move.h" #include "seen.h" #include "travelthru.h" +#include "keyword.h" #include #include @@ -13,6 +14,8 @@ #include #include #include +#include +#include #include @@ -284,6 +287,50 @@ static void test_write_unit(CuTest *tc) { test_cleanup(); } +static void test_write_spell_syntax(CuTest *tc) { + stream strm; + char buf[1024]; + char *line; + size_t len; + spell *sp; + spellbook *spb; + spellbook_entry * sbe; + struct locale *lang; + + + test_cleanup(); + + lang = get_or_create_locale("de"); + locale_setstring(lang, "spell::testspell", "Testzauber"); + locale_setstring(lang, "nr_spell_type", "Typ:"); + locale_setstring(lang, "sptype_normal", "Normal"); + locale_setstring(lang, "nr_spell_modifiers", "Modifier:"); + locale_setstring(lang, "smod_none", "Keine"); + locale_setstring(lang, keyword(K_CAST), "ZAUBERE"); + + spb = create_spellbook("testbook"); + sp = test_create_spell(); + spellbook_add(spb, sp, 1); + sbe = spellbook_get(spb, sp); + mstream_init(&strm); + + nr_spell(&strm, sbe, lang); + stream_printf(&strm, "\n"); + + strm.api->rewind(strm.handle); + + len = strm.api->read(strm.handle, buf, sizeof(buf)); + buf[len] = '\0'; + + line = strtok(buf, "\n"); + while (line && !strstr(line, "ZAUBERE")) line = strtok(NULL, "\n") ; + + CuAssertTrue(tc, (bool) line); + CuAssertStrEquals(tc, " ZAUBERE \"Testzauber\"", line); + mstream_done(&strm); + test_cleanup(); +} + CuSuite *get_reports_suite(void) { CuSuite *suite = CuSuiteNew(); @@ -296,5 +343,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; } diff --git a/src/tests.c b/src/tests.c index e7518211e..380eb7069 100644 --- a/src/tests.c +++ b/src/tests.c @@ -190,6 +190,24 @@ 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; + return sp; +} + void test_translate_param(const struct locale *lang, param_t param, const char *text) { struct critbit_tree **cb; diff --git a/src/tests.h b/src/tests.h index f0b64729b..ffed85c21 100644 --- a/src/tests.h +++ b/src/tests.h @@ -22,6 +22,7 @@ extern "C" { struct terrain_type; struct castorder; struct spellparameter; + struct spell; struct CuTest; @@ -41,6 +42,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); From 4ab92e3caf1ef59bd5a82d16fb876d180c450464 Mon Sep 17 00:00:00 2001 From: Steffen Mecke Date: Tue, 13 Jan 2015 22:03:09 +0100 Subject: [PATCH 2/4] fixed descriptions of spells with parameters (bug #2060+1867) --- res/e3a/spells.xml | 2 +- res/eressea/spells.xml | 4 ++-- src/kernel/xmlreader.c | 10 ++++++++++ src/report.c | 12 ++++++------ 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/res/e3a/spells.xml b/res/e3a/spells.xml index 498777952..4bb3470d8 100644 --- a/res/e3a/spells.xml +++ b/res/e3a/spells.xml @@ -585,7 +585,7 @@ - + diff --git a/res/eressea/spells.xml b/res/eressea/spells.xml index 6bd5fc9c4..015d64a7e 100644 --- a/res/eressea/spells.xml +++ b/res/eressea/spells.xml @@ -313,7 +313,7 @@ - + @@ -332,7 +332,7 @@ - + diff --git a/src/kernel/xmlreader.c b/src/kernel/xmlreader.c index 28a351a72..d5aba590b 100644 --- a/src/kernel/xmlreader.c +++ b/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]; diff --git a/src/report.c b/src/report.c index 42a9e8cdf..6cafe1d9d 100644 --- a/src/report.c +++ b/src/report.c @@ -305,7 +305,7 @@ void nr_spell(stream *out, spellbook_entry * sbe, const struct locale *lang) if (sp->sptyp & SPELLLEVEL) { bytes = _snprintf(bufp, size, " %d %s", itemanz, LOC(lang, resourcename(rtype, - itemanz != 1))); + itemanz != 1))); if (wrptr(&bufp, &size, bytes) != 0) WARN_STATIC_BUFFER(); if (costtyp == SPC_LEVEL || costtyp == SPC_LINEAR) { @@ -464,24 +464,24 @@ void nr_spell(stream *out, spellbook_entry * sbe, const struct locale *lang) 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); + locp); if (*params == '+') { ++params; if (wrptr(&bufp, &size, bytes) != 0) @@ -497,7 +497,7 @@ void nr_spell(stream *out, spellbook_entry * sbe, const struct locale *lang) WARN_STATIC_BUFFER(); } } - if (maxparam > 1) { + if (!maxparam || maxparam > 1) { bytes = (int)strlcpy(bufp, " )", size); if (wrptr(&bufp, &size, bytes) != 0) WARN_STATIC_BUFFER(); From 477d69152fb79460ed000e70bebcb9b1bd7984eb Mon Sep 17 00:00:00 2001 From: Steffen Mecke Date: Thu, 12 Nov 2015 22:29:56 +0100 Subject: [PATCH 3/4] added tests for all kinds of syntax parameters --- src/report.c | 31 +++++++++-- src/report.h | 1 + src/reports.test.c | 126 ++++++++++++++++++++++++++++++++++++--------- 3 files changed, 129 insertions(+), 29 deletions(-) diff --git a/src/report.c b/src/report.c index 6cafe1d9d..03e5461bd 100644 --- a/src/report.c +++ b/src/report.c @@ -248,6 +248,8 @@ static size_t write_spell_modifier(spell * sp, int flag, const char * str, bool return 0; } +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; @@ -255,7 +257,6 @@ void nr_spell(stream *out, spellbook_entry * sbe, const struct locale *lang) 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 @@ void nr_spell(stream *out, spellbook_entry * sbe, const struct locale *lang) 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,10 +471,15 @@ void nr_spell(stream *out, spellbook_entry * sbe, const struct locale *lang) 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; @@ -482,8 +502,7 @@ void nr_spell(stream *out, spellbook_entry * sbe, const struct locale *lang) bytes = (int)_snprintf(bufp, size, " %s <%s>", parameters[targetp->param], locp); - if (*params == '+') { - ++params; + if (multi) { if (wrptr(&bufp, &size, bytes) != 0) WARN_STATIC_BUFFER(); bytes = (int)_snprintf(bufp, size, " [<%s> ...]", locp); @@ -520,11 +539,13 @@ void nr_spell(stream *out, spellbook_entry * sbe, const struct locale *lang) 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 diff --git a/src/report.h b/src/report.h index 6379dbeeb..66eb4a4b9 100644 --- a/src/report.h +++ b/src/report.h @@ -28,6 +28,7 @@ extern "C" { 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 diff --git a/src/reports.test.c b/src/reports.test.c index 7b14680a5..c03726be4 100644 --- a/src/reports.test.c +++ b/src/reports.test.c @@ -287,47 +287,125 @@ static void test_write_unit(CuTest *tc) { test_cleanup(); } -static void test_write_spell_syntax(CuTest *tc) { - stream strm; - char buf[1024]; - char *line; - size_t len; +typedef struct { + struct locale *lang; spell *sp; spellbook *spb; spellbook_entry * sbe; - struct locale *lang; +} 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"); - test_cleanup(); + 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); - lang = get_or_create_locale("de"); - locale_setstring(lang, "spell::testspell", "Testzauber"); - locale_setstring(lang, "nr_spell_type", "Typ:"); - locale_setstring(lang, "sptype_normal", "Normal"); - locale_setstring(lang, "nr_spell_modifiers", "Modifier:"); - locale_setstring(lang, "smod_none", "Keine"); - locale_setstring(lang, keyword(K_CAST), "ZAUBERE"); +} + +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; - spb = create_spellbook("testbook"); - sp = test_create_spell(); - spellbook_add(spb, sp, 1); - sbe = spellbook_get(spb, sp); mstream_init(&strm); - nr_spell(&strm, sbe, lang); - stream_printf(&strm, "\n"); + + 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'; - line = strtok(buf, "\n"); - while (line && !strstr(line, "ZAUBERE")) line = strtok(NULL, "\n") ; + 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); - CuAssertTrue(tc, (bool) line); - CuAssertStrEquals(tc, " ZAUBERE \"Testzauber\"", line); mstream_done(&strm); +} + +static void set_parameter(spell_fixture spell, char *value) { + if (spell.sp->parameter) + strcpy(spell.sp->parameter, value); + else + 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 | SCHIFF | BURG )"); + + spell.sp->sptyp |= BUILDINGSPELL; + test_spell_syntax(tc, "kc typed", &spell, " ZAUBERE \"Testzauber\" BURG "); + spell.sp->sptyp = 0; + + set_parameter(spell, "b"); + test_spell_syntax(tc, "b", &spell, " ZAUBERE \"Testzauber\" "); + + set_parameter(spell, "s"); + test_spell_syntax(tc, "s", &spell, " ZAUBERE \"Testzauber\" "); + + set_parameter(spell, "s+"); + test_spell_syntax(tc, "s+", &spell, " ZAUBERE \"Testzauber\" [ ...]"); + + set_parameter(spell, "u"); + test_spell_syntax(tc, "u", &spell, " ZAUBERE \"Testzauber\" "); + + set_parameter(spell, "r"); + test_spell_syntax(tc, "r", &spell, " ZAUBERE \"Testzauber\" "); + + set_parameter(spell, "bc"); + spell.sp->syntax = _strdup("hodor"); + test_spell_syntax(tc, "bc hodor", &spell, " ZAUBERE \"Testzauber\" "); + free(spell.sp->syntax); + + /* no idea what ? is supposed to mean, optional parameter maybe? + set_parameter(spell, "kcc?"); + spell.sp->syntax = _strdup("hodor"); + test_spell_syntax(tc, "kcc?", &spell, " ZAUBERE \"Testzauber\" "); + free(spell.sp->syntax); + */ + + set_parameter(spell, "kc+"); + test_spell_syntax(tc, "kc+", &spell, + " ZAUBERE \"Testzauber\" ( REGION | EINHEIT [ ...] | SCHIFF \n [ ...] | BURG [ ...] )"); + test_cleanup(); } From f9c2994de14deb80bd60de488423c105109f1aab Mon Sep 17 00:00:00 2001 From: Steffen Mecke Date: Fri, 13 Nov 2015 15:35:17 +0100 Subject: [PATCH 4/4] fixing memory bug --- src/reports.test.c | 18 ++++++++++-------- src/tests.c | 2 ++ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/reports.test.c b/src/reports.test.c index c03726be4..40870c435 100644 --- a/src/reports.test.c +++ b/src/reports.test.c @@ -347,10 +347,8 @@ static void test_spell_syntax(CuTest *tc, char *msg, spell_fixture *spell, char } static void set_parameter(spell_fixture spell, char *value) { - if (spell.sp->parameter) - strcpy(spell.sp->parameter, value); - else - spell.sp->parameter = _strdup(value); + free(spell.sp->parameter); + spell.sp->parameter = _strdup(value); } static void test_write_spell_syntax(CuTest *tc) { @@ -391,15 +389,19 @@ static void test_write_spell_syntax(CuTest *tc) { test_spell_syntax(tc, "r", &spell, " ZAUBERE \"Testzauber\" "); set_parameter(spell, "bc"); + free(spell.sp->syntax); spell.sp->syntax = _strdup("hodor"); test_spell_syntax(tc, "bc hodor", &spell, " ZAUBERE \"Testzauber\" "); free(spell.sp->syntax); + spell.sp->syntax = 0; - /* no idea what ? is supposed to mean, optional parameter maybe? - set_parameter(spell, "kcc?"); - spell.sp->syntax = _strdup("hodor"); - test_spell_syntax(tc, "kcc?", &spell, " ZAUBERE \"Testzauber\" "); + /* 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\" []"); + free(spell.sp->syntax); + spell.sp->syntax = 0; */ set_parameter(spell, "kc+"); diff --git a/src/tests.c b/src/tests.c index 380eb7069..c8a526990 100644 --- a/src/tests.c +++ b/src/tests.c @@ -205,6 +205,8 @@ spell * test_create_spell(void) 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; }