From 6bf2ac34e3b4d880f4d9ed5e0e93723107b57da0 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 25 Sep 2017 22:04:16 +0200 Subject: [PATCH 01/27] test that whitespace is ignored when parsing orders --- src/kernel/order.h | 14 +++++++------- src/kernel/order.test.c | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/kernel/order.h b/src/kernel/order.h index bf4d02982..9ad3aabd8 100644 --- a/src/kernel/order.h +++ b/src/kernel/order.h @@ -41,17 +41,17 @@ extern "C" { } order; /* constructor */ - extern order *create_order(keyword_t kwd, const struct locale *lang, + order *create_order(keyword_t kwd, const struct locale *lang, const char *params, ...); - extern order *parse_order(const char *s, const struct locale *lang); - extern void replace_order(order ** dst, order * orig, const order * src); + order *parse_order(const char *s, const struct locale *lang); + void replace_order(order ** dst, order * orig, const order * src); /* reference counted copies of orders: */ - extern order *copy_order(const order * ord); - extern void free_order(order * ord); - extern void free_orders(order ** olist); + order *copy_order(const order * ord); + void free_order(order * ord); + void free_orders(order ** olist); - extern void push_order(struct order **olist, struct order *ord); + void push_order(struct order **olist, struct order *ord); /* access functions for orders */ keyword_t getkeyword(const order * ord); diff --git a/src/kernel/order.test.c b/src/kernel/order.test.c index 876d75b1e..124d0d889 100644 --- a/src/kernel/order.test.c +++ b/src/kernel/order.test.c @@ -49,24 +49,40 @@ static void test_parse_order(CuTest *tc) { ord = parse_order("!MOVE NORTH", lang); CuAssertPtrNotNull(tc, ord); + CuAssertPtrNotNull(tc, ord->data); + CuAssertIntEquals(tc, K_MOVE, getkeyword(ord)); CuAssertTrue(tc, ord->_noerror); CuAssertTrue(tc, !ord->_persistent); free_order(ord); ord = parse_order("@MOVE NORTH", lang); CuAssertPtrNotNull(tc, ord); + CuAssertPtrNotNull(tc, ord->data); + CuAssertIntEquals(tc, K_MOVE, getkeyword(ord)); CuAssertTrue(tc, !ord->_noerror); CuAssertTrue(tc, ord->_persistent); free_order(ord); ord = parse_order("@!MOVE NORTH", lang); CuAssertPtrNotNull(tc, ord); + CuAssertPtrNotNull(tc, ord->data); + CuAssertIntEquals(tc, K_MOVE, getkeyword(ord)); CuAssertTrue(tc, ord->_noerror); CuAssertTrue(tc, ord->_persistent); free_order(ord); ord = parse_order("!@MOVE NORTH", lang); CuAssertPtrNotNull(tc, ord); + CuAssertPtrNotNull(tc, ord->data); + CuAssertIntEquals(tc, K_MOVE, getkeyword(ord)); + CuAssertTrue(tc, ord->_noerror); + CuAssertTrue(tc, ord->_persistent); + free_order(ord); + + ord = parse_order(" !@MOVE NORTH", lang); + CuAssertPtrNotNull(tc, ord); + CuAssertPtrNotNull(tc, ord->data); + CuAssertIntEquals(tc, K_MOVE, getkeyword(ord)); CuAssertTrue(tc, ord->_noerror); CuAssertTrue(tc, ord->_persistent); free_order(ord); From 778f487afa0bb74d6a1bbc270bea497c026b28b6 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 25 Sep 2017 22:08:23 +0200 Subject: [PATCH 02/27] remove whitespace eating from parse_order. --- src/kernel/order.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/kernel/order.c b/src/kernel/order.c index d64da0131..6403889e3 100644 --- a/src/kernel/order.c +++ b/src/kernel/order.c @@ -386,35 +386,32 @@ order *parse_order(const char *s, const struct locale * lang) { assert(lang); assert(s); - while (*s && !isalnum(*(unsigned char *)s) && !ispunct(*(unsigned char *)s)) { - ++s; - } if (*s != 0) { - keyword_t kwd; - const char *sptr; + keyword_t kwd = NOKEYWORD; + const char *sptr = s; bool persistent = false, noerror = false; const char * p; - while (*s == '!' || *s=='@') { - if (*s=='!') noerror = true; - else if (*s == '@') persistent = true; - ++s; - } - sptr = s; p = *sptr ? parse_token_depr(&sptr) : 0; - kwd = p ? get_keyword(p, lang) : NOKEYWORD; + if (p) { + while (*p == '!' || *p == '@') { + if (*p == '!') noerror = true; + else if (*p == '@') persistent = true; + ++p; + } + kwd = get_keyword(p, lang); + } if (kwd == K_MAKE) { - const char *s, *sp = sptr; - s = parse_token_depr(&sp); - if (s && isparam(s, lang, P_TEMP)) { + const char *sp = sptr; + p = parse_token_depr(&sp); + if (p && isparam(p, lang, P_TEMP)) { kwd = K_MAKETEMP; sptr = sp; } } if (kwd != NOKEYWORD) { while (isspace(*(unsigned char *)sptr)) ++sptr; - s = sptr; - return create_order_i(kwd, s, persistent, noerror, lang); + return create_order_i(kwd, sptr, persistent, noerror, lang); } } return NULL; From 9a1295b4a5d81227c47be2e17ad3468436a43bc7 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 25 Sep 2017 22:12:49 +0200 Subject: [PATCH 03/27] push whitespace eating down the stack a bit. --- src/kernel/order.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kernel/order.c b/src/kernel/order.c index 6403889e3..f81d65b70 100644 --- a/src/kernel/order.c +++ b/src/kernel/order.c @@ -321,6 +321,7 @@ static order *create_order_i(keyword_t kwd, const char *sptr, bool persistent, ord->_noerror = noerror; ord->next = NULL; + while (isspace(*(unsigned char *)sptr)) ++sptr; ord->data = create_data(kwd, sptr, lindex); return ord; @@ -410,7 +411,6 @@ order *parse_order(const char *s, const struct locale * lang) } } if (kwd != NOKEYWORD) { - while (isspace(*(unsigned char *)sptr)) ++sptr; return create_order_i(kwd, sptr, persistent, noerror, lang); } } From e0514eddb59a691ae7b42143441d51f79080cf71 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Mon, 25 Sep 2017 22:15:13 +0200 Subject: [PATCH 04/27] let callers pass memory into create_order_i --- src/kernel/order.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/kernel/order.c b/src/kernel/order.c index f81d65b70..6d18f80c5 100644 --- a/src/kernel/order.c +++ b/src/kernel/order.c @@ -284,12 +284,12 @@ void close_orders(void) { } } -static order *create_order_i(keyword_t kwd, const char *sptr, bool persistent, +static order *create_order_i(order *ord, keyword_t kwd, const char *sptr, bool persistent, bool noerror, const struct locale *lang) { - order *ord = NULL; int lindex; + assert(ord); if (kwd == NOKEYWORD || keyword_disabled(kwd)) { log_error("trying to create an order for disabled keyword %s.", keyword(kwd)); return NULL; @@ -316,7 +316,6 @@ static order *create_order_i(keyword_t kwd, const char *sptr, bool persistent, } locale_array[lindex]->lang = lang; - ord = (order *)malloc(sizeof(order)); ord->_persistent = persistent; ord->_noerror = noerror; ord->next = NULL; @@ -330,6 +329,7 @@ static order *create_order_i(keyword_t kwd, const char *sptr, bool persistent, order *create_order(keyword_t kwd, const struct locale * lang, const char *params, ...) { + order *ord; char zBuffer[DISPLAYSIZE]; if (params) { char *bufp = zBuffer; @@ -380,7 +380,8 @@ order *create_order(keyword_t kwd, const struct locale * lang, else { zBuffer[0] = 0; } - return create_order_i(kwd, zBuffer, false, false, lang); + ord = (order *)malloc(sizeof(order)); + return create_order_i(ord, kwd, zBuffer, false, false, lang); } order *parse_order(const char *s, const struct locale * lang) @@ -411,7 +412,8 @@ order *parse_order(const char *s, const struct locale * lang) } } if (kwd != NOKEYWORD) { - return create_order_i(kwd, sptr, persistent, noerror, lang); + order *ord = (order *)malloc(sizeof(order)); + return create_order_i(ord, kwd, sptr, persistent, noerror, lang); } } return NULL; From 8b39133dbf8958cab1131555b6054ce1063b9646 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Tue, 26 Sep 2017 17:07:49 +0200 Subject: [PATCH 05/27] refactor: extract a read_order function. --- src/kernel/save.c | 86 +++++++++++++++++++++--------------------- src/kernel/save.h | 4 ++ src/kernel/save.test.c | 21 +++++++++++ 3 files changed, 68 insertions(+), 43 deletions(-) diff --git a/src/kernel/save.c b/src/kernel/save.c index eb072c03d..1c55db3ea 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -122,6 +122,41 @@ char *rns(FILE * f, char *c, size_t size) return c; } +struct order *read_order(const char *s, const struct locale *lang) { + assert(s); + assert(lang); + if (s[0]) { + keyword_t kwd; + char token[64]; + const char *stok; + + stok = parse_token(&s, token, sizeof(token)); + if (stok) { + param_t param = findparam(token, lang); + switch (param) { + case P_UNIT: + case P_REGION: + return NULL; + case P_FACTION: + case P_NEXT: + case P_GAMENAME: + /* these terminate the orders, so we apply extra checking */ + if (strlen(stok) >= 3) { + return NULL; + } + break; + default: + break; + } + } + /* Nun wird der Befehl erzeut und eingehängt */ + kwd = get_keyword(stok, lang); + if (kwd != NOKEYWORD) { + return create_order(kwd, lang, s); + } + } + return NULL; +} static unit *unitorders(FILE * F, int enc, struct faction *f) { @@ -164,6 +199,7 @@ static unit *unitorders(FILE * F, int enc, struct faction *f) for (;;) { const char *s; + order * ord; /* Erst wenn wir sicher sind, dass kein Befehl * eingegeben wurde, checken wir, ob nun eine neue * Einheit oder ein neuer Spieler drankommt */ @@ -172,49 +208,13 @@ static unit *unitorders(FILE * F, int enc, struct faction *f) if (s == NULL) break; - if (s[0]) { - if (s[0] != '@') { - char token[64]; - const char *stok = s; - stok = parse_token(&stok, token, sizeof(token)); - - if (stok) { - bool quit = false; - param_t param = findparam(stok, u->faction->locale); - switch (param) { - case P_UNIT: - case P_REGION: - quit = true; - break; - case P_FACTION: - case P_NEXT: - case P_GAMENAME: - /* these terminate the orders, so we apply extra checking */ - if (strlen(stok) >= 3) { - quit = true; - break; - } - else { - quit = false; - } - break; - default: - break; - } - if (quit) { - break; - } - } - } - /* Nun wird der Befehl erzeut und eingehängt */ - *ordp = parse_order(s, u->faction->locale); - if (*ordp) { - ordp = &(*ordp)->next; - } - else { - ADDMSG(&f->msgs, msg_message("parse_error", "unit command", u, s)); - } + ord = read_order(s, f->locale); + if (!ord) { + ADDMSG(&f->msgs, msg_message("parse_error", "unit command", u, s)); + break; } + *ordp = ord; + ordp = &(*ordp)->next; } } @@ -304,7 +304,7 @@ int readorders(const char *filename) } init_tokens_str(b); s = gettoken(token, sizeof(token)); - p = (s && s[0] != '@') ? findparam(s, lang) : NOPARAM; + p = (s && s[0] != '@' && s[0] != '!') ? findparam(s, lang) : NOPARAM; } while ((p != P_UNIT || !f) && p != P_FACTION && p != P_NEXT && p != P_GAMENAME); } diff --git a/src/kernel/save.h b/src/kernel/save.h index 763ff66b6..f706a311b 100644 --- a/src/kernel/save.h +++ b/src/kernel/save.h @@ -31,6 +31,8 @@ extern "C" { struct spellbook; struct unit; struct building; + struct order; + struct locale; struct faction; struct region; struct ship; @@ -43,6 +45,8 @@ extern "C" { /* TODO: is this *really* still in use? */ extern int enc_gamedata; + struct order *read_order(const char *s, const struct locale *lang); + int readorders(const char *filename); int readgame(const char *filename); int writegame(const char *filename); diff --git a/src/kernel/save.test.c b/src/kernel/save.test.c index 81d5a507d..f70d6ff07 100644 --- a/src/kernel/save.test.c +++ b/src/kernel/save.test.c @@ -5,6 +5,7 @@ #include #include "save.h" +#include "order.h" #include "version.h" #include "building.h" #include "ship.h" @@ -452,6 +453,25 @@ static void test_version_no(CuTest *tc) { CuAssertIntEquals(tc, 0x10203, version_no("1.2.3-what.is.42")); } +static void test_read_order(CuTest *tc) { + char cmd[32]; + order *ord; + struct locale * lang; + + test_setup(); + lang = test_create_locale(); + + ord = read_order("MOVE NORTH", lang); + CuAssertPtrNotNull(tc, ord); + CuAssertTrue(tc, !ord->_noerror); + CuAssertTrue(tc, !ord->_persistent); + CuAssertIntEquals(tc, K_MOVE, getkeyword(ord)); + CuAssertStrEquals(tc, "move NORTH", get_command(ord, cmd, sizeof(cmd))); + free_order(ord); + + test_cleanup(); +} + CuSuite *get_save_suite(void) { CuSuite *suite = CuSuiteNew(); @@ -468,6 +488,7 @@ CuSuite *get_save_suite(void) SUITE_ADD_TEST(suite, test_readwrite_dead_faction_group); SUITE_ADD_TEST(suite, test_read_password); SUITE_ADD_TEST(suite, test_read_password_external); + SUITE_ADD_TEST(suite, test_read_order); SUITE_ADD_TEST(suite, test_version_no); return suite; From 86e99a560c43de8f470aa897a335c3f32f12f850 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Tue, 26 Sep 2017 18:52:26 +0200 Subject: [PATCH 06/27] read_order must use parse_order (not create) to properly handle make temp. --- src/kernel/save.c | 9 +++++---- src/kernel/save.test.c | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/kernel/save.c b/src/kernel/save.c index 1c55db3ea..ed64b7bfd 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -122,10 +122,11 @@ char *rns(FILE * f, char *c, size_t size) return c; } -struct order *read_order(const char *s, const struct locale *lang) { - assert(s); +struct order *read_order(const char *in, const struct locale *lang) { + assert(in); assert(lang); - if (s[0]) { + if (in[0]) { + const char *s = in; keyword_t kwd; char token[64]; const char *stok; @@ -152,7 +153,7 @@ struct order *read_order(const char *s, const struct locale *lang) { /* Nun wird der Befehl erzeut und eingehängt */ kwd = get_keyword(stok, lang); if (kwd != NOKEYWORD) { - return create_order(kwd, lang, s); + return parse_order(in, lang); } } return NULL; diff --git a/src/kernel/save.test.c b/src/kernel/save.test.c index f70d6ff07..15600393a 100644 --- a/src/kernel/save.test.c +++ b/src/kernel/save.test.c @@ -469,6 +469,29 @@ static void test_read_order(CuTest *tc) { CuAssertStrEquals(tc, "move NORTH", get_command(ord, cmd, sizeof(cmd))); free_order(ord); + ord = read_order("MAKE TEMP foo", lang); + CuAssertPtrNotNull(tc, ord); + CuAssertTrue(tc, !ord->_noerror); + CuAssertTrue(tc, !ord->_persistent); + CuAssertIntEquals(tc, K_MAKETEMP, getkeyword(ord)); + CuAssertStrEquals(tc, "maketemp foo", get_command(ord, cmd, sizeof(cmd))); + free_order(ord); + + ord = read_order("MAKETEMP foo", lang); + CuAssertPtrNotNull(tc, ord); + CuAssertTrue(tc, !ord->_noerror); + CuAssertTrue(tc, !ord->_persistent); + CuAssertIntEquals(tc, K_MAKETEMP, getkeyword(ord)); + CuAssertStrEquals(tc, "maketemp foo", get_command(ord, cmd, sizeof(cmd))); + free_order(ord); + + CuAssertPtrEquals(tc, NULL, read_order("HODOR HODOR HODOR", lang)); + CuAssertPtrEquals(tc, NULL, read_order("FACTION abcd", lang)); + CuAssertPtrEquals(tc, NULL, read_order("UNIT abcd", lang)); + CuAssertPtrEquals(tc, NULL, read_order("ERESSEA abcd", lang)); + CuAssertPtrEquals(tc, NULL, read_order("REGION 2,3", lang)); + CuAssertPtrEquals(tc, NULL, read_order("NEXT", lang)); + test_cleanup(); } From 54fdda85cd50dfd65777758ec6a4cc16338c3058 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 30 Sep 2017 19:22:24 +0200 Subject: [PATCH 07/27] smallify order --- src/kernel/messages.c | 2 +- src/kernel/messages.test.c | 4 ++-- src/kernel/order.c | 14 +++++++------- src/kernel/order.h | 7 +++++-- src/kernel/order.test.c | 36 +++++++++++++++--------------------- src/kernel/save.test.c | 9 +++------ 6 files changed, 33 insertions(+), 39 deletions(-) diff --git a/src/kernel/messages.c b/src/kernel/messages.c index b8d33f08c..114a80559 100644 --- a/src/kernel/messages.c +++ b/src/kernel/messages.c @@ -83,7 +83,7 @@ struct message *msg_feedback(const struct unit *u, struct order *ord, variant var; memset(args, 0, sizeof(args)); - if (ord && ord->_noerror) { + if (ord && (ord->command & CMD_QUIET)) { return NULL; } diff --git a/src/kernel/messages.test.c b/src/kernel/messages.test.c index cd301756d..997abc704 100644 --- a/src/kernel/messages.test.c +++ b/src/kernel/messages.test.c @@ -79,8 +79,8 @@ static void test_noerror(CuTest *tc) { lang = test_create_locale(); u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL)); u->thisorder = parse_order("!@move", lang); - CuAssertTrue(tc, u->thisorder->_persistent); - CuAssertTrue(tc, u->thisorder->_noerror); + CuAssertIntEquals(tc, K_MOVE | CMD_QUIET | CMD_PERSIST, u->thisorder->command); + CuAssertTrue(tc, is_persistent(u->thisorder)); CuAssertPtrEquals(tc, NULL, msg_error(u, u->thisorder, 100)); CuAssertPtrEquals(tc, NULL, msg_feedback(u, u->thisorder, "error_unit_not_found", NULL)); test_cleanup(); diff --git a/src/kernel/order.c b/src/kernel/order.c index 6d18f80c5..5c760b188 100644 --- a/src/kernel/order.c +++ b/src/kernel/order.c @@ -94,7 +94,7 @@ char* get_command(const order *ord, char *sbuffer, size_t size) { keyword_t kwd = ORD_KEYWORD(ord); int bytes; - if (ord->_noerror) { + if (ord->command & CMD_QUIET) { if (size > 0) { *bufp++ = '!'; --size; @@ -103,7 +103,7 @@ char* get_command(const order *ord, char *sbuffer, size_t size) { WARN_STATIC_BUFFER(); } } - if (ord->_persistent) { + if (ord->command & CMD_PERSIST) { if (size > 0) { *bufp++ = '@'; --size; @@ -162,8 +162,7 @@ order *copy_order(const order * src) if (src != NULL) { order *ord = (order *)malloc(sizeof(order)); ord->next = NULL; - ord->_persistent = src->_persistent; - ord->_noerror = src->_noerror; + ord->command = src->command; ord->data = src->data; ++ord->data->_refcount; return ord; @@ -316,8 +315,9 @@ static order *create_order_i(order *ord, keyword_t kwd, const char *sptr, bool p } locale_array[lindex]->lang = lang; - ord->_persistent = persistent; - ord->_noerror = noerror; + ord->command = (int)kwd; + if (persistent) ord->command |= CMD_PERSIST; + if (noerror) ord->command |= CMD_QUIET; ord->next = NULL; while (isspace(*(unsigned char *)sptr)) ++sptr; @@ -560,7 +560,7 @@ bool is_persistent(const order * ord) case K_KOMMENTAR: return true; default: - return ord->_persistent || is_repeated(kwd); + return (ord->command & CMD_PERSIST) || is_repeated(kwd); } } diff --git a/src/kernel/order.h b/src/kernel/order.h index 9ad3aabd8..4911983f9 100644 --- a/src/kernel/order.h +++ b/src/kernel/order.h @@ -32,12 +32,15 @@ extern "C" { struct order_data; +#define CMD_QUIET 0x010000 +#define CMD_PERSIST 0x020000 +#define CMD_DEFAULT 0x040000 + typedef struct order { struct order *next; /* do not access this data: */ struct order_data *data; - bool _persistent; - bool _noerror; + int command; } order; /* constructor */ diff --git a/src/kernel/order.test.c b/src/kernel/order.test.c index 124d0d889..d813542d2 100644 --- a/src/kernel/order.test.c +++ b/src/kernel/order.test.c @@ -38,8 +38,7 @@ static void test_parse_order(CuTest *tc) { ord = parse_order("MOVE NORTH", lang); CuAssertPtrNotNull(tc, ord); - CuAssertTrue(tc, !ord->_noerror); - CuAssertTrue(tc, !ord->_persistent); + CuAssertIntEquals(tc, K_MOVE, ord->command); CuAssertIntEquals(tc, K_MOVE, getkeyword(ord)); CuAssertStrEquals(tc, "move NORTH", get_command(ord, cmd, sizeof(cmd))); @@ -51,40 +50,35 @@ static void test_parse_order(CuTest *tc) { CuAssertPtrNotNull(tc, ord); CuAssertPtrNotNull(tc, ord->data); CuAssertIntEquals(tc, K_MOVE, getkeyword(ord)); - CuAssertTrue(tc, ord->_noerror); - CuAssertTrue(tc, !ord->_persistent); + CuAssertIntEquals(tc, K_MOVE | CMD_QUIET, ord->command); free_order(ord); ord = parse_order("@MOVE NORTH", lang); CuAssertPtrNotNull(tc, ord); CuAssertPtrNotNull(tc, ord->data); CuAssertIntEquals(tc, K_MOVE, getkeyword(ord)); - CuAssertTrue(tc, !ord->_noerror); - CuAssertTrue(tc, ord->_persistent); - free_order(ord); - - ord = parse_order("@!MOVE NORTH", lang); - CuAssertPtrNotNull(tc, ord); - CuAssertPtrNotNull(tc, ord->data); - CuAssertIntEquals(tc, K_MOVE, getkeyword(ord)); - CuAssertTrue(tc, ord->_noerror); - CuAssertTrue(tc, ord->_persistent); + CuAssertIntEquals(tc, K_MOVE | CMD_PERSIST, ord->command); free_order(ord); ord = parse_order("!@MOVE NORTH", lang); CuAssertPtrNotNull(tc, ord); CuAssertPtrNotNull(tc, ord->data); CuAssertIntEquals(tc, K_MOVE, getkeyword(ord)); - CuAssertTrue(tc, ord->_noerror); - CuAssertTrue(tc, ord->_persistent); + CuAssertIntEquals(tc, K_MOVE | CMD_PERSIST | CMD_QUIET, ord->command); + free_order(ord); + + ord = parse_order("@!MOVE NORTH", lang); + CuAssertPtrNotNull(tc, ord); + CuAssertPtrNotNull(tc, ord->data); + CuAssertIntEquals(tc, K_MOVE, getkeyword(ord)); + CuAssertIntEquals(tc, K_MOVE | CMD_PERSIST | CMD_QUIET, ord->command); free_order(ord); ord = parse_order(" !@MOVE NORTH", lang); CuAssertPtrNotNull(tc, ord); CuAssertPtrNotNull(tc, ord->data); CuAssertIntEquals(tc, K_MOVE, getkeyword(ord)); - CuAssertTrue(tc, ord->_noerror); - CuAssertTrue(tc, ord->_persistent); + CuAssertIntEquals(tc, K_MOVE | CMD_PERSIST | CMD_QUIET, ord->command); free_order(ord); test_cleanup(); @@ -220,11 +214,11 @@ static void test_get_command(CuTest *tc) { lang = test_create_locale(); ord = create_order(K_MAKE, lang, "iron"); CuAssertStrEquals(tc, "make iron", get_command(ord, buf, sizeof(buf))); - ord->_noerror = true; + ord->command |= CMD_QUIET; CuAssertStrEquals(tc, "!make iron", get_command(ord, buf, sizeof(buf))); - ord->_persistent = true; + ord->command |= CMD_PERSIST; CuAssertStrEquals(tc, "!@make iron", get_command(ord, buf, sizeof(buf))); - ord->_noerror = false; + ord->command = K_MAKE | CMD_PERSIST; CuAssertStrEquals(tc, "@make iron", get_command(ord, buf, sizeof(buf))); free_order(ord); test_cleanup(); diff --git a/src/kernel/save.test.c b/src/kernel/save.test.c index 15600393a..73fe4663b 100644 --- a/src/kernel/save.test.c +++ b/src/kernel/save.test.c @@ -463,24 +463,21 @@ static void test_read_order(CuTest *tc) { ord = read_order("MOVE NORTH", lang); CuAssertPtrNotNull(tc, ord); - CuAssertTrue(tc, !ord->_noerror); - CuAssertTrue(tc, !ord->_persistent); + CuAssertIntEquals(tc, K_MOVE, ord->command); CuAssertIntEquals(tc, K_MOVE, getkeyword(ord)); CuAssertStrEquals(tc, "move NORTH", get_command(ord, cmd, sizeof(cmd))); free_order(ord); ord = read_order("MAKE TEMP foo", lang); CuAssertPtrNotNull(tc, ord); - CuAssertTrue(tc, !ord->_noerror); - CuAssertTrue(tc, !ord->_persistent); + CuAssertIntEquals(tc, K_MAKETEMP, ord->command); CuAssertIntEquals(tc, K_MAKETEMP, getkeyword(ord)); CuAssertStrEquals(tc, "maketemp foo", get_command(ord, cmd, sizeof(cmd))); free_order(ord); ord = read_order("MAKETEMP foo", lang); CuAssertPtrNotNull(tc, ord); - CuAssertTrue(tc, !ord->_noerror); - CuAssertTrue(tc, !ord->_persistent); + CuAssertIntEquals(tc, K_MAKETEMP, ord->command); CuAssertIntEquals(tc, K_MAKETEMP, getkeyword(ord)); CuAssertStrEquals(tc, "maketemp foo", get_command(ord, cmd, sizeof(cmd))); free_order(ord); From 5c63d20ff7e7897ec0b358cf0ae139790447c24b Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 30 Sep 2017 19:35:40 +0200 Subject: [PATCH 08/27] fix remaining bugs --- src/kernel/messages.test.c | 2 +- src/kernel/order.test.c | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/kernel/messages.test.c b/src/kernel/messages.test.c index 997abc704..a26530895 100644 --- a/src/kernel/messages.test.c +++ b/src/kernel/messages.test.c @@ -80,7 +80,7 @@ static void test_noerror(CuTest *tc) { u = test_create_unit(test_create_faction(NULL), test_create_region(0, 0, NULL)); u->thisorder = parse_order("!@move", lang); CuAssertIntEquals(tc, K_MOVE | CMD_QUIET | CMD_PERSIST, u->thisorder->command); - CuAssertTrue(tc, is_persistent(u->thisorder)); + CuAssertTrue(tc, !is_persistent(u->thisorder)); CuAssertPtrEquals(tc, NULL, msg_error(u, u->thisorder, 100)); CuAssertPtrEquals(tc, NULL, msg_feedback(u, u->thisorder, "error_unit_not_found", NULL)); test_cleanup(); diff --git a/src/kernel/order.test.c b/src/kernel/order.test.c index d813542d2..b42c7bf74 100644 --- a/src/kernel/order.test.c +++ b/src/kernel/order.test.c @@ -224,6 +224,29 @@ static void test_get_command(CuTest *tc) { test_cleanup(); } +static void test_is_persistent(CuTest *tc) { + order *ord; + struct locale *lang; + + test_setup(); + lang = test_create_locale(); + + ord = parse_order("@move", lang); + CuAssertIntEquals(tc, K_MOVE | CMD_PERSIST, ord->command); + CuAssertTrue(tc, !is_persistent(ord)); + free_order(ord); + + ord = parse_order("@invalid", lang); + CuAssertPtrEquals(tc, NULL, ord); + + ord = parse_order("// comment", lang); + CuAssertTrue(tc, is_persistent(ord)); + CuAssertIntEquals(tc, K_KOMMENTAR, ord->command); + free_order(ord); + + test_cleanup(); +} + CuSuite *get_order_suite(void) { CuSuite *suite = CuSuiteNew(); @@ -237,5 +260,6 @@ CuSuite *get_order_suite(void) SUITE_ADD_TEST(suite, test_skip_token); SUITE_ADD_TEST(suite, test_getstrtoken); SUITE_ADD_TEST(suite, test_get_command); + SUITE_ADD_TEST(suite, test_is_persistent); return suite; } From a067838fa0384f73a0d6f1241df679dc068e835c Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 30 Sep 2017 19:44:39 +0200 Subject: [PATCH 09/27] test all the edge cases. --- src/kernel/messages.c | 2 +- src/kernel/order.c | 5 +++++ src/kernel/order.h | 1 + src/kernel/order.test.c | 48 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/kernel/messages.c b/src/kernel/messages.c index 114a80559..f2a175269 100644 --- a/src/kernel/messages.c +++ b/src/kernel/messages.c @@ -83,7 +83,7 @@ struct message *msg_feedback(const struct unit *u, struct order *ord, variant var; memset(args, 0, sizeof(args)); - if (ord && (ord->command & CMD_QUIET)) { + if (ord && is_silent(ord)) { return NULL; } diff --git a/src/kernel/order.c b/src/kernel/order.c index 5c760b188..f40646a33 100644 --- a/src/kernel/order.c +++ b/src/kernel/order.c @@ -564,6 +564,11 @@ bool is_persistent(const order * ord) } } +bool is_silent(const order * ord) +{ + return (ord->command & CMD_QUIET) != 0; +} + char *write_order(const order * ord, char *buffer, size_t size) { if (ord == 0) { diff --git a/src/kernel/order.h b/src/kernel/order.h index 4911983f9..b9d235c88 100644 --- a/src/kernel/order.h +++ b/src/kernel/order.h @@ -61,6 +61,7 @@ extern "C" { void set_order(order ** destp, order * src); char* get_command(const order *ord, char *buffer, size_t size); bool is_persistent(const order * ord); + bool is_silent(const order * ord); bool is_exclusive(const order * ord); bool is_repeated(keyword_t kwd); bool is_long(keyword_t kwd); diff --git a/src/kernel/order.test.c b/src/kernel/order.test.c index b42c7bf74..71938ca7e 100644 --- a/src/kernel/order.test.c +++ b/src/kernel/order.test.c @@ -231,11 +231,57 @@ static void test_is_persistent(CuTest *tc) { test_setup(); lang = test_create_locale(); + ord = parse_order("@invalid", lang); + CuAssertPtrEquals(tc, NULL, ord); + + ord = parse_order("give", lang); + CuAssertIntEquals(tc, K_GIVE, ord->command); + CuAssertTrue(tc, !is_persistent(ord)); + free_order(ord); + + ord = parse_order("@give", lang); + CuAssertTrue(tc, !is_repeated(K_GIVE)); + CuAssertIntEquals(tc, K_GIVE | CMD_PERSIST, ord->command); + CuAssertTrue(tc, is_persistent(ord)); + free_order(ord); + + ord = parse_order("make", lang); + CuAssertTrue(tc, is_repeated(K_MAKE)); + CuAssertIntEquals(tc, K_MAKE , ord->command); + CuAssertTrue(tc, is_persistent(ord)); + free_order(ord); + ord = parse_order("@move", lang); CuAssertIntEquals(tc, K_MOVE | CMD_PERSIST, ord->command); CuAssertTrue(tc, !is_persistent(ord)); free_order(ord); + ord = parse_order("// comment", lang); + CuAssertTrue(tc, is_persistent(ord)); + CuAssertIntEquals(tc, K_KOMMENTAR, ord->command); + free_order(ord); + + test_cleanup(); +} + + +static void test_is_silent(CuTest *tc) { + order *ord; + struct locale *lang; + + test_setup(); + lang = test_create_locale(); + + ord = parse_order("make", lang); + CuAssertIntEquals(tc, K_MAKE, ord->command); + CuAssertTrue(tc, !is_silent(ord)); + free_order(ord); + + ord = parse_order("!make", lang); + CuAssertIntEquals(tc, K_MAKE | CMD_QUIET, ord->command); + CuAssertTrue(tc, is_silent(ord)); + free_order(ord); + ord = parse_order("@invalid", lang); CuAssertPtrEquals(tc, NULL, ord); @@ -246,7 +292,6 @@ static void test_is_persistent(CuTest *tc) { test_cleanup(); } - CuSuite *get_order_suite(void) { CuSuite *suite = CuSuiteNew(); @@ -261,5 +306,6 @@ CuSuite *get_order_suite(void) SUITE_ADD_TEST(suite, test_getstrtoken); SUITE_ADD_TEST(suite, test_get_command); SUITE_ADD_TEST(suite, test_is_persistent); + SUITE_ADD_TEST(suite, test_is_silent); return suite; } From e50c421b3f1762239e7929ecbd126c54c94a1fda Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 30 Sep 2017 20:08:22 +0200 Subject: [PATCH 10/27] can remove the keyword from order_data now --- src/kernel/order.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/kernel/order.c b/src/kernel/order.c index f40646a33..03af9be5d 100644 --- a/src/kernel/order.c +++ b/src/kernel/order.c @@ -30,7 +30,7 @@ #include #include -# define ORD_KEYWORD(ord) (ord)->data->_keyword +# define ORD_KEYWORD(ord) (keyword_t)((ord)->command & 0xFFFF) # define ORD_LOCALE(ord) locale_array[(ord)->data->_lindex]->lang # define ORD_STRING(ord) (ord)->data->_str @@ -46,7 +46,6 @@ typedef struct order_data { const char *_str; int _refcount; int _lindex; - keyword_t _keyword; } order_data; static void release_data(order_data * data) @@ -188,13 +187,12 @@ void free_orders(order ** olist) } } -static char *mkdata(order_data **pdata, size_t len, keyword_t kwd, int lindex, const char *str) +static char *mkdata(order_data **pdata, size_t len, int lindex, const char *str) { order_data *data; char *result; data = malloc(sizeof(order_data) + len + 1); result = (char *)(data + 1); - data->_keyword = kwd; data->_lindex = lindex; data->_refcount = 0; data->_str = 0; @@ -228,7 +226,7 @@ static order_data *create_data(keyword_t kwd, const char *sptr, int lindex) const char *skname = skillname(sk, lang); const char *spc = strchr(skname, ' '); size_t len = strlen(skname); - char *dst = mkdata(&data, len + (spc ? 3 : 0), kwd, lindex, spc ? 0 : skname); + char *dst = mkdata(&data, len + (spc ? 3 : 0), lindex, spc ? 0 : skname); locale_array[lindex]->study_orders[sk] = data; if (spc) { dst[0] = '\"'; @@ -247,14 +245,14 @@ static order_data *create_data(keyword_t kwd, const char *sptr, int lindex) else if (kwd != NOKEYWORD && *sptr == 0) { data = locale_array[lindex]->short_orders[kwd]; if (data == NULL) { - mkdata(&data, 0, kwd, lindex, 0); + mkdata(&data, 0, lindex, 0); data->_refcount = 1; locale_array[lindex]->short_orders[kwd] = data; } ++data->_refcount; return data; } - mkdata(&data, s ? strlen(s) : 0, kwd, lindex, s); + mkdata(&data, s ? strlen(s) : 0, lindex, s); data->_refcount = 1; return data; } @@ -599,5 +597,5 @@ keyword_t init_order(const struct order *ord) { assert(ord && ord->data); init_tokens_str(ord->data->_str); - return ord->data->_keyword; + return ORD_KEYWORD(ord); } From 03f46e35c4e28f2712d61a7806a37681b6ed8324 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 30 Sep 2017 20:09:30 +0200 Subject: [PATCH 11/27] one short order to rule them all. --- src/kernel/order.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/kernel/order.c b/src/kernel/order.c index 03af9be5d..a48825333 100644 --- a/src/kernel/order.c +++ b/src/kernel/order.c @@ -35,7 +35,7 @@ # define ORD_STRING(ord) (ord)->data->_str typedef struct locale_data { - struct order_data *short_orders[MAXKEYWORDS]; + struct order_data *short_orders; struct order_data *study_orders[MAXSKILLS]; const struct locale *lang; } locale_data; @@ -243,11 +243,11 @@ static order_data *create_data(keyword_t kwd, const char *sptr, int lindex) /* orders with no parameter, only one order_data per order required */ else if (kwd != NOKEYWORD && *sptr == 0) { - data = locale_array[lindex]->short_orders[kwd]; + data = locale_array[lindex]->short_orders; if (data == NULL) { mkdata(&data, 0, lindex, 0); data->_refcount = 1; - locale_array[lindex]->short_orders[kwd] = data; + locale_array[lindex]->short_orders = data; } ++data->_refcount; return data; @@ -259,10 +259,8 @@ static order_data *create_data(keyword_t kwd, const char *sptr, int lindex) static void clear_localedata(int lindex) { int i; - for (i = 0; i != MAXKEYWORDS; ++i) { - release_data(locale_array[lindex]->short_orders[i]); - locale_array[lindex]->short_orders[i] = 0; - } + release_data(locale_array[lindex]->short_orders); + locale_array[lindex]->short_orders = NULL; for (i = 0; i != MAXSKILLS; ++i) { release_data(locale_array[lindex]->study_orders[i]); locale_array[lindex]->study_orders[i] = 0; From 0f10b58167791b22890d590e0bd1ec073edc77c0 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sun, 1 Oct 2017 15:08:26 +0200 Subject: [PATCH 12/27] add tests and rewrite MAKE TEMP --- src/laws.c | 156 +++++++++++++++++++++++++++--------------------- src/laws.test.c | 65 ++++++++++++++++++++ 2 files changed, 154 insertions(+), 67 deletions(-) diff --git a/src/laws.c b/src/laws.c index d7f73fa6c..c1a30cac5 100644 --- a/src/laws.c +++ b/src/laws.c @@ -3007,10 +3007,92 @@ int checkunitnumber(const faction * f, int add) return 0; } +void maketemp_cmd(unit *u, order **olist) +{ + order *makeord; + int err = checkunitnumber(u->faction, 1); + + makeord = *olist; + if (err) { + if (err == 1) { + ADDMSG(&u->faction->msgs, + msg_feedback(u, makeord, + "too_many_units_in_alliance", + "allowed", maxunits(u->faction))); + } + else { + ADDMSG(&u->faction->msgs, + msg_feedback(u, makeord, + "too_many_units_in_faction", + "allowed", maxunits(u->faction))); + } + *olist = makeord->next; + makeord->next = NULL; + free_order(makeord); + while (*olist) { + keyword_t kwd; + order * ord = *olist; + *olist = ord->next; + ord->next = NULL; + kwd = getkeyword(ord); + free_order(ord); + if (kwd == K_END) { + break; + } + } + } + else { + char token[128]; + const char *s; + int alias; + ship *sh; + unit *u2; + order **ordp, **oinsert; +#ifndef NDEBUG + keyword_t kwd = init_order(makeord); + assert(kwd == K_MAKETEMP); +#endif + alias = getid(); + s = gettoken(token, sizeof(token)); + if (s && s[0] == '\0') { + /* empty name? => generate one */ + s = NULL; + } + u2 = create_unit(u->region, u->faction, 0, u->faction->race, alias, s, u); + fset(u2, UFL_ISNEW); + a_add(&u2->attribs, a_new(&at_alias))->data.i = alias; + sh = leftship(u); + if (sh) { + set_leftship(u2, sh); + } + setstatus(u2, u->status); + + /* copy orders until K_END from u to u2 */ + ordp = &makeord->next; + oinsert = &u2->orders; + + while (*ordp) { + order *ord = *ordp; + *ordp = ord->next; + if (getkeyword(ord) == K_END) { + ord->next = NULL; + free_order(ord); + break; + } + *oinsert = ord; + oinsert = &ord->next; + *oinsert = NULL; + } + *olist = *ordp; + makeord->next = NULL; + free_order(makeord); + } +} + void new_units(void) { region *r; - unit *u, *u2; + unit *u; /* neue einheiten werden gemacht und ihre befehle (bis zum "ende" zu * ihnen rueberkopiert, damit diese einheiten genauso wie die alten @@ -3028,73 +3110,13 @@ void new_units(void) } while (*ordp) { - order *makeord = *ordp; - if (getkeyword(makeord) == K_MAKETEMP) { - char token[128], *name = NULL; - const char *s; - int alias; - ship *sh; - order **newordersp; - int err = checkunitnumber(u->faction, 1); - - if (err) { - if (err == 1) { - ADDMSG(&u->faction->msgs, - msg_feedback(u, makeord, - "too_many_units_in_alliance", - "allowed", maxunits(u->faction))); - } - else { - ADDMSG(&u->faction->msgs, - msg_feedback(u, makeord, - "too_many_units_in_faction", - "allowed", maxunits(u->faction))); - } - ordp = &makeord->next; - - while (*ordp) { - order *ord = *ordp; - if (getkeyword(ord) == K_END) - break; - *ordp = ord->next; - ord->next = NULL; - free_order(ord); - } - continue; - } - init_order(makeord); - alias = getid(); - - s = gettoken(token, sizeof(token)); - if (s && s[0]) { - name = strdup(s); - } - u2 = create_unit(r, u->faction, 0, u->faction->race, alias, name, u); - if (name != NULL) - free(name); /* TODO: use a buffer on the stack instead? */ - fset(u2, UFL_ISNEW); - - a_add(&u2->attribs, a_new(&at_alias))->data.i = alias; - sh = leftship(u); - if (sh) { - set_leftship(u2, sh); - } - setstatus(u2, u->status); - - ordp = &makeord->next; - newordersp = &u2->orders; - while (*ordp) { - order *ord = *ordp; - if (getkeyword(ord) == K_END) - break; - *ordp = ord->next; - ord->next = NULL; - *newordersp = ord; - newordersp = &ord->next; - } + order *ord = *ordp; + if (getkeyword(ord) == K_MAKETEMP) { + maketemp_cmd(u, ordp); + } + else { + ordp = &ord->next; } - if (*ordp == makeord) - ordp = &makeord->next; } } } diff --git a/src/laws.test.c b/src/laws.test.c index 9f7884fc5..ee5c55f19 100644 --- a/src/laws.test.c +++ b/src/laws.test.c @@ -428,6 +428,63 @@ static void test_unit_limit(CuTest * tc) test_cleanup(); } +static void test_maketemp(CuTest * tc) +{ + faction *f; + unit *u, *u2; + + test_setup(); + f = test_create_faction(NULL); + u = test_create_unit(f, test_create_region(0, 0, NULL)); + + u->orders = create_order(K_MAKETEMP, f->locale, "1"); + u->orders->next = create_order(K_ENTERTAIN, f->locale, NULL); + u->orders->next->next = create_order(K_END, f->locale, NULL); + u->orders->next->next->next = create_order(K_TAX, f->locale, NULL); + + new_units(); + CuAssertIntEquals(tc, 2, f->num_units); + CuAssertPtrNotNull(tc, u2 = u->next); + CuAssertPtrNotNull(tc, u2->orders); + CuAssertPtrEquals(tc, NULL, u2->orders->next); + CuAssertIntEquals(tc, K_ENTERTAIN, getkeyword(u2->orders)); + + CuAssertPtrNotNull(tc, u->orders); + CuAssertPtrEquals(tc, NULL, u->orders->next); + CuAssertIntEquals(tc, K_TAX, getkeyword(u->orders)); + test_cleanup(); +} + +static void test_maketemp_default_order(CuTest * tc) +{ + faction *f; + unit *u, *u2; + + test_setup(); + config_set("orders.default", "work"); + f = test_create_faction(NULL); + u = test_create_unit(f, test_create_region(0, 0, NULL)); + + new_units(); + CuAssertIntEquals(tc, 1, f->num_units); + + u->orders = create_order(K_MAKETEMP, f->locale, "1"); + u->orders->next = create_order(K_END, f->locale, NULL); + u->orders->next->next = create_order(K_TAX, f->locale, NULL); + + new_units(); + CuAssertIntEquals(tc, 2, f->num_units); + CuAssertPtrNotNull(tc, u2 = u->next); + CuAssertPtrNotNull(tc, u2->orders); + CuAssertPtrEquals(tc, NULL, u2->orders->next); + CuAssertIntEquals(tc, K_WORK, getkeyword(u2->orders)); + + CuAssertPtrNotNull(tc, u->orders); + CuAssertPtrEquals(tc, NULL, u->orders->next); + CuAssertIntEquals(tc, K_TAX, getkeyword(u->orders)); + test_cleanup(); +} + static void test_limit_new_units(CuTest * tc) { faction *f; @@ -449,6 +506,8 @@ static void test_limit_new_units(CuTest * tc) CuAssertPtrNotNull(tc, u->next); CuAssertIntEquals(tc, 2, f->num_units); + CuAssertPtrEquals(tc, NULL, u->orders); + u->orders = create_order(K_MAKETEMP, f->locale, "1"); new_units(); CuAssertIntEquals(tc, 2, f->num_units); CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "too_many_units_in_faction")); @@ -458,6 +517,8 @@ static void test_limit_new_units(CuTest * tc) config_set("rules.limit.faction", "3"); config_set("rules.limit.alliance", "2"); + CuAssertPtrEquals(tc, NULL, u->orders); + u->orders = create_order(K_MAKETEMP, f->locale, "1"); new_units(); CuAssertIntEquals(tc, 2, f->num_units); CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "too_many_units_in_alliance")); @@ -466,6 +527,8 @@ static void test_limit_new_units(CuTest * tc) u = test_create_unit(test_create_faction(NULL), u->region); setalliance(u->faction, al); + CuAssertPtrEquals(tc, NULL, u->orders); + u->orders = create_order(K_MAKETEMP, f->locale, "1"); new_units(); CuAssertIntEquals(tc, 2, f->num_units); CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "too_many_units_in_alliance")); @@ -1555,6 +1618,8 @@ static void test_armedmen(CuTest *tc) { CuSuite *get_laws_suite(void) { CuSuite *suite = CuSuiteNew(); + SUITE_ADD_TEST(suite, test_maketemp_default_order); + SUITE_ADD_TEST(suite, test_maketemp); SUITE_ADD_TEST(suite, test_nmr_warnings); SUITE_ADD_TEST(suite, test_ally_cmd); SUITE_ADD_TEST(suite, test_name_cmd); From 02cfde0a49c0cb03b4d0aad3325783967f1395ec Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sun, 1 Oct 2017 17:08:05 +0200 Subject: [PATCH 13/27] fix create_unit default order memory leak --- src/kernel/unit.c | 8 -------- src/laws.c | 8 ++++++++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/kernel/unit.c b/src/kernel/unit.c index 2c6378ca1..a1ad74038 100644 --- a/src/kernel/unit.c +++ b/src/kernel/unit.c @@ -1492,14 +1492,6 @@ unit *create_unit(region * r, faction * f, int number, const struct race *urace, if (f) { assert(faction_alive(f)); u_setfaction(u, f); - - if (f->locale) { - order *deford = default_order(f->locale); - if (deford) { - set_order(&u->thisorder, NULL); - addlist(&u->orders, deford); - } - } } set_number(u, number); diff --git a/src/laws.c b/src/laws.c index c1a30cac5..e4c861177 100644 --- a/src/laws.c +++ b/src/laws.c @@ -3086,6 +3086,14 @@ void maketemp_cmd(unit *u, order **olist) *olist = *ordp; makeord->next = NULL; free_order(makeord); + + if (!u2->orders) { + order *deford = default_order(u2->faction->locale); + if (deford) { + set_order(&u2->thisorder, NULL); + addlist(&u2->orders, deford); + } + } } } From 4452f93009a68a2ed5d7217f11e2e1bf16ac1a41 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Thu, 5 Oct 2017 22:13:39 +0200 Subject: [PATCH 14/27] BUG 2367: add a test for set_familiar. --- src/kernel/unit.test.c | 25 +---------------------- src/magic.c | 8 +++++--- src/magic.test.c | 46 ++++++++++++++++++++++++++++++++++++++++++ src/test_eressea.c | 1 + 4 files changed, 53 insertions(+), 27 deletions(-) diff --git a/src/kernel/unit.test.c b/src/kernel/unit.test.c index 2e4ce3568..6ca15a7f0 100644 --- a/src/kernel/unit.test.c +++ b/src/kernel/unit.test.c @@ -287,33 +287,11 @@ static void test_skill_familiar(CuTest *tc) { test_cleanup(); } -static void test_age_familiar(CuTest *tc) { - unit *mag, *fam; - - test_cleanup(); - - mag = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); - fam = test_create_unit(mag->faction, test_create_region(0, 0, 0)); - CuAssertPtrEquals(tc, 0, get_familiar(mag)); - CuAssertPtrEquals(tc, 0, get_familiar_mage(fam)); - CuAssertIntEquals(tc, true, create_newfamiliar(mag, fam)); - CuAssertPtrEquals(tc, fam, get_familiar(mag)); - CuAssertPtrEquals(tc, mag, get_familiar_mage(fam)); - a_age(&fam->attribs, fam); - a_age(&mag->attribs, mag); - CuAssertPtrEquals(tc, fam, get_familiar(mag)); - CuAssertPtrEquals(tc, mag, get_familiar_mage(fam)); - set_number(fam, 0); - a_age(&mag->attribs, mag); - CuAssertPtrEquals(tc, 0, get_familiar(mag)); - test_cleanup(); -} - static void test_inside_building(CuTest *tc) { unit *u; building *b; - test_cleanup(); + test_setup(); u = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); b = test_create_building(u->region, 0); @@ -631,7 +609,6 @@ CuSuite *get_unit_suite(void) SUITE_ADD_TEST(suite, test_skillmod); SUITE_ADD_TEST(suite, test_skill_hunger); SUITE_ADD_TEST(suite, test_skill_familiar); - SUITE_ADD_TEST(suite, test_age_familiar); SUITE_ADD_TEST(suite, test_inside_building); SUITE_ADD_TEST(suite, test_skills); SUITE_ADD_TEST(suite, test_limited_skills); diff --git a/src/magic.c b/src/magic.c index d41ce71e4..210781e9e 100644 --- a/src/magic.c +++ b/src/magic.c @@ -2192,17 +2192,19 @@ void set_familiar(unit * mage, unit * familiar) a = a_add(&mage->attribs, a_new(&at_familiar)); a->data.v = familiar; } - else + else { assert(!a->data.v || a->data.v == familiar); - /* TODO: Diese Attribute beim Tod des Familiars entfernen: */ + } + /* TODO: Diese Attribute beim Tod des Familiars entfernen: */ a = a_find(familiar->attribs, &at_familiarmage); if (a == NULL) { a = a_add(&familiar->attribs, a_new(&at_familiarmage)); a->data.v = mage; } - else + else { assert(!a->data.v || a->data.v == mage); + } } void remove_familiar(unit * mage) diff --git a/src/magic.test.c b/src/magic.test.c index c6866ce47..0ce454af1 100644 --- a/src/magic.test.c +++ b/src/magic.test.c @@ -477,6 +477,52 @@ static void test_illusioncastle(CuTest *tc) test_cleanup(); } +static void test_familiar_set(CuTest *tc) { + unit *mag, *fam; + + test_setup(); + + mag = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); + fam = test_create_unit(mag->faction, test_create_region(0, 0, 0)); + CuAssertPtrEquals(tc, NULL, get_familiar(mag)); + CuAssertPtrEquals(tc, NULL, get_familiar_mage(fam)); + CuAssertPtrEquals(tc, NULL, a_find(mag->attribs, &at_skillmod)); + set_familiar(mag, fam); + CuAssertPtrEquals(tc, fam, get_familiar(mag)); + CuAssertPtrEquals(tc, mag, get_familiar_mage(fam)); + CuAssertPtrNotNull(tc, a_find(mag->attribs, &at_skillmod)); + test_cleanup(); +} + +static void test_familiar_age(CuTest *tc) { + unit *mag, *fam; + + test_setup(); + + mag = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); + fam = test_create_unit(mag->faction, test_create_region(0, 0, 0)); + set_familiar(mag, fam); + CuAssertPtrEquals(tc, fam, get_familiar(mag)); + CuAssertPtrEquals(tc, mag, get_familiar_mage(fam)); + a_age(&fam->attribs, fam); + a_age(&mag->attribs, mag); + CuAssertPtrEquals(tc, fam, get_familiar(mag)); + CuAssertPtrEquals(tc, mag, get_familiar_mage(fam)); + set_number(fam, 0); + a_age(&mag->attribs, mag); + CuAssertPtrEquals(tc, NULL, get_familiar(mag)); + test_cleanup(); +} + + +CuSuite *get_familiar_suite(void) +{ + CuSuite *suite = CuSuiteNew(); + SUITE_ADD_TEST(suite, test_familiar_set); + SUITE_ADD_TEST(suite, test_familiar_age); + return suite; +} + CuSuite *get_magic_suite(void) { CuSuite *suite = CuSuiteNew(); diff --git a/src/test_eressea.c b/src/test_eressea.c index 5361ddf5e..215bdd2c5 100644 --- a/src/test_eressea.c +++ b/src/test_eressea.c @@ -101,6 +101,7 @@ int RunAllTests(int argc, char *argv[]) ADD_SUITE(pool); ADD_SUITE(curse); ADD_SUITE(equipment); + ADD_SUITE(familiar); ADD_SUITE(item); ADD_SUITE(magic); ADD_SUITE(alchemy); From f00ff2d16e59ea7ab6be76ea22397c2efc00c811 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 6 Oct 2017 04:14:07 +0200 Subject: [PATCH 15/27] BUG 2368: additional space --- src/report.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/report.c b/src/report.c index 2dac94011..c23a9466c 100644 --- a/src/report.c +++ b/src/report.c @@ -1090,6 +1090,9 @@ void report_region(struct stream *out, const region * r, faction * f) message *msg; if (owner != NULL) { + bytes = (int)strlcpy(bufp, " ", size); + if (wrptr(&bufp, &size, bytes) != 0) + WARN_STATIC_BUFFER(); msg = msg_message("nr_region_owner", "faction", owner); bytes = (int)nr_render(msg, f->locale, bufp, size, f); msg_release(msg); From 9bfc0139f5f32fb550b25adeb16377fe8cb58a34 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 6 Oct 2017 17:32:56 +0200 Subject: [PATCH 16/27] set_familiar code reduction. remembering that at_skillmod is not persistent. --- src/magic.c | 33 +++------------------------------ 1 file changed, 3 insertions(+), 30 deletions(-) diff --git a/src/magic.c b/src/magic.c index 210781e9e..0961071b4 100644 --- a/src/magic.c +++ b/src/magic.c @@ -2181,10 +2181,8 @@ void set_familiar(unit * mage, unit * familiar) a = a->next; } if (a == NULL) { - attrib *an = a_add(&mage->attribs, a_new(&at_skillmod)); - skillmod_data *smd = (skillmod_data *)an->data.v; - smd->special = sm_familiar; - smd->skill = NOSKILL; + a = make_skillmod(NOSKILL, sm_familiar, 0.0, 0); + a_add(&mage->attribs, a); } a = a_find(mage->attribs, &at_familiar); @@ -2228,19 +2226,7 @@ void remove_familiar(unit * mage) bool create_newfamiliar(unit * mage, unit * familiar) { - /* if the skill modifier for the mage does not yet exist, add it */ - attrib *a; - attrib *afam = a_find(mage->attribs, &at_familiar); - attrib *amage = a_find(familiar->attribs, &at_familiarmage); - - if (afam == NULL) { - afam = a_add(&mage->attribs, a_new(&at_familiar)); - } - afam->data.v = familiar; - if (amage == NULL) { - amage = a_add(&familiar->attribs, a_new(&at_familiarmage)); - } - amage->data.v = mage; + set_familiar(mage, familiar); /* TODO: Diese Attribute beim Tod des Familiars entfernen: */ /* Wenn der Magier stirbt, dann auch der Vertraute */ @@ -2248,19 +2234,6 @@ bool create_newfamiliar(unit * mage, unit * familiar) /* Wenn der Vertraute stirbt, dann bekommt der Magier einen Schock */ add_trigger(&familiar->attribs, "destroy", trigger_shock(mage)); - a = a_find(mage->attribs, &at_skillmod); - while (a && a->type == &at_skillmod) { - skillmod_data *smd = (skillmod_data *)a->data.v; - if (smd->special == sm_familiar) - break; - a = a->next; - } - if (a == NULL) { - attrib *an = a_add(&mage->attribs, a_new(&at_skillmod)); - skillmod_data *smd = (skillmod_data *)an->data.v; - smd->special = sm_familiar; - smd->skill = NOSKILL; - } return true; } From 2ce94f2d47ef11af91aba68aa9a6ab0544f92fee Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 6 Oct 2017 21:23:58 +0200 Subject: [PATCH 17/27] BUG 2367: refactor create_newfamiliar. add first failing test for bug report. --- src/bind_unit.c | 10 ++++++++-- src/kernel/unit.test.c | 2 +- src/magic.c | 27 ++++++++++++++++++++------- src/magic.h | 2 +- src/magic.test.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/spells.c | 31 +++++++++++-------------------- src/util/attrib.c | 1 - 7 files changed, 82 insertions(+), 32 deletions(-) diff --git a/src/bind_unit.c b/src/bind_unit.c index 1d6579b0e..d95d9d930 100644 --- a/src/bind_unit.c +++ b/src/bind_unit.c @@ -638,8 +638,14 @@ static int tolua_unit_get_familiar(lua_State * L) static int tolua_unit_set_familiar(lua_State * L) { - unit *self = (unit *)tolua_tousertype(L, 1, 0); - create_newfamiliar(self, (unit *)tolua_tousertype(L, 2, 0)); + unit *mag = (unit *)tolua_tousertype(L, 1, NULL); + unit *fam = (unit *)tolua_tousertype(L, 2, NULL); + if (fam) { + set_familiar(mag, fam); + } + else { + remove_familiar(mag); + } return 0; } diff --git a/src/kernel/unit.test.c b/src/kernel/unit.test.c index 6ca15a7f0..1d2fc8124 100644 --- a/src/kernel/unit.test.c +++ b/src/kernel/unit.test.c @@ -274,7 +274,7 @@ static void test_skill_familiar(CuTest *tc) { CuAssertIntEquals(tc, 6, effskill(mag, SK_PERCEPTION, 0)); /* make them mage and familiar to each other */ - CuAssertIntEquals(tc, true, create_newfamiliar(mag, fam)); + create_newfamiliar(mag, fam); /* when they are in the same region, the mage gets half their skill as a bonus */ CuAssertIntEquals(tc, 6, effskill(fam, SK_PERCEPTION, 0)); diff --git a/src/magic.c b/src/magic.c index 0961071b4..21db1e672 100644 --- a/src/magic.c +++ b/src/magic.c @@ -42,6 +42,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. #include #include #include +#include #include #include #include @@ -2218,23 +2219,35 @@ void remove_familiar(unit * mage) while (a && a->type == &at_skillmod) { an = a->next; smd = (skillmod_data *)a->data.v; - if (smd->special == sm_familiar) + if (smd->special == sm_familiar) { a_remove(&mage->attribs, a); + } a = an; } } -bool create_newfamiliar(unit * mage, unit * familiar) +void create_newfamiliar(unit * mage, unit * fam) { - set_familiar(mage, familiar); + /* skills and spells: */ + const struct equipment *eq; + char eqname[64]; + const race *rc = u_race(fam); + set_familiar(mage, fam); + + snprintf(eqname, sizeof(eqname), "fam_%s", rc->_name); + eq = get_equipment(eqname); + if (eq != NULL) { + equip_items(&fam->items, eq); + } + else { + log_info("could not perform initialization for familiar %s.\n", rc->_name); + } /* TODO: Diese Attribute beim Tod des Familiars entfernen: */ /* Wenn der Magier stirbt, dann auch der Vertraute */ - add_trigger(&mage->attribs, "destroy", trigger_killunit(familiar)); + add_trigger(&mage->attribs, "destroy", trigger_killunit(fam)); /* Wenn der Vertraute stirbt, dann bekommt der Magier einen Schock */ - add_trigger(&familiar->attribs, "destroy", trigger_shock(mage)); - - return true; + add_trigger(&fam->attribs, "destroy", trigger_shock(mage)); } static void * resolve_familiar(int id, void *data) { diff --git a/src/magic.h b/src/magic.h index f5d2ff0f4..329a0012a 100644 --- a/src/magic.h +++ b/src/magic.h @@ -329,7 +329,7 @@ extern "C" { struct unit *get_clone(const struct unit *u); struct unit *get_clone_mage(const struct unit *u); void remove_familiar(struct unit *mage); - bool create_newfamiliar(struct unit *mage, struct unit *familiar); + void create_newfamiliar(struct unit *mage, struct unit *familiar); void create_newclone(struct unit *mage, struct unit *familiar); struct unit *has_clone(struct unit *mage); diff --git a/src/magic.test.c b/src/magic.test.c index 0ce454af1..b069dd165 100644 --- a/src/magic.test.c +++ b/src/magic.test.c @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -491,6 +492,9 @@ static void test_familiar_set(CuTest *tc) { CuAssertPtrEquals(tc, fam, get_familiar(mag)); CuAssertPtrEquals(tc, mag, get_familiar_mage(fam)); CuAssertPtrNotNull(tc, a_find(mag->attribs, &at_skillmod)); + remove_familiar(mag); + CuAssertPtrEquals(tc, NULL, get_familiar(mag)); + CuAssertPtrEquals(tc, NULL, a_find(mag->attribs, &at_skillmod)); test_cleanup(); } @@ -514,10 +518,47 @@ static void test_familiar_age(CuTest *tc) { test_cleanup(); } +static void test_familiar_equip(CuTest *tc) { + unit *mag, *u; + equipment *eq; + const item_type * itype; + spell *sp; + sc_mage * mage; + + test_setup(); + + itype = test_create_itemtype("horse"); + CuAssertPtrNotNull(tc, itype); + sp = create_spell("testspell"); + CuAssertPtrNotNull(tc, sp); + + eq = get_or_create_equipment("fam_human"); + equipment_setitem(eq, itype, "1"); + equipment_setskill(eq, SK_ENTERTAINMENT, "5"); + equipment_addspell(eq, sp->sname, 1); + + mag = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); + u = test_create_unit(mag->faction, test_create_region(0, 0, 0)); + set_familiar(mag, u); + create_newfamiliar(mag, u); + CuAssertIntEquals(tc, 1, i_get(u->items, itype)); + CuAssertIntEquals(tc, 5, get_level(u, SK_ENTERTAINMENT)); + CuAssertIntEquals(tc, 0, get_level(u, SK_MAGIC)); + + mage = get_mage(u); + CuAssertPtrNotNull(tc, mage); + CuAssertPtrNotNull(tc, mage->spellbook); + set_level(u, SK_MAGIC, 1); + CuAssertPtrEquals(tc, mage, get_mage(u)); + CuAssertTrue(tc, u_hasspell(u, sp)); + + test_cleanup(); +} CuSuite *get_familiar_suite(void) { CuSuite *suite = CuSuiteNew(); + SUITE_ADD_TEST(suite, test_familiar_equip); SUITE_ADD_TEST(suite, test_familiar_set); SUITE_ADD_TEST(suite, test_familiar_age); return suite; diff --git a/src/spells.c b/src/spells.c index 9d5481a60..0e9f5cffd 100644 --- a/src/spells.c +++ b/src/spells.c @@ -39,7 +39,6 @@ #include #include #include -#include #include #include #include @@ -513,27 +512,22 @@ static const race *select_familiar(const race * magerace, magic_t magiegebiet) /* ------------------------------------------------------------- */ /* der Vertraue des Magiers */ -static void make_familiar(unit * familiar, unit * mage) +static unit * make_familiar(unit * mage, region *r, const race *rc, const char *name) { - /* skills and spells: */ - const struct equipment *eq; - char eqname[64]; - const race * rc = u_race(familiar); - snprintf(eqname, sizeof(eqname), "fam_%s", rc->_name); - eq = get_equipment(eqname); - if (eq != NULL) { - equip_items(&familiar->items, eq); - } - else { - log_info("could not perform initialization for familiar %s.\n", rc->_name); - } + unit *fam; + + fam = create_unit(r, mage->faction, 1, rc, 0, name, mage); + setstatus(fam, ST_FLEE); + fset(fam, UFL_LOCKED); /* triggers: */ - create_newfamiliar(mage, familiar); + create_newfamiliar(mage, fam); /* Hitpoints nach Talenten korrigieren, sonst starten vertraute * mit Ausdauerbonus verwundet */ - familiar->hp = unit_max_hp(familiar); + fam->hp = unit_max_hp(fam); + + return fam; } static int sp_summon_familiar(castorder * co) @@ -586,10 +580,7 @@ static int sp_summon_familiar(castorder * co) msg = msg_message("familiar_name", "unit", mage); nr_render(msg, mage->faction->locale, zText, sizeof(zText), mage->faction); msg_release(msg); - familiar = create_unit(r, mage->faction, 1, rc, 0, zText, mage); - setstatus(familiar, ST_FLEE); - fset(familiar, UFL_LOCKED); - make_familiar(familiar, mage); + familiar = make_familiar(mage, r, rc, zText); dh = 0; dh1 = 0; diff --git a/src/util/attrib.c b/src/util/attrib.c index b3b9c0a6e..b6214fdd6 100644 --- a/src/util/attrib.c +++ b/src/util/attrib.c @@ -396,7 +396,6 @@ int a_age(attrib ** p, void *owner) static critbit_tree cb_deprecated = { 0 }; - typedef struct deprecated_s { unsigned int hash; int(*reader)(attrib *, void *, struct gamedata *); From f594a908dfda416a7e304009ac6a789f71d0ad6d Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 6 Oct 2017 21:53:07 +0200 Subject: [PATCH 18/27] deprecate old get_mage implementation. fix test_familiar_equip with new get_mage. --- src/bind_unit.c | 6 +++--- src/creport.c | 2 +- src/give.c | 2 +- src/kernel/equipment.c | 2 +- src/kernel/equipment.test.c | 2 +- src/kernel/save.c | 2 +- src/kernel/unit.c | 4 ++-- src/laws.c | 4 ++-- src/magic.c | 37 +++++++++++++++++++++++-------------- src/magic.h | 1 + src/magic.test.c | 2 +- src/move.c | 2 +- src/reports.c | 5 +++-- src/spells.c | 14 +++++++------- src/spy.c | 2 +- src/study.c | 6 +++--- src/study.test.c | 6 +++--- 17 files changed, 55 insertions(+), 44 deletions(-) diff --git a/src/bind_unit.c b/src/bind_unit.c index d95d9d930..1e1aee00e 100644 --- a/src/bind_unit.c +++ b/src/bind_unit.c @@ -273,7 +273,7 @@ static int tolua_unit_set_guard(lua_State * L) static const char *unit_getmagic(const unit * u) { - sc_mage *mage = get_mage(u); + sc_mage *mage = get_mage_depr(u); return mage ? magic_school[mage->magietyp] : NULL; } @@ -286,7 +286,7 @@ static int tolua_unit_get_magic(lua_State * L) static void unit_setmagic(unit * u, const char *type) { - sc_mage *mage = get_mage(u); + sc_mage *mage = get_mage_depr(u); int mtype; for (mtype = 0; mtype != MAXMAGIETYP; ++mtype) { if (strcmp(magic_school[mtype], type) == 0) @@ -753,7 +753,7 @@ static int tolua_unit_get_items(lua_State * L) static int tolua_unit_get_spells(lua_State * L) { unit *self = (unit *) tolua_tousertype(L, 1, 0); - sc_mage *mage = self ? get_mage(self) : 0; + sc_mage *mage = self ? get_mage_depr(self) : 0; spellbook *sb = mage ? mage->spellbook : 0; selist *slist = 0; if (sb) { diff --git a/src/creport.c b/src/creport.c index b68f3e4fe..4ec7f9b0b 100644 --- a/src/creport.c +++ b/src/creport.c @@ -954,7 +954,7 @@ void cr_output_unit(stream *out, const region * r, const faction * f, } /* spells that this unit can cast */ - mage = get_mage(u); + mage = get_mage_depr(u); if (mage) { int i, maxlevel = effskill(u, SK_MAGIC, 0); cr_output_spells(out, u, maxlevel); diff --git a/src/give.c b/src/give.c index 18d355031..da75e56c5 100644 --- a/src/give.c +++ b/src/give.c @@ -591,7 +591,7 @@ void give_unit(unit * u, unit * u2, order * ord) cmistake(u, ord, 155, MSG_COMMERCE); return; } - mage = get_mage(u); + mage = get_mage_depr(u); if (!mage || u2->faction->magiegebiet != mage->magietyp) { cmistake(u, ord, 157, MSG_COMMERCE); return; diff --git a/src/kernel/equipment.c b/src/kernel/equipment.c index b87f401dd..f24d4acfd 100644 --- a/src/kernel/equipment.c +++ b/src/kernel/equipment.c @@ -121,7 +121,7 @@ void equip_unit_mask(struct unit *u, const struct equipment *eq, int mask) if (eq->spells) { selist * ql = eq->spells; int qi; - sc_mage * mage = get_mage(u); + sc_mage * mage = get_mage_depr(u); for (qi = 0; ql; selist_advance(&ql, &qi, 1)) { lazy_spell *sbe = (lazy_spell *)selist_get(ql, qi); diff --git a/src/kernel/equipment.test.c b/src/kernel/equipment.test.c index 7952001fe..0e0ebf196 100644 --- a/src/kernel/equipment.test.c +++ b/src/kernel/equipment.test.c @@ -39,7 +39,7 @@ static void test_equipment(CuTest * tc) CuAssertIntEquals(tc, 1, i_get(u->items, it_horses)); CuAssertIntEquals(tc, 5, get_level(u, SK_MAGIC)); - mage = get_mage(u); + mage = get_mage_depr(u); CuAssertPtrNotNull(tc, mage); CuAssertPtrNotNull(tc, mage->spellbook); CuAssertTrue(tc, u_hasspell(u, sp)); diff --git a/src/kernel/save.c b/src/kernel/save.c index ed64b7bfd..23d6f3e71 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -1791,7 +1791,7 @@ int read_game(gamedata *data) else { for (u = f->units; u; u = u->nextF) { if (data->version < SPELL_LEVEL_VERSION) { - sc_mage *mage = get_mage(u); + sc_mage *mage = get_mage_depr(u); if (mage) { faction *f = u->faction; int skl = effskill(u, SK_MAGIC, 0); diff --git a/src/kernel/unit.c b/src/kernel/unit.c index a1ad74038..674bdad08 100644 --- a/src/kernel/unit.c +++ b/src/kernel/unit.c @@ -1814,7 +1814,7 @@ void u_setrace(struct unit *u, const struct race *rc) void unit_add_spell(unit * u, sc_mage * m, struct spell * sp, int level) { - sc_mage *mage = m ? m : get_mage(u); + sc_mage *mage = m ? m : get_mage_depr(u); if (!mage) { log_debug("adding new spell %s to a previously non-mage unit %s\n", sp->sname, unitname(u)); @@ -1828,7 +1828,7 @@ void unit_add_spell(unit * u, sc_mage * m, struct spell * sp, int level) struct spellbook * unit_get_spellbook(const struct unit * u) { - sc_mage * mage = get_mage(u); + sc_mage * mage = get_mage_depr(u); if (mage) { if (mage->spellbook) { return mage->spellbook; diff --git a/src/laws.c b/src/laws.c index e4c861177..f60af7f15 100644 --- a/src/laws.c +++ b/src/laws.c @@ -3387,7 +3387,7 @@ static int faction_getmages(faction * f, unit ** results, int numresults) for (u = f->units; u; u = u->nextF) { if (u->number > 0) { - sc_mage *mage = get_mage(u); + sc_mage *mage = get_mage_depr(u); if (mage) { int level = effskill(u, SK_MAGIC, 0); if (level > maxlevel) { @@ -3446,7 +3446,7 @@ static void update_spells(void) show_new_spells(f, maxlevel, faction_get_spellbook(f)); for (i = 0; i != MAXMAGES && mages[i]; ++i) { unit * u = mages[i]; - sc_mage *mage = get_mage(u); + sc_mage *mage = get_mage_depr(u); if (mage && mage->spellbook) { int level = effskill(u, SK_MAGIC, 0); show_new_spells(f, level, mage->spellbook); diff --git a/src/magic.c b/src/magic.c index 21db1e672..7ec220875 100644 --- a/src/magic.c +++ b/src/magic.c @@ -323,10 +323,19 @@ attrib_type at_mage = { bool is_mage(const unit * u) { - return get_mage(u) != NULL; + return get_mage_depr(u) != NULL; } sc_mage *get_mage(const unit * u) +{ + attrib *a = a_find(u->attribs, &at_mage); + if (a) { + return (sc_mage *)a->data.v; + } + return NULL; +} + +sc_mage *get_mage_depr(const unit * u) { if (has_skill(u, SK_MAGIC)) { attrib *a = a_find(u->attribs, &at_mage); @@ -334,7 +343,7 @@ sc_mage *get_mage(const unit * u) return (sc_mage *)a->data.v; } } - return (sc_mage *)NULL; + return NULL; } /* ------------------------------------------------------------- */ @@ -507,7 +516,7 @@ int u_hasspell(const unit *u, const struct spell *sp) int get_combatspelllevel(const unit * u, int nr) { - sc_mage *m = get_mage(u); + sc_mage *m = get_mage_depr(u); assert(nr < MAXCOMBATSPELLS); if (m) { @@ -525,7 +534,7 @@ const spell *get_combatspell(const unit * u, int nr) sc_mage *m; assert(nr < MAXCOMBATSPELLS); - m = get_mage(u); + m = get_mage_depr(u); if (m) { return m->combatspells[nr].sp; } @@ -534,7 +543,7 @@ const spell *get_combatspell(const unit * u, int nr) void set_combatspell(unit * u, spell * sp, struct order *ord, int level) { - sc_mage *mage = get_mage(u); + sc_mage *mage = get_mage_depr(u); int i = -1; assert(mage || !"trying to set a combat spell for non-mage"); @@ -574,7 +583,7 @@ void unset_combatspell(unit * u, spell * sp) int nr = 0; int i; - m = get_mage(u); + m = get_mage_depr(u); if (!m) return; @@ -610,7 +619,7 @@ int get_spellpoints(const unit * u) { sc_mage *m; - m = get_mage(u); + m = get_mage_depr(u); if (!m) return 0; @@ -621,7 +630,7 @@ void set_spellpoints(unit * u, int sp) { sc_mage *m; - m = get_mage(u); + m = get_mage_depr(u); if (!m) return; @@ -638,7 +647,7 @@ int change_spellpoints(unit * u, int mp) sc_mage *m; int sp; - m = get_mage(u); + m = get_mage_depr(u); if (!m) { return 0; } @@ -657,7 +666,7 @@ static int get_spchange(const unit * u) { sc_mage *m; - m = get_mage(u); + m = get_mage_depr(u); if (!m) return 0; @@ -711,7 +720,7 @@ int change_maxspellpoints(unit * u, int csp) { sc_mage *m; - m = get_mage(u); + m = get_mage_depr(u); if (!m) { return 0; } @@ -729,7 +738,7 @@ int countspells(unit * u, int step) sc_mage *m; int count; - m = get_mage(u); + m = get_mage_depr(u); if (!m) return 0; @@ -1313,7 +1322,7 @@ bool fumble(region * r, unit * u, const spell * sp, int cast_grade) } /* CHAOSPATZERCHANCE 10 : +10% Chance zu Patzern */ - mage = get_mage(u); + mage = get_mage_depr(u); if (mage->magietyp == M_DRAIG) { fumble_chance += CHAOSPATZERCHANCE; } @@ -2238,7 +2247,7 @@ void create_newfamiliar(unit * mage, unit * fam) snprintf(eqname, sizeof(eqname), "fam_%s", rc->_name); eq = get_equipment(eqname); if (eq != NULL) { - equip_items(&fam->items, eq); + equip_unit(fam, eq); } else { log_info("could not perform initialization for familiar %s.\n", rc->_name); diff --git a/src/magic.h b/src/magic.h index 329a0012a..403dafdc6 100644 --- a/src/magic.h +++ b/src/magic.h @@ -221,6 +221,7 @@ extern "C" { /* macht die struct unit zu einem neuen Magier: legt die struct u->mage an * und initialisiert den Magiertypus mit mtyp. */ sc_mage *get_mage(const struct unit *u); + sc_mage *get_mage_depr(const struct unit *u); /* gibt u->mage zurück, bei nicht-Magiern *NULL */ bool is_mage(const struct unit *u); /* gibt true, wenn u->mage gesetzt. */ diff --git a/src/magic.test.c b/src/magic.test.c index b069dd165..eea9ec575 100644 --- a/src/magic.test.c +++ b/src/magic.test.c @@ -549,7 +549,7 @@ static void test_familiar_equip(CuTest *tc) { CuAssertPtrNotNull(tc, mage); CuAssertPtrNotNull(tc, mage->spellbook); set_level(u, SK_MAGIC, 1); - CuAssertPtrEquals(tc, mage, get_mage(u)); + CuAssertPtrEquals(tc, mage, get_mage_depr(u)); CuAssertTrue(tc, u_hasspell(u, sp)); test_cleanup(); diff --git a/src/move.c b/src/move.c index 33ed38126..2b0e4d8d7 100644 --- a/src/move.c +++ b/src/move.c @@ -1451,7 +1451,7 @@ static int movement_speed(unit * u) * Nicht kumulativ mit anderen Beschleunigungen! */ if (mp * dk <= BP_WALKING * u_race(u)->speed && is_astral(u->region) && is_mage(u)) { - sc_mage *mage = get_mage(u); + sc_mage *mage = get_mage_depr(u); if (mage->magietyp == M_TYBIED || mage->magietyp == M_ILLAUN) { mp *= 2; } diff --git a/src/reports.c b/src/reports.c index 1182f89c9..3d762a4ce 100644 --- a/src/reports.c +++ b/src/reports.c @@ -819,8 +819,9 @@ spskill(char *buffer, size_t size, const struct locale * lang, bufp = STRLCPY(bufp, " ", size); if (sv->id == SK_MAGIC) { - sc_mage *mage = get_mage(u); - if (mage && mage->magietyp != M_GRAY) { + sc_mage *mage = get_mage_depr(u); + assert(mage); + if (mage->magietyp != M_GRAY) { bufp = STRLCPY(bufp, LOC(lang, mkname("school", magic_school[mage->magietyp])), size); bufp = STRLCPY(bufp, " ", size); diff --git a/src/spells.c b/src/spells.c index 0e9f5cffd..526761169 100644 --- a/src/spells.c +++ b/src/spells.c @@ -731,7 +731,7 @@ static int sp_transferaura(castorder * co) int cast_level = co->level; spellparameter *pa = co->par; unit *u; - sc_mage *scm_dst, *scm_src = get_mage(mage); + sc_mage *scm_dst, *scm_src = get_mage_depr(mage); /* wenn kein Ziel gefunden, Zauber abbrechen */ if (pa->param[0]->flag == TARGET_NOTFOUND) @@ -745,7 +745,7 @@ static int sp_transferaura(castorder * co) /* Wieviel Transferieren? */ aura = pa->param[1]->data.i; u = pa->param[0]->data.u; - scm_dst = get_mage(u); + scm_dst = get_mage_depr(u); if (scm_dst == NULL) { /* "Zu dieser Einheit kann ich keine Aura uebertragen." */ @@ -5813,7 +5813,7 @@ int sp_permtransfer(castorder * co) change_maxspellpoints(mage, -aura); change_spellpoints(mage, -aura); - if (get_mage(tu)->magietyp == get_mage(mage)->magietyp) { + if (get_mage_depr(tu)->magietyp == get_mage_depr(mage)->magietyp) { change_maxspellpoints(tu, aura / 2); } else { @@ -5936,18 +5936,18 @@ int sp_stealaura(castorder * co) /* Zieleinheit */ u = pa->param[0]->data.u; - if (!get_mage(u)) { + if (!get_mage_depr(u)) { ADDMSG(&mage->faction->msgs, msg_message("stealaura_fail", "unit target", mage, u)); ADDMSG(&u->faction->msgs, msg_message("stealaura_fail_detect", "unit", u)); return 0; } - taura = (get_mage(u)->spellpoints * (rng_int() % (int)(3 * power) + 1)) / 100; + taura = (get_mage_depr(u)->spellpoints * (rng_int() % (int)(3 * power) + 1)) / 100; if (taura > 0) { - get_mage(u)->spellpoints -= taura; - get_mage(mage)->spellpoints += taura; + get_mage_depr(u)->spellpoints -= taura; + get_mage_depr(mage)->spellpoints += taura; /* sprintf(buf, "%s entzieht %s %d Aura.", unitname(mage), unitname(u), taura); */ ADDMSG(&mage->faction->msgs, msg_message("stealaura_success", diff --git a/src/spy.c b/src/spy.c index 837bc28ee..210be63e4 100644 --- a/src/spy.c +++ b/src/spy.c @@ -68,7 +68,7 @@ void spy_message(int spy, const unit * u, const unit * target) ADDMSG(&u->faction->msgs, msg_message("spyreport", "spy target status", u, target, status)); if (spy > 20) { - sc_mage *mage = get_mage(target); + sc_mage *mage = get_mage_depr(target); /* for mages, spells and magic school */ if (mage) { ADDMSG(&u->faction->msgs, msg_message("spyreport_mage", "spy target type", u, diff --git a/src/study.c b/src/study.c index 6949db22e..88550fc30 100644 --- a/src/study.c +++ b/src/study.c @@ -427,8 +427,8 @@ int teach_cmd(unit * teacher, struct order *ord) if (sk == SK_MAGIC) { /* ist der Magier schon spezialisiert, so versteht er nur noch * Lehrer seines Gebietes */ - sc_mage *mage1 = get_mage(teacher); - sc_mage *mage2 = get_mage(student); + sc_mage *mage1 = get_mage_depr(teacher); + sc_mage *mage2 = get_mage_depr(student); if (mage2 && mage1 && mage2->magietyp != M_GRAY && mage1->magietyp != mage2->magietyp) { if (feedback) { @@ -782,7 +782,7 @@ int study_cmd(unit * u, order * ord) } } else if (sk == SK_MAGIC) { - sc_mage *mage = get_mage(u); + sc_mage *mage = get_mage_depr(u); if (!mage) { mage = create_mage(u, u->faction->magiegebiet); } diff --git a/src/study.test.c b/src/study.test.c index 21f76ad4c..75c169b74 100644 --- a/src/study.test.c +++ b/src/study.test.c @@ -409,11 +409,11 @@ static void test_study_magic(CuTest *tc) { study_cmd(u, u->thisorder); CuAssertIntEquals(tc, M_GWYRRD, f->magiegebiet); CuAssertIntEquals(tc, 0, i_get(u->items, rtype->itype)); - CuAssertPtrNotNull(tc, get_mage(u)); + CuAssertPtrNotNull(tc, get_mage_depr(u)); CuAssertPtrEquals(tc, 0, test_find_messagetype(f->msgs, "error65")); - CuAssertIntEquals(tc, M_GWYRRD, get_mage(u)->magietyp); + CuAssertIntEquals(tc, M_GWYRRD, get_mage_depr(u)->magietyp); - /* the static cost array in study_cost prevents this test: + /* TODO: the static cost array in study_cost prevents this test: test_clear_messages(f); config_set("skills.cost.magic", "50"); i_change(&u->items, rtype->itype, 50); From c73af8b89bf8ba91aa1527ef2bd4115630d98bae Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 6 Oct 2017 22:01:02 +0200 Subject: [PATCH 19/27] wrap some tests around is_mage and get_mage --- src/magic.test.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/magic.test.c b/src/magic.test.c index eea9ec575..a10c02b7b 100644 --- a/src/magic.test.c +++ b/src/magic.test.c @@ -478,6 +478,40 @@ static void test_illusioncastle(CuTest *tc) test_cleanup(); } +static void test_is_mage(CuTest *tc) { + unit *u; + sc_mage *mage; + + test_setup(); + u = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); + CuAssertPtrEquals(tc, NULL, get_mage(u)); + CuAssertTrue(tc, !is_mage(u)); + set_level(u, SK_MAGIC, 1); + CuAssertTrue(tc, !is_mage(u)); + CuAssertPtrEquals(tc, NULL, get_mage(u)); + CuAssertPtrNotNull(tc, mage = create_mage(u, M_CERDDOR)); + CuAssertPtrEquals(tc, mage, get_mage(u)); + CuAssertTrue(tc, is_mage(u)); + test_cleanup(); +} + +static void test_get_mage(CuTest *tc) { + unit *u; + sc_mage *mage; + + test_setup(); + u = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); + CuAssertPtrEquals(tc, NULL, get_mage(u)); + CuAssertPtrEquals(tc, NULL, get_mage_depr(u)); + CuAssertPtrNotNull(tc, mage = create_mage(u, M_CERDDOR)); + CuAssertPtrEquals(tc, mage, get_mage(u)); + CuAssertPtrEquals(tc, NULL, get_mage_depr(u)); + set_level(u, SK_MAGIC, 1); + CuAssertPtrEquals(tc, mage, get_mage(u)); + CuAssertPtrEquals(tc, mage, get_mage_depr(u)); + test_cleanup(); +} + static void test_familiar_set(CuTest *tc) { unit *mag, *fam; @@ -567,6 +601,8 @@ CuSuite *get_familiar_suite(void) CuSuite *get_magic_suite(void) { CuSuite *suite = CuSuiteNew(); + SUITE_ADD_TEST(suite, test_is_mage); + SUITE_ADD_TEST(suite, test_get_mage); SUITE_ADD_TEST(suite, test_multi_cast); SUITE_ADD_TEST(suite, test_updatespells); SUITE_ADD_TEST(suite, test_spellbooks); From 6265abac746e27a1fdec3d5abf34483963d796b1 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 6 Oct 2017 22:15:48 +0200 Subject: [PATCH 20/27] remove side-effects from movement_speed(). --- src/move.c | 85 +++++++++++++++++++++---------------------------- src/move.h | 9 ++++++ src/move.test.c | 24 ++++++++++++++ src/tests.c | 2 +- 4 files changed, 70 insertions(+), 50 deletions(-) diff --git a/src/move.c b/src/move.c index 2b0e4d8d7..46a8f200f 100644 --- a/src/move.c +++ b/src/move.c @@ -89,14 +89,6 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. #include #include -/* Bewegungsweiten: */ -#define BP_WALKING 4 -#define BP_RIDING 6 -#define BP_UNICORN 9 -#define BP_DRAGON 4 -#define BP_NORMAL 3 -#define BP_ROAD 2 - int *storms; typedef struct traveldir { @@ -265,7 +257,7 @@ get_transporters(const item * itm, int *p_animals, int *p_acap, int *p_vehicles, *p_acap = acap; } -static int ridingcapacity(unit * u) +static int ridingcapacity(const unit * u) { int vehicles = 0, vcap = 0; int animals = 0, acap = 0; @@ -433,7 +425,7 @@ bool canswim(unit * u) return false; } -static int canride(unit * u) +static int canride(const unit * u) { int horses = 0, maxhorses, unicorns = 0, maxunicorns; int skill = effskill(u, SK_RIDING, 0); @@ -460,17 +452,17 @@ static int canride(unit * u) if (!(u_race(u)->flags & RCF_HORSE) && ((horses == 0 && unicorns == 0) || horses > maxhorses || unicorns > maxunicorns)) { - return 0; + return BP_WALKING; } if (ridingcapacity(u) - eff_weight(u) >= 0) { if (horses == 0 && unicorns >= u->number && !(u_race(u)->flags & RCF_HORSE)) { - return 2; + return BP_UNICORN; } - return 1; + return BP_RIDING; } - return 0; + return BP_WALKING; } static bool cansail(const region * r, ship * sh) @@ -1399,9 +1391,9 @@ static void make_route(unit * u, order * ord, region_list ** routep) * Normalerweise verliert man 3 BP pro Region, bei Straßen nur 2 BP. * Außerdem: Wenn Einheit transportiert, nur halbe BP */ -static int movement_speed(unit * u) +int movement_speed(const unit * u) { - int mp = BP_WALKING; + int mp = 0; const race *rc = u_race(u); double dk = rc->speed; assert(u->number); @@ -1415,6 +1407,10 @@ static int movement_speed(unit * u) mp = BP_DRAGON; break; default: + mp = canride(u); + if (mp>=BP_RIDING) { + dk = 1.0; + } break; } @@ -1426,38 +1422,21 @@ static int movement_speed(unit * u) } } - switch (canride(u)) { - case 1: /* Pferd */ - mp = BP_RIDING; - break; - - case 2: /* Einhorn */ - mp = BP_UNICORN; - break; - - default: - /* Siebenmeilentee */ - if (get_effect(u, oldpotiontype[P_FAST]) >= u->number) { - mp *= 2; - change_effect(u, oldpotiontype[P_FAST], -u->number); - } - - /* unicorn in inventory */ - if (u->number <= i_get(u->items, it_find("fairyboot"))) { - mp *= 2; - } - - /* Im Astralraum sind Tyb und Ill-Magier doppelt so schnell. - * Nicht kumulativ mit anderen Beschleunigungen! */ - if (mp * dk <= BP_WALKING * u_race(u)->speed && is_astral(u->region) - && is_mage(u)) { - sc_mage *mage = get_mage_depr(u); - if (mage->magietyp == M_TYBIED || mage->magietyp == M_ILLAUN) { - mp *= 2; - } - } - break; + /* unicorn in inventory */ + if (u->number <= i_get(u->items, it_find("fairyboot"))) { + mp *= 2; } + + /* Im Astralraum sind Tyb und Ill-Magier doppelt so schnell. + * Nicht kumulativ mit anderen Beschleunigungen! */ + if (mp * dk <= BP_WALKING * u_race(u)->speed && is_astral(u->region) + && is_mage(u)) { + sc_mage *mage = get_mage_depr(u); + if (mage->magietyp == M_TYBIED || mage->magietyp == M_ILLAUN) { + mp *= 2; + } + } + return (int)(dk * mp); } @@ -2044,7 +2023,7 @@ static const region_list *travel_i(unit * u, const region_list * route_begin, const region_list * route_end, order * ord, int mode, follower ** followers) { region *r = u->region; - + int mp; if (u->building && !can_leave(u)) { cmistake(u, u->thisorder, 150, MSG_MOVE); return route_begin; @@ -2060,7 +2039,15 @@ static const region_list *travel_i(unit * u, const region_list * route_begin, cmistake(u, ord, 42, MSG_MOVE); return route_begin; } - route_end = cap_route(r, route_begin, route_end, movement_speed(u)); + + mp = movement_speed(u); + /* Siebenmeilentee */ + if (get_effect(u, oldpotiontype[P_FAST]) >= u->number) { + mp *= 2; + change_effect(u, oldpotiontype[P_FAST], -u->number); + } + + route_end = cap_route(r, route_begin, route_end, mp); route_end = travel_route(u, route_begin, route_end, ord, mode); if (u->flags&UFL_FOLLOWED) { diff --git a/src/move.h b/src/move.h index d5b07fef9..4b9c75749 100644 --- a/src/move.h +++ b/src/move.h @@ -36,6 +36,14 @@ extern "C" { extern struct attrib_type at_shiptrail; extern int *storms; + /* Bewegungsweiten: */ +#define BP_WALKING 4 +#define BP_RIDING 6 +#define BP_UNICORN 9 +#define BP_DRAGON 4 +#define BP_NORMAL 3 +#define BP_ROAD 2 + /* die Zahlen sind genau äquivalent zu den race Flags */ #define MV_CANNOTMOVE (1<<5) #define MV_FLY (1<<7) /* kann fliegen */ @@ -70,6 +78,7 @@ extern "C" { struct ship *move_ship(struct ship *sh, struct region *from, struct region *to, struct region_list *route); int walkingcapacity(const struct unit *u); + int movement_speed(const struct unit * u); void follow_unit(struct unit *u); struct unit *owner_buildingtyp(const struct region *r, const struct building_type *bt); diff --git a/src/move.test.c b/src/move.test.c index ab30300e8..c8fdb2436 100644 --- a/src/move.test.c +++ b/src/move.test.c @@ -522,9 +522,33 @@ static void test_ship_leave_trail(CuTest *tc) { test_cleanup(); } +static void test_movement_speed(CuTest *tc) { + unit * u; + race * rc; + const struct item_type *it_horse; + + test_setup(); + it_horse = test_create_horse(); + rc = test_create_race(NULL); + u = test_create_unit(test_create_faction(rc), test_create_region(0, 0, NULL)); + + rc->speed = 1.0; + CuAssertIntEquals(tc, BP_WALKING, movement_speed(u)); + + rc->speed = 2.0; + CuAssertIntEquals(tc, 2 * BP_WALKING, movement_speed(u)); + + set_level(u, SK_RIDING, 1); + i_change(&u->items, it_horse, 1); + CuAssertIntEquals(tc, BP_RIDING, movement_speed(u)); + + test_cleanup(); +} + CuSuite *get_move_suite(void) { CuSuite *suite = CuSuiteNew(); + SUITE_ADD_TEST(suite, test_movement_speed); SUITE_ADD_TEST(suite, test_walkingcapacity); SUITE_ADD_TEST(suite, test_ship_not_allowed_in_coast); SUITE_ADD_TEST(suite, test_ship_leave_trail); diff --git a/src/tests.c b/src/tests.c index 49b5b7c05..7d20b59c4 100644 --- a/src/tests.c +++ b/src/tests.c @@ -39,7 +39,7 @@ struct race *test_create_race(const char *name) { - race *rc = rc_get_or_create(name); + race *rc = rc_get_or_create(name ? name : "smurf"); rc->maintenance = 10; rc->hitpoints = 20; rc->maxaura = 100; From 86e265c7c48aaeab0d653cfc131acc321177d2de Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 6 Oct 2017 22:38:01 +0200 Subject: [PATCH 21/27] miniature is_mage get_mage optimization --- src/move.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/move.c b/src/move.c index 46a8f200f..3f899ac22 100644 --- a/src/move.c +++ b/src/move.c @@ -1429,11 +1429,12 @@ int movement_speed(const unit * u) /* Im Astralraum sind Tyb und Ill-Magier doppelt so schnell. * Nicht kumulativ mit anderen Beschleunigungen! */ - if (mp * dk <= BP_WALKING * u_race(u)->speed && is_astral(u->region) - && is_mage(u)) { - sc_mage *mage = get_mage_depr(u); - if (mage->magietyp == M_TYBIED || mage->magietyp == M_ILLAUN) { - mp *= 2; + if (mp * dk <= BP_WALKING * u_race(u)->speed && is_astral(u->region)) { + sc_mage *mage = get_mage(u); + if (mage && (mage->magietyp == M_TYBIED || mage->magietyp == M_ILLAUN)) { + if (has_skill(u, SK_MAGIC)) { + mp *= 2; + } } } From bb54e0d5abb4778ef061b2f40d7f6ac01205a94f Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 6 Oct 2017 23:18:47 +0200 Subject: [PATCH 22/27] gcc unused variable --- src/spells.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/spells.c b/src/spells.c index 526761169..c9e914904 100644 --- a/src/spells.c +++ b/src/spells.c @@ -532,7 +532,6 @@ static unit * make_familiar(unit * mage, region *r, const race *rc, const char * static int sp_summon_familiar(castorder * co) { - unit *familiar; region *r = co_get_region(co); unit *mage = co->magician.u; int cast_level = co->level; @@ -580,7 +579,7 @@ static int sp_summon_familiar(castorder * co) msg = msg_message("familiar_name", "unit", mage); nr_render(msg, mage->faction->locale, zText, sizeof(zText), mage->faction); msg_release(msg); - familiar = make_familiar(mage, r, rc, zText); + make_familiar(mage, r, rc, zText); dh = 0; dh1 = 0; From 0537d39b17b10ca8d3dd0e7e51dcff99f12b1667 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 7 Oct 2017 03:22:35 +0200 Subject: [PATCH 23/27] add a fix for 2367 to fix_familiars --- src/kernel/save.c | 82 ++++++++++++++++++++++++++++----------------- src/util/gamedata.h | 2 +- 2 files changed, 52 insertions(+), 32 deletions(-) diff --git a/src/kernel/save.c b/src/kernel/save.c index 23d6f3e71..e3a63156c 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -24,8 +24,9 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. #include "alchemy.h" #include "alliance.h" #include "ally.h" -#include "connection.h" #include "building.h" +#include "connection.h" +#include "equipment.h" #include "faction.h" #include "group.h" #include "item.h" @@ -1600,41 +1601,60 @@ static void fix_familiars(void) { if (u->_race != u->faction->race && (u->_race->flags & RCF_FAMILIAR)) { /* unit is potentially a familiar */ attrib * a = a_find(u->attribs, &at_mage); - if (a) { - /* unit is magical */ - attrib * am = a_find(u->attribs, &at_familiarmage); - if (!am) { - /* but it is not a familiar? */ - attrib * ae = a_find(u->attribs, &at_eventhandler); - if (ae) { - trigger **tlist; - tlist = get_triggers(ae, "destroy"); - if (tlist) { - trigger *t; - unit *um = NULL; - for (t = *tlist; t; t = t->next) { - if (t->type == &tt_shock) { - um = (unit *)t->data.v; - break; - } + attrib * am = a_find(u->attribs, &at_familiarmage); + if (am) { + sc_mage *mage = a ? (sc_mage *)a->data.v : NULL; + /* a familiar */ + if (!mage) { + log_error("%s seems to be a familiar with no magic.", + unitname(u)); + mage = create_mage(u, M_GRAY); + } + if (!mage->spellbook) { + char eqname[32]; + equipment *eq; + + snprintf(eqname, sizeof(eqname), "fam_%s", u->_race->_name); + eq = get_equipment(eqname); + if (eq && eq->spells) { + log_error("%s seems to be a familiar with no spells.", + unitname(u)); + /* magical familiar, no spells */ + equip_unit_mask(u, eq, EQUIP_SPELLS); + } + } + } + else if (a) { + /* not a familiar, but magical */ + attrib * ae = a_find(u->attribs, &at_eventhandler); + if (ae) { + trigger **tlist; + tlist = get_triggers(ae, "destroy"); + if (tlist) { + trigger *t; + unit *um = NULL; + for (t = *tlist; t; t = t->next) { + if (t->type == &tt_shock) { + um = (unit *)t->data.v; + break; } - if (um) { - attrib *af = a_find(um->attribs, &at_familiar); - log_error("%s seems to be a broken familiar of %s.", - unitname(u), unitname(um)); - if (af) { - unit * uf = (unit *)af->data.v; - log_error("%s already has a familiar: %s.", - unitname(um), unitname(uf)); - } - else { - set_familiar(um, u); - } + } + if (um) { + attrib *af = a_find(um->attribs, &at_familiar); + log_error("%s seems to be a broken familiar of %s.", + unitname(u), unitname(um)); + if (af) { + unit * uf = (unit *)af->data.v; + log_error("%s already has a familiar: %s.", + unitname(um), unitname(uf)); } else { - log_error("%s seems to be a broken familiar with no trigger.", unitname(u)); + set_familiar(um, u); } } + else { + log_error("%s seems to be a broken familiar with no trigger.", unitname(u)); + } } } } diff --git a/src/util/gamedata.h b/src/util/gamedata.h index 82df55cc3..22c1d9052 100644 --- a/src/util/gamedata.h +++ b/src/util/gamedata.h @@ -39,7 +39,7 @@ /* unfinished: */ #define CRYPT_VERSION 400 /* passwords are encrypted */ -#define RELEASE_VERSION SKILLSORT_VERSION /* current datafile */ +#define RELEASE_VERSION LANDDISPLAY_VERSION /* current datafile */ #define MIN_VERSION UIDHASH_VERSION /* minimal datafile we support */ #define MAX_VERSION RELEASE_VERSION /* change this if we can need to read the future datafile, and we can do so */ From b255af52e2c990dfced87f5f4faec84f09ea5bf8 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 7 Oct 2017 13:56:14 +0200 Subject: [PATCH 24/27] add a failing integration test for the parser bug --- tests/run-turn.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/run-turn.sh b/tests/run-turn.sh index fd6eb63ee..ad2e0e6b8 100755 --- a/tests/run-turn.sh +++ b/tests/run-turn.sh @@ -18,7 +18,7 @@ assert_grep_count() { file=$1 expr=$2 expect=$3 -count=`grep -cE $expr $file` +count=`grep -cE "$expr" $file` [ $count -eq $expect ] || quit 1 "expected $expect counts of $expr in $file, got $count" echo "PASS: $expr is $expect" } @@ -33,7 +33,7 @@ setup cleanup VALGRIND=`which valgrind` SERVER=../Debug/eressea/eressea -set -e +#set -e if [ -n "$VALGRIND" ]; then SUPP=../share/ubuntu-12_04.supp SERVER="$VALGRIND --track-origins=yes --gen-suppressions=all --suppressions=$SUPP --error-exitcode=1 --leak-check=no $SERVER" @@ -62,5 +62,6 @@ assert_grep_count reports/185-heg.cr '"lighthouse";visibility' 6 assert_grep_count reports/185-heg.cr '"neighbour";visibility' 11 assert_grep_count reports/185-6rLo.cr '^EINHEIT' 2 assert_grep_count reports/185-6rLo.cr '^REGION' 13 +assert_grep_count reports/185-6rLo.cr "Befehl ist unbekannt" 0 echo "integration tests: PASS" cleanup From ecce8e5d916b5e032051222c029f6c41c209e1c2 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 7 Oct 2017 18:06:54 +0200 Subject: [PATCH 25/27] Revert "refactor: extract a read_order function." This reverts commit 8b39133dbf8958cab1131555b6054ce1063b9646. --- src/kernel/save.c | 87 +++++++++++++++++++++--------------------- src/kernel/save.h | 4 -- src/kernel/save.test.c | 41 -------------------- 3 files changed, 43 insertions(+), 89 deletions(-) diff --git a/src/kernel/save.c b/src/kernel/save.c index e3a63156c..6338b6827 100644 --- a/src/kernel/save.c +++ b/src/kernel/save.c @@ -123,42 +123,6 @@ char *rns(FILE * f, char *c, size_t size) return c; } -struct order *read_order(const char *in, const struct locale *lang) { - assert(in); - assert(lang); - if (in[0]) { - const char *s = in; - keyword_t kwd; - char token[64]; - const char *stok; - - stok = parse_token(&s, token, sizeof(token)); - if (stok) { - param_t param = findparam(token, lang); - switch (param) { - case P_UNIT: - case P_REGION: - return NULL; - case P_FACTION: - case P_NEXT: - case P_GAMENAME: - /* these terminate the orders, so we apply extra checking */ - if (strlen(stok) >= 3) { - return NULL; - } - break; - default: - break; - } - } - /* Nun wird der Befehl erzeut und eingehängt */ - kwd = get_keyword(stok, lang); - if (kwd != NOKEYWORD) { - return parse_order(in, lang); - } - } - return NULL; -} static unit *unitorders(FILE * F, int enc, struct faction *f) { @@ -201,7 +165,6 @@ static unit *unitorders(FILE * F, int enc, struct faction *f) for (;;) { const char *s; - order * ord; /* Erst wenn wir sicher sind, dass kein Befehl * eingegeben wurde, checken wir, ob nun eine neue * Einheit oder ein neuer Spieler drankommt */ @@ -210,13 +173,49 @@ static unit *unitorders(FILE * F, int enc, struct faction *f) if (s == NULL) break; - ord = read_order(s, f->locale); - if (!ord) { - ADDMSG(&f->msgs, msg_message("parse_error", "unit command", u, s)); - break; + if (s[0]) { + if (s[0] != '@') { + char token[64]; + const char *stok = s; + stok = parse_token(&stok, token, sizeof(token)); + + if (stok) { + bool quit = false; + param_t param = findparam(stok, u->faction->locale); + switch (param) { + case P_UNIT: + case P_REGION: + quit = true; + break; + case P_FACTION: + case P_NEXT: + case P_GAMENAME: + /* these terminate the orders, so we apply extra checking */ + if (strlen(stok) >= 3) { + quit = true; + break; + } + else { + quit = false; + } + break; + default: + break; + } + if (quit) { + break; + } + } + } + /* Nun wird der Befehl erzeut und eingehängt */ + *ordp = parse_order(s, u->faction->locale); + if (*ordp) { + ordp = &(*ordp)->next; + } + else { + ADDMSG(&f->msgs, msg_message("parse_error", "unit command", u, s)); + } } - *ordp = ord; - ordp = &(*ordp)->next; } } @@ -306,7 +305,7 @@ int readorders(const char *filename) } init_tokens_str(b); s = gettoken(token, sizeof(token)); - p = (s && s[0] != '@' && s[0] != '!') ? findparam(s, lang) : NOPARAM; + p = (s && s[0] != '@') ? findparam(s, lang) : NOPARAM; } while ((p != P_UNIT || !f) && p != P_FACTION && p != P_NEXT && p != P_GAMENAME); } diff --git a/src/kernel/save.h b/src/kernel/save.h index f706a311b..763ff66b6 100644 --- a/src/kernel/save.h +++ b/src/kernel/save.h @@ -31,8 +31,6 @@ extern "C" { struct spellbook; struct unit; struct building; - struct order; - struct locale; struct faction; struct region; struct ship; @@ -45,8 +43,6 @@ extern "C" { /* TODO: is this *really* still in use? */ extern int enc_gamedata; - struct order *read_order(const char *s, const struct locale *lang); - int readorders(const char *filename); int readgame(const char *filename); int writegame(const char *filename); diff --git a/src/kernel/save.test.c b/src/kernel/save.test.c index 73fe4663b..81d5a507d 100644 --- a/src/kernel/save.test.c +++ b/src/kernel/save.test.c @@ -5,7 +5,6 @@ #include #include "save.h" -#include "order.h" #include "version.h" #include "building.h" #include "ship.h" @@ -453,45 +452,6 @@ static void test_version_no(CuTest *tc) { CuAssertIntEquals(tc, 0x10203, version_no("1.2.3-what.is.42")); } -static void test_read_order(CuTest *tc) { - char cmd[32]; - order *ord; - struct locale * lang; - - test_setup(); - lang = test_create_locale(); - - ord = read_order("MOVE NORTH", lang); - CuAssertPtrNotNull(tc, ord); - CuAssertIntEquals(tc, K_MOVE, ord->command); - CuAssertIntEquals(tc, K_MOVE, getkeyword(ord)); - CuAssertStrEquals(tc, "move NORTH", get_command(ord, cmd, sizeof(cmd))); - free_order(ord); - - ord = read_order("MAKE TEMP foo", lang); - CuAssertPtrNotNull(tc, ord); - CuAssertIntEquals(tc, K_MAKETEMP, ord->command); - CuAssertIntEquals(tc, K_MAKETEMP, getkeyword(ord)); - CuAssertStrEquals(tc, "maketemp foo", get_command(ord, cmd, sizeof(cmd))); - free_order(ord); - - ord = read_order("MAKETEMP foo", lang); - CuAssertPtrNotNull(tc, ord); - CuAssertIntEquals(tc, K_MAKETEMP, ord->command); - CuAssertIntEquals(tc, K_MAKETEMP, getkeyword(ord)); - CuAssertStrEquals(tc, "maketemp foo", get_command(ord, cmd, sizeof(cmd))); - free_order(ord); - - CuAssertPtrEquals(tc, NULL, read_order("HODOR HODOR HODOR", lang)); - CuAssertPtrEquals(tc, NULL, read_order("FACTION abcd", lang)); - CuAssertPtrEquals(tc, NULL, read_order("UNIT abcd", lang)); - CuAssertPtrEquals(tc, NULL, read_order("ERESSEA abcd", lang)); - CuAssertPtrEquals(tc, NULL, read_order("REGION 2,3", lang)); - CuAssertPtrEquals(tc, NULL, read_order("NEXT", lang)); - - test_cleanup(); -} - CuSuite *get_save_suite(void) { CuSuite *suite = CuSuiteNew(); @@ -508,7 +468,6 @@ CuSuite *get_save_suite(void) SUITE_ADD_TEST(suite, test_readwrite_dead_faction_group); SUITE_ADD_TEST(suite, test_read_password); SUITE_ADD_TEST(suite, test_read_password_external); - SUITE_ADD_TEST(suite, test_read_order); SUITE_ADD_TEST(suite, test_version_no); return suite; From 5ce4939351b5f5c38c4d39bd208a893ea5ef5ec3 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 7 Oct 2017 18:08:28 +0200 Subject: [PATCH 26/27] ifndef --- clibs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clibs b/clibs index da2c0cc39..147584ad7 160000 --- a/clibs +++ b/clibs @@ -1 +1 @@ -Subproject commit da2c0cc39b27c98ed8d31b0503426788fc236bd8 +Subproject commit 147584ad70b220abf6a4e97ca76e785729b9ac32 From b676bb09686952f794bf7c51c8bb2b7b23b84405 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 7 Oct 2017 18:43:04 +0200 Subject: [PATCH 27/27] remove unused newfaction.bonus thanks to @stm2. This feature is long dead. --- src/modules/autoseed.c | 8 +++----- src/modules/autoseed.h | 1 - 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/modules/autoseed.c b/src/modules/autoseed.c index 343c742ee..52a5344cc 100644 --- a/src/modules/autoseed.c +++ b/src/modules/autoseed.c @@ -147,8 +147,7 @@ newfaction *read_newfactions(const char *filename) faction *f; char race[20], email[64], lang[8], password[16]; newfaction *nf, **nfi; - int bonus = 0, subscription = 0; - int alliance = 0; + int alliance = 0, subscription = 0; if (fgets(buf, sizeof(buf), F) == NULL) break; @@ -156,8 +155,8 @@ newfaction *read_newfactions(const char *filename) email[0] = '\0'; password[0] = '\0'; - if (sscanf(buf, "%54s %20s %8s %d %d %16s %d", email, race, lang, &bonus, - &subscription, password, &alliance) < 3) { + if (sscanf(buf, "%54s %20s %8s %16s %d %d", email, race, lang, + password, &subscription, &alliance) < 3) { break; } if (email[0] == '#') { @@ -228,7 +227,6 @@ newfaction *read_newfactions(const char *filename) } } nf->lang = get_locale(lang); - nf->bonus = bonus; assert(nf->race && nf->email && nf->lang); nfi = &newfactions; while (*nfi) { diff --git a/src/modules/autoseed.h b/src/modules/autoseed.h index 65bdccb7f..937a1f8b5 100644 --- a/src/modules/autoseed.h +++ b/src/modules/autoseed.h @@ -24,7 +24,6 @@ extern "C" { char *password; const struct locale *lang; const struct race *race; - int bonus; int subscription; bool oldregions; struct alliance *allies;