From 10806a1d95603965aa1071e9d32a7456c18abe26 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Tue, 27 Sep 2016 08:25:58 +0200 Subject: [PATCH 1/5] add a failing test for bug 2234 --- src/economy.test.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/economy.test.c b/src/economy.test.c index fbcc720bc..c1459544e 100644 --- a/src/economy.test.c +++ b/src/economy.test.c @@ -246,6 +246,7 @@ static void test_maintain_buildings(CuTest *tc) { building *b; building_type *btype; unit *u; + faction *f; maintenance *req; item_type *itype; @@ -253,7 +254,8 @@ static void test_maintain_buildings(CuTest *tc) { btype = test_create_buildingtype("Hort"); btype->maxsize = 10; r = test_create_region(0, 0, 0); - u = test_create_unit(test_create_faction(0), r); + f = test_create_faction(0); + u = test_create_unit(f, r); b = test_create_building(r, btype); itype = test_create_itemtype("money"); b->size = btype->maxsize; @@ -263,6 +265,7 @@ static void test_maintain_buildings(CuTest *tc) { b->flags = 0; maintain_buildings(r); CuAssertIntEquals(tc, BLD_MAINTAINED, fval(b, BLD_MAINTAINED)); + CuAssertPtrEquals(tc, 0, test_find_messagetype(f->msgs, "maintenance_nowork")); req = calloc(2, sizeof(maintenance)); req[0].number = 100; @@ -273,6 +276,8 @@ static void test_maintain_buildings(CuTest *tc) { b->flags = 0; maintain_buildings(r); CuAssertIntEquals(tc, 0, fval(b, BLD_MAINTAINED)); + CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "maintenance_nowork")); + CuAssertPtrEquals(tc, 0, test_find_messagetype(r->msgs, "maintenance_noowner")); // we can afford to pay: i_change(&u->items, itype, 100); @@ -286,6 +291,8 @@ static void test_maintain_buildings(CuTest *tc) { b->flags = 0; maintain_buildings(r); CuAssertIntEquals(tc, 0, fval(b, BLD_MAINTAINED)); + CuAssertPtrEquals(tc, 0, test_find_messagetype(f->msgs, "maintenance_nowork")); + CuAssertPtrNotNull(tc, test_find_messagetype(r->msgs, "maintenance_noowner")); test_cleanup(); } From f2d0d74e70b72941d60d358997fb69ee5b1156fa Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Tue, 27 Sep 2016 15:02:22 +0200 Subject: [PATCH 2/5] fix the test, except it doesn't fail anymore? --- src/economy.test.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/economy.test.c b/src/economy.test.c index c1459544e..bbdd75df5 100644 --- a/src/economy.test.c +++ b/src/economy.test.c @@ -278,13 +278,15 @@ static void test_maintain_buildings(CuTest *tc) { CuAssertIntEquals(tc, 0, fval(b, BLD_MAINTAINED)); CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "maintenance_nowork")); CuAssertPtrEquals(tc, 0, test_find_messagetype(r->msgs, "maintenance_noowner")); - + test_clear_messages(f); + // we can afford to pay: i_change(&u->items, itype, 100); b->flags = 0; maintain_buildings(r); CuAssertIntEquals(tc, BLD_MAINTAINED, fval(b, BLD_MAINTAINED)); CuAssertIntEquals(tc, 0, i_get(u->items, itype)); + test_clear_messages(f); // this building has no owner, it doesn't work: u_set_building(u, NULL); From fc6ab724a5762e710c5aa98c90b656e4bedf93a1 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Tue, 27 Sep 2016 15:40:03 +0200 Subject: [PATCH 3/5] fix the economy test. no idea why it passes now? this doesn't represent the bug 2234 that I am trying to repro, yet. --- src/economy.test.c | 5 ++++- src/tests.c | 8 ++++++++ src/tests.h | 1 + 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/economy.test.c b/src/economy.test.c index bbdd75df5..e7fb5e332 100644 --- a/src/economy.test.c +++ b/src/economy.test.c @@ -286,6 +286,9 @@ static void test_maintain_buildings(CuTest *tc) { maintain_buildings(r); CuAssertIntEquals(tc, BLD_MAINTAINED, fval(b, BLD_MAINTAINED)); CuAssertIntEquals(tc, 0, i_get(u->items, itype)); + CuAssertPtrEquals(tc, 0, r->msgs); + CuAssertPtrEquals(tc, 0, test_find_messagetype(f->msgs, "maintenance_nowork")); + CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "maintenance")); test_clear_messages(f); // this building has no owner, it doesn't work: @@ -293,7 +296,7 @@ static void test_maintain_buildings(CuTest *tc) { b->flags = 0; maintain_buildings(r); CuAssertIntEquals(tc, 0, fval(b, BLD_MAINTAINED)); - CuAssertPtrEquals(tc, 0, test_find_messagetype(f->msgs, "maintenance_nowork")); + CuAssertPtrEquals(tc, 0, f->msgs); CuAssertPtrNotNull(tc, test_find_messagetype(r->msgs, "maintenance_noowner")); test_cleanup(); diff --git a/src/tests.c b/src/tests.c index 53f5ea470..177ad50fc 100644 --- a/src/tests.c +++ b/src/tests.c @@ -452,6 +452,14 @@ struct message * test_find_messagetype(struct message_list *msgs, const char *na return test_find_messagetype_ex(msgs, name, NULL); } +void test_clear_messagelist(message_list **msgs) { + if (*msgs) { + free_messagelist((*msgs)->begin); + free(*msgs); + *msgs = NULL; + } +} + void test_clear_messages(faction *f) { if (f->msgs) { free_messagelist(f->msgs->begin); diff --git a/src/tests.h b/src/tests.h index 10934c3d7..cbc2a2511 100644 --- a/src/tests.h +++ b/src/tests.h @@ -61,6 +61,7 @@ extern "C" { struct message * test_find_messagetype(struct message_list *msgs, const char *name); struct message * test_get_last_message(struct message_list *mlist); void test_clear_messages(struct faction *f); + void test_clear_messagelist(struct message_list **msgs); void assert_message(struct CuTest * tc, struct message *msg, char *name, int numpar); void assert_pointer_parameter(struct CuTest * tc, struct message *msg, int index, void *arg); From cbd1e8c457b949c1ef9067367054890890135f0e Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 1 Oct 2016 18:34:38 +0200 Subject: [PATCH 4/5] Fixing https://bugs.eressea.de/view.php?id=2234 remove the MTF_VITAL materials flag. remove remaining BLD_MAINTAINED vs BLD_WORKING code. --- res/core/common/buildings.xml | 26 +++++------ res/e3a/buildings.xml | 2 +- src/economy.c | 85 +++++++---------------------------- src/economy.test.c | 16 ++++--- src/kernel/building.h | 1 - src/kernel/jsonconf.c | 2 +- src/kernel/jsonconf.test.c | 4 +- src/kernel/xmlreader.c | 3 -- 8 files changed, 45 insertions(+), 94 deletions(-) diff --git a/res/core/common/buildings.xml b/res/core/common/buildings.xml index 423e90deb..cf183086b 100644 --- a/res/core/common/buildings.xml +++ b/res/core/common/buildings.xml @@ -23,7 +23,7 @@ - + @@ -34,7 +34,7 @@ - + @@ -45,7 +45,7 @@ - + @@ -56,7 +56,7 @@ - + @@ -75,7 +75,7 @@ - + @@ -85,7 +85,7 @@ - + @@ -96,7 +96,7 @@ - + @@ -107,7 +107,7 @@ - + @@ -119,7 +119,7 @@ - + @@ -129,7 +129,7 @@ - + @@ -138,7 +138,7 @@ - + @@ -148,7 +148,7 @@ - + @@ -158,7 +158,7 @@ - + diff --git a/res/e3a/buildings.xml b/res/e3a/buildings.xml index 8ca30d27d..c1edbcc0b 100644 --- a/res/e3a/buildings.xml +++ b/res/e3a/buildings.xml @@ -19,7 +19,7 @@ - + diff --git a/src/economy.c b/src/economy.c index 06b8c0f84..099e9786d 100644 --- a/src/economy.c +++ b/src/economy.c @@ -717,7 +717,7 @@ static int maintain(building * b) { int c; region *r = b->region; - bool paid = true, work = true; + bool paid = true; unit *u; if (fval(b, BLD_MAINTAINED) || b->type == NULL || b->type->maintenance == NULL) { @@ -738,78 +738,40 @@ static int maintain(building * b) return 0; } } - for (c = 0; b->type->maintenance[c].number; ++c) { + for (c = 0; b->type->maintenance[c].number && paid; ++c) { const maintenance *m = b->type->maintenance + c; int need = m->number; if (fval(m, MTF_VARIABLE)) need = need * b->size; - if (u) { - /* first ist im ersten versuch true, im zweiten aber false! Das - * bedeutet, das in der Runde in die Region geschafften Resourcen - * nicht genutzt werden können, weil die reserviert sind! */ - need -= get_pooled(u, m->rtype, GET_DEFAULT, need); - } + need -= get_pooled(u, m->rtype, GET_DEFAULT, need); if (need > 0) { - if (!fval(m, MTF_VITAL)) - work = false; - else { - paid = false; - break; - } + paid = false; } } if (fval(b, BLD_DONTPAY)) { + ADDMSG(&r->msgs, msg_message("maintenance_nowork", "building", b)); return 0; } - u = building_owner(b); - if (!u) { + if (!paid) { + ADDMSG(&u->faction->msgs, msg_message("maintenancefail", "unit building", u, b)); + ADDMSG(&r->msgs, msg_message("maintenance_nowork", "building", b)); return 0; } for (c = 0; b->type->maintenance[c].number; ++c) { const maintenance *m = b->type->maintenance + c; - int need = m->number; + int cost = m->number; - if (fval(m, MTF_VARIABLE)) - need = need * b->size; - if (u) { - need -= get_pooled(u, m->rtype, GET_DEFAULT, need); - if (need > 0) { - work = false; - if (fval(m, MTF_VITAL)) { - paid = false; - break; - } - } + if (fval(m, MTF_VARIABLE)) { + cost = cost * b->size; } + cost -= + use_pooled(u, m->rtype, GET_SLACK | GET_RESERVE | GET_POOLED_SLACK, + cost); + assert(cost == 0); } - if (paid && c > 0) { - if (!work) { - ADDMSG(&u->faction->msgs, msg_message("maintenancefail", "unit building", u, b)); - return 0; - } - - for (c = 0; b->type->maintenance[c].number; ++c) { - const maintenance *m = b->type->maintenance + c; - int cost = m->number; - - if (!fval(m, MTF_VITAL) && !work) - continue; - if (fval(m, MTF_VARIABLE)) - cost = cost * b->size; - - cost -= - use_pooled(u, m->rtype, GET_SLACK | GET_RESERVE | GET_POOLED_SLACK, - cost); - assert(cost == 0); - } - if (work) { - ADDMSG(&u->faction->msgs, msg_message("maintenance", "unit building", u, b)); - return BLD_MAINTAINED; - } - } - ADDMSG(&u->faction->msgs, msg_message("maintenancefail", "unit building", u, b)); - return 0; + ADDMSG(&u->faction->msgs, msg_message("maintenance", "unit building", u, b)); + return BLD_MAINTAINED; } void maintain_buildings(region * r) @@ -824,19 +786,6 @@ void maintain_buildings(region * r) flags = maintain(b); } fset(b, flags); - - if (!fval(b, BLD_MAINTAINED)) { - unit *u = building_owner(b); - struct message *msg = msg_message("maintenance_nowork", "building", b); - if (u) { - add_message(&u->faction->msgs, msg); - r_addmessage(r, u->faction, msg); - } - else { - add_message(&r->msgs, msg); - } - msg_release(msg); - } bp = &b->next; } } diff --git a/src/economy.test.c b/src/economy.test.c index e7fb5e332..7f48df289 100644 --- a/src/economy.test.c +++ b/src/economy.test.c @@ -241,6 +241,9 @@ static void test_tax_cmd(CuTest *tc) { test_cleanup(); } +/** + * see https://bugs.eressea.de/view.php?id=2234 + */ static void test_maintain_buildings(CuTest *tc) { region *r; building *b; @@ -265,7 +268,8 @@ static void test_maintain_buildings(CuTest *tc) { b->flags = 0; maintain_buildings(r); CuAssertIntEquals(tc, BLD_MAINTAINED, fval(b, BLD_MAINTAINED)); - CuAssertPtrEquals(tc, 0, test_find_messagetype(f->msgs, "maintenance_nowork")); + CuAssertPtrEquals(tc, 0, f->msgs); + CuAssertPtrEquals(tc, 0, r->msgs); req = calloc(2, sizeof(maintenance)); req[0].number = 100; @@ -276,9 +280,10 @@ static void test_maintain_buildings(CuTest *tc) { b->flags = 0; maintain_buildings(r); CuAssertIntEquals(tc, 0, fval(b, BLD_MAINTAINED)); - CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "maintenance_nowork")); - CuAssertPtrEquals(tc, 0, test_find_messagetype(r->msgs, "maintenance_noowner")); - test_clear_messages(f); + CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "maintenancefail")); + CuAssertPtrNotNull(tc, test_find_messagetype(r->msgs, "maintenance_nowork")); + test_clear_messagelist(&f->msgs); + test_clear_messagelist(&r->msgs); // we can afford to pay: i_change(&u->items, itype, 100); @@ -289,7 +294,7 @@ static void test_maintain_buildings(CuTest *tc) { CuAssertPtrEquals(tc, 0, r->msgs); CuAssertPtrEquals(tc, 0, test_find_messagetype(f->msgs, "maintenance_nowork")); CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "maintenance")); - test_clear_messages(f); + test_clear_messagelist(&f->msgs); // this building has no owner, it doesn't work: u_set_building(u, NULL); @@ -298,6 +303,7 @@ static void test_maintain_buildings(CuTest *tc) { CuAssertIntEquals(tc, 0, fval(b, BLD_MAINTAINED)); CuAssertPtrEquals(tc, 0, f->msgs); CuAssertPtrNotNull(tc, test_find_messagetype(r->msgs, "maintenance_noowner")); + test_clear_messagelist(&r->msgs); test_cleanup(); } diff --git a/src/kernel/building.h b/src/kernel/building.h index dad9ffdde..1ea1836dc 100644 --- a/src/kernel/building.h +++ b/src/kernel/building.h @@ -31,7 +31,6 @@ extern "C" { /* maintenance::flags */ #define MTF_NONE 0x00 #define MTF_VARIABLE 0x01 /* resource usage scales with size */ -#define MTF_VITAL 0x02 /* if resource missing, building may crash */ typedef struct maintenance { const struct resource_type *rtype; /* type of resource required */ diff --git a/src/kernel/jsonconf.c b/src/kernel/jsonconf.c index 721e6e60f..9e6001f8d 100644 --- a/src/kernel/jsonconf.c +++ b/src/kernel/jsonconf.c @@ -106,7 +106,7 @@ static void json_maintenance_i(cJSON *json, maintenance *mt) { break; case cJSON_Array: if (strcmp(child->string, "flags") == 0) { - const char * flags[] = { "variable", "required", 0 }; + const char * flags[] = { "variable", 0 }; mt->flags = json_flags(child, flags); } else { diff --git a/src/kernel/jsonconf.test.c b/src/kernel/jsonconf.test.c index 38b2047e6..3fac3478a 100644 --- a/src/kernel/jsonconf.test.c +++ b/src/kernel/jsonconf.test.c @@ -319,7 +319,7 @@ static void test_spells(CuTest * tc) static const char * building_data = "{\"buildings\": { " "\"house\" : { " "\"maintenance\" : " -"{ \"type\" : \"iron\", \"amount\" : 1, \"flags\" : [ \"required\", \"variable\" ] }" +"{ \"type\" : \"iron\", \"amount\" : 1, \"flags\" : [ \"variable\" ] }" "," "\"construction\" : {" "\"maxsize\" : 20," @@ -363,7 +363,7 @@ static void test_buildings(CuTest * tc) CuAssertPtrNotNull(tc, bt->maintenance); CuAssertIntEquals(tc, 1, bt->maintenance[0].number); CuAssertPtrEquals(tc, (void *)get_resourcetype(R_IRON), (void *)bt->maintenance[0].rtype); - CuAssertIntEquals(tc, MTF_VARIABLE | MTF_VITAL, bt->maintenance[0].flags); + CuAssertIntEquals(tc, MTF_VARIABLE, bt->maintenance[0].flags); CuAssertIntEquals(tc, 0, bt->maintenance[1].number); CuAssertPtrNotNull(tc, bt->construction); diff --git a/src/kernel/xmlreader.c b/src/kernel/xmlreader.c index 66825a9b8..06363de48 100644 --- a/src/kernel/xmlreader.c +++ b/src/kernel/xmlreader.c @@ -339,9 +339,6 @@ static int parse_buildings(xmlDocPtr doc) if (xml_bvalue(node, "variable", false)) mt->flags |= MTF_VARIABLE; - if (xml_bvalue(node, "vital", false)) - mt->flags |= MTF_VITAL; - } xmlXPathFreeObject(result); From c640fb4be85a682dce80da91ebc257b9d1e0ccd4 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 1 Oct 2016 16:39:35 +0000 Subject: [PATCH 5/5] compile in the absence of libxml2-dev --- src/kernel/config.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/kernel/config.c b/src/kernel/config.c index 5921535c7..f7eb0e303 100644 --- a/src/kernel/config.c +++ b/src/kernel/config.c @@ -740,7 +740,9 @@ void kernel_done(void) /* calling this function releases memory assigned to static variables, etc. * calling it is optional, e.g. a release server will most likely not do it. */ +#ifdef USE_LIBXML2 xml_done(); +#endif attrib_done(); item_done(); message_done();