From 18fcba3ed6af034c8d47e9cbb639b84fbdbf9eb6 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sun, 18 Sep 2016 11:14:00 +0200 Subject: [PATCH 1/4] failing test for a new bug: empty dummy units get stuck in faction.units after recruiting. --- scripts/tests/common.lua | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/scripts/tests/common.lua b/scripts/tests/common.lua index 446a7cf6d..187f78920 100644 --- a/scripts/tests/common.lua +++ b/scripts/tests/common.lua @@ -1139,3 +1139,16 @@ function test_prefix() u1.faction.locale = "en" assert_not_nil(u1:show():find("archelf")) end + +function test_recruit() + local r = region.create(0, 0, "plain") + local f = faction.create("noreply@eressea.de", "human", "de") + local u = unit.create(f, r, 1) + + u:add_item("money", 1000) + set_order(u, "REKRUTIERE 5") + process_orders() + for u in f.units do + assert_equal(6, u.number) + end +end From b3c31856aaaaf74c4463cee7554912cca54ec6c1 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sun, 18 Sep 2016 11:34:54 +0200 Subject: [PATCH 2/4] add (failing) test for recruiting into existing units. --- src/economy.c | 2 +- src/economy.h | 2 +- src/economy.test.c | 17 +++++++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/economy.c b/src/economy.c index 8d4939d4a..1eda7bf61 100644 --- a/src/economy.c +++ b/src/economy.c @@ -244,7 +244,7 @@ static recruitment *select_recruitment(request ** rop, return recruits; } -static void add_recruits(unit * u, int number, int wanted) +void add_recruits(unit * u, int number, int wanted) { region *r = u->region; assert(number <= wanted); diff --git a/src/economy.h b/src/economy.h index b6c8002c9..4bce314a9 100644 --- a/src/economy.h +++ b/src/economy.h @@ -61,7 +61,7 @@ extern "C" { void give_control(struct unit * u, struct unit * u2); void tax_cmd(struct unit * u, struct order *ord, struct request ** taxorders); void expandtax(struct region * r, struct request * taxorders); - + void add_recruits(struct unit * u, int number, int wanted); struct message * check_steal(const struct unit * u, struct order *ord); #ifdef __cplusplus diff --git a/src/economy.test.c b/src/economy.test.c index ce09aae43..9c4311c70 100644 --- a/src/economy.test.c +++ b/src/economy.test.c @@ -290,6 +290,22 @@ static void test_maintain_buildings(CuTest *tc) { test_cleanup(); } +static void test_recruit(CuTest *tc) { + unit *u; + faction *f; + + test_setup(); + f = test_create_faction(0); + u = test_create_unit(f, test_create_region(0, 0, 0)); + CuAssertIntEquals(tc, 1, u->number); + add_recruits(u, 1, 1); + CuAssertIntEquals(tc, 2, u->number); + CuAssertPtrEquals(tc, u, f->units); + CuAssertPtrEquals(tc, NULL, u->nextF); + CuAssertPtrEquals(tc, NULL, u->prevF); + test_cleanup(); +} + CuSuite *get_economy_suite(void) { CuSuite *suite = CuSuiteNew(); @@ -302,5 +318,6 @@ CuSuite *get_economy_suite(void) SUITE_ADD_TEST(suite, test_heroes_dont_recruit); SUITE_ADD_TEST(suite, test_tax_cmd); SUITE_ADD_TEST(suite, test_maintain_buildings); + SUITE_ADD_TEST(suite, test_recruit); return suite; } From bb689aa7b61d590288d8f447dd5e62965b4aa2c7 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sun, 18 Sep 2016 11:39:04 +0200 Subject: [PATCH 3/4] assert that f->units list is correct after remove_unit (fails) --- src/kernel/unit.test.c | 45 +++++++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/src/kernel/unit.test.c b/src/kernel/unit.test.c index 162bf1570..bf0284b58 100644 --- a/src/kernel/unit.test.c +++ b/src/kernel/unit.test.c @@ -405,7 +405,7 @@ static void test_unit_description(CuTest *tc) { static void test_remove_unit(CuTest *tc) { region *r; - unit *u; + unit *u1, *u2; faction *f; int uno; const resource_type *rtype; @@ -415,19 +415,42 @@ static void test_remove_unit(CuTest *tc) { rtype = get_resourcetype(R_SILVER); r = test_create_region(0, 0, 0); f = test_create_faction(0); - u = test_create_unit(f, r); - uno = u->no; + u2 = test_create_unit(f, r); + u1 = test_create_unit(f, r); + CuAssertPtrEquals(tc, u1, f->units); + CuAssertPtrEquals(tc, u2, u1->nextF); + CuAssertPtrEquals(tc, u1, u2->prevF); + CuAssertPtrEquals(tc, 0, u2->nextF); + uno = u1->no; region_setresource(r, rtype, 0); - i_change(&u->items, rtype->itype, 100); - remove_unit(&r->units, u); - CuAssertIntEquals(tc, 100, region_getresource(r, rtype)); - CuAssertIntEquals(tc, 0, u->number); - CuAssertPtrEquals(tc, 0, u->region); - CuAssertPtrEquals(tc, 0, u->items); - CuAssertPtrEquals(tc, 0, u->nextF); - CuAssertPtrEquals(tc, 0, r->units); + i_change(&u1->items, rtype->itype, 100); + remove_unit(&r->units, u1); + CuAssertIntEquals(tc, 0, u1->number); + CuAssertPtrEquals(tc, 0, u1->region); + // money is given to a survivor: + CuAssertPtrEquals(tc, 0, u1->items); + CuAssertIntEquals(tc, 0, region_getresource(r, rtype)); + CuAssertIntEquals(tc, 100, i_get(u2->items, rtype->itype)); + + // unit is removed from f->units: + CuAssertPtrEquals(tc, 0, u1->nextF); + CuAssertPtrEquals(tc, u2, f->units); + CuAssertPtrEquals(tc, 0, u2->nextF); + CuAssertPtrEquals(tc, 0, u2->prevF); + // unit is no longer in r->units: + CuAssertPtrEquals(tc, u2, r->units); + CuAssertPtrEquals(tc, 0, u2->next); + + // unit is in deleted_units: CuAssertPtrEquals(tc, 0, findunit(uno)); CuAssertPtrEquals(tc, f, dfindhash(uno)); + + remove_unit(&r->units, u2); + // no survivor, give money to peasants: + CuAssertIntEquals(tc, 100, region_getresource(r, rtype)); + // there are now no more units: + CuAssertPtrEquals(tc, 0, r->units); + CuAssertPtrEquals(tc, 0, f->units); test_cleanup(); } From 6c9c460815f2cb3ffc40fa18dc537f9d1e0ec4e3 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sun, 18 Sep 2016 11:46:54 +0200 Subject: [PATCH 4/4] fix failing unit tests, remove deleted unit from f->units list. --- src/kernel/unit.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/kernel/unit.c b/src/kernel/unit.c index fb4723f29..f0636b681 100644 --- a/src/kernel/unit.c +++ b/src/kernel/unit.c @@ -439,6 +439,9 @@ int remove_unit(unit ** ulist, unit * u) *ulist = u->next; } + if (u->faction && u->faction->units == u) { + u->faction->units = u->nextF; + } if (u->prevF) { u->prevF->nextF = u->nextF; }