From 481275aef1295cf4ffb503d53a81134eb51dc34d Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Fri, 12 Dec 2014 20:53:39 +0100 Subject: [PATCH] refactor: eliminate the getunitpeasants global variable. bugfix: it was possible to uncover stealth units with GIVE CONTROL. --- src/battle.c | 2 +- src/economy.c | 16 ++++++-------- src/give.c | 54 +++++++++++++++++++++------------------------ src/give.h | 1 + src/give.test.c | 2 -- src/kernel/config.c | 31 +++++++++++++++++++++----- src/kernel/config.h | 9 ++++++-- src/laws.c | 2 +- src/move.c | 12 +++++----- src/spy.c | 2 +- src/study.c | 4 ++-- 11 files changed, 76 insertions(+), 59 deletions(-) diff --git a/src/battle.c b/src/battle.c index c7ccf2ae8..c43ea2989 100644 --- a/src/battle.c +++ b/src/battle.c @@ -4004,7 +4004,7 @@ static bool start_battle(region * r, battle ** bp) init_order(ord); /* attackierte Einheit ermitteln */ - u2 = getunit(r, u->faction); + getunit(r, u->faction, &u2); /* Beginn Fehlerbehandlung */ /* Fehler: "Die Einheit wurde nicht gefunden" */ diff --git a/src/economy.c b/src/economy.c index 5de7fde59..566536f07 100644 --- a/src/economy.c +++ b/src/economy.c @@ -629,20 +629,18 @@ int give_control_cmd(unit * u, order * ord) region *r = u->region; unit *u2; const char *s; - param_t p; init_order(ord); - u2 = getunit(r, u->faction); - s = getstrtoken(); - p = findparam(s, u->faction->locale); + getunit(r, u->faction, &u2); - /* first, do all the ones that do not require HELP_GIVE or CONTACT */ - if (p == P_CONTROL) { + s = getstrtoken(); + if (isparam(s, u->faction->locale, P_CONTROL)) { message *msg = 0; - if (!u2 || u2->number == 0) { - msg = msg_feedback(u, ord, "feedback_unit_not_found", ""); - ADDMSG(&u->faction->msgs, msg); + if (!can_give_to(u, u2)) { + ADDMSG(&u->faction->msgs, + msg_feedback(u, ord, "feedback_unit_not_found", "")); + return; } else if (!u->building && !u->ship) { msg = cmistake(u, ord, 140, MSG_EVENT); diff --git a/src/give.c b/src/give.c index b76e735e8..7a17b2da3 100644 --- a/src/give.c +++ b/src/give.c @@ -434,7 +434,7 @@ void give_unit(unit * u, unit * u2, order * ord) cmistake(u, ord, 152, MSG_COMMERCE); } } - else if (getunitpeasants) { + else { unit *u3; for (u3 = r->units; u3; u3 = u3->next) @@ -461,10 +461,6 @@ void give_unit(unit * u, unit * u2, order * ord) cmistake(u, ord, 153, MSG_COMMERCE); } } - else { - ADDMSG(&u->faction->msgs, msg_feedback(u, ord, "feedback_unit_not_found", - "")); - } return; } @@ -520,12 +516,25 @@ void give_unit(unit * u, unit * u2, order * ord) u2->faction->newbies += n; } +bool can_give_to(unit *u, unit *u2) { + /* Damit Tarner nicht durch die Fehlermeldung enttarnt werden können */ + if (!u2 || u2->number == 0) { + return false; + } + if (u2 && !alliedunit(u2, u->faction, HELP_GIVE) + && !cansee(u->faction, u->region, u2, 0) && !ucontact(u2, u) + && !fval(u2, UFL_TAKEALL)) { + return false; + } + return true; +} + void give_cmd(unit * u, order * ord) { region *r = u->region; unit *u2; const char *s; - int n; + int err, n; const item_type *itype; param_t p; plane *pl; @@ -534,7 +543,7 @@ void give_cmd(unit * u, order * ord) kwd = init_order(ord); assert(kwd == K_GIVE); - u2 = getunit(r, u->faction); + err = getunit(r, u->faction, &u2); s = getstrtoken(); n = s ? atoip(s) : 0; p = (n > 0) ? NOPARAM : findparam(s, u->faction->locale); @@ -545,9 +554,13 @@ void give_cmd(unit * u, order * ord) return; } - if (!u2 && !getunitpeasants) { - ADDMSG(&u->faction->msgs, msg_feedback(u, ord, "feedback_unit_not_found", - "")); + if (err == GET_NOTFOUND || (err != GET_PEASANTS && !can_give_to(u, u2))) { + ADDMSG(&u->faction->msgs, + msg_feedback(u, ord, "feedback_unit_not_found", "")); + return; + } + if (u == u2) { + cmistake(u, ord, 8, MSG_COMMERCE); return; } @@ -557,19 +570,6 @@ void give_cmd(unit * u, order * ord) return; } - /* Damit Tarner nicht durch die Fehlermeldung enttarnt werden können */ - if (u2 && !alliedunit(u2, u->faction, HELP_GIVE) - && !cansee(u->faction, r, u2, 0) && !ucontact(u2, u) - && !fval(u2, UFL_TAKEALL)) { - ADDMSG(&u->faction->msgs, msg_feedback(u, ord, "feedback_unit_not_found", - "")); - return; - } - if (u == u2) { - cmistake(u, ord, 8, MSG_COMMERCE); - return; - } - /* UFL_TAKEALL ist ein grober Hack. Generalisierung tut not, ist aber nicht * wirklich einfach. */ pl = rplane(r); @@ -605,11 +605,6 @@ void give_cmd(unit * u, order * ord) msg_feedback(u, ord, "race_notake", "race", u_race(u2))); return; } - if (!u2 && !getunitpeasants) { - ADDMSG(&u->faction->msgs, msg_feedback(u, ord, - "feedback_unit_not_found", "")); - return; - } if (u->items) { item **itmp = &u->items; while (*itmp) { @@ -628,8 +623,9 @@ void give_cmd(unit * u, order * ord) itmp = &itm->next; } } - if (!given) + if (!given) { cmistake(u, ord, 38, MSG_COMMERCE); + } return; } diff --git a/src/give.h b/src/give.h index dfd53cddc..7a579d3df 100644 --- a/src/give.h +++ b/src/give.h @@ -29,6 +29,7 @@ extern "C" { void give_unit(struct unit *u, struct unit *u2, struct order *ord); void give_cmd(struct unit * u, struct order * ord); struct message * check_give(const struct unit * u, const struct unit * u2, struct order *ord); + bool can_give_to(struct unit *u, struct unit *u2); #ifdef __cplusplus } diff --git a/src/give.test.c b/src/give.test.c index 517675e4a..f4d929c31 100644 --- a/src/give.test.c +++ b/src/give.test.c @@ -43,7 +43,6 @@ static void test_give_unit_to_peasants(CuTest * tc) { env.f2 = 0; setup_give(&env); rsetpeasants(env.r, 0); - getunitpeasants = true; give_unit(env.src, NULL, NULL); CuAssertIntEquals(tc, 0, env.src->number); CuAssertIntEquals(tc, 1, env.r->land->peasants); @@ -57,7 +56,6 @@ static void test_give_unit_in_ocean(CuTest * tc) { env.f2 = 0; setup_give(&env); env.r->terrain = test_create_terrain("ocean", SEA_REGION); - getunitpeasants = true; give_unit(env.src, NULL, NULL); CuAssertIntEquals(tc, 0, env.src->number); test_cleanup(); diff --git a/src/kernel/config.c b/src/kernel/config.c index 3dd22b159..5a68597d4 100644 --- a/src/kernel/config.c +++ b/src/kernel/config.c @@ -1015,19 +1015,38 @@ int read_unitid(const faction * f, const region * r) return atoi36((const char *)s); } -/* exported symbol */ -bool getunitpeasants; - -unit *getunit(const region * r, const faction * f) +int getunit(const region * r, const faction * f, unit **uresult) +{ + int n = read_unitid(f, r); + unit *u2; + + if (n == 0) { + *uresult = 0; + return GET_PEASANTS; + } + if (n < 0) + return 0; + + *uresult = u2 = findunit(n); + if (u2 != NULL && u2->region == r) { + /* there used to be a 'u2->flags & UFL_ISNEW || u2->number>0' condition + * here, but it got removed because of a bug that made units disappear: + * http://eressea.upb.de/mantis/bug_view_page.php?bug_id=0000172 + */ + return GET_UNIT; + } + + return GET_NOTFOUND; +} + +unit *getunit_deprecated(const region * r, const faction * f) { int n = read_unitid(f, r); unit *u2; if (n == 0) { - getunitpeasants = 1; return NULL; } - getunitpeasants = 0; if (n < 0) return 0; diff --git a/src/kernel/config.h b/src/kernel/config.h index 0a1f5b242..da94ae072 100644 --- a/src/kernel/config.h +++ b/src/kernel/config.h @@ -60,6 +60,11 @@ extern "C" { #define PLAGUE_HEALCHANCE 0.25F /* Wahrscheinlichkeit Heilung */ #define PLAGUE_HEALCOST 30 /* Heilkosten */ + + /* getunit results: */ +#define GET_UNIT 0 +#define GET_NOTFOUND 1 +#define GET_PEASANTS 2 /* Bewegungsweiten: */ #define BP_WALKING 4 #define BP_RIDING 6 @@ -163,7 +168,8 @@ extern "C" { struct unit *createunit(struct region *r, struct faction *f, int number, const struct race *rc); void create_unitid(struct unit *u, int id); - struct unit *getunit(const struct region *r, const struct faction *f); + struct unit *getunit_deprecated(const struct region *r, const struct faction *f); + int getunit(const struct region * r, const struct faction * f, struct unit **uresult); int read_unitid(const struct faction *f, const struct region *r); @@ -371,7 +377,6 @@ extern "C" { extern int turn; extern int verbosity; - extern bool getunitpeasants; /** report options **/ extern const char *options[MAXOPTIONS]; diff --git a/src/laws.c b/src/laws.c index 44bec3642..6b0b229ab 100755 --- a/src/laws.c +++ b/src/laws.c @@ -1830,7 +1830,7 @@ int name_cmd(struct unit *u, struct order *ord) case P_UNIT: if (foreign) { - unit *u2 = getunit(r, u->faction); + unit *u2 = getunit_deprecated(r, u->faction); if (!u2 || !cansee(u->faction, r, u2, 0)) { ADDMSG(&u->faction->msgs, msg_feedback(u, ord, diff --git a/src/move.c b/src/move.c index b7fa6feb4..f51884937 100644 --- a/src/move.c +++ b/src/move.c @@ -1248,7 +1248,7 @@ static bool transport(unit * ut, unit * u) for (ord = ut->orders; ord; ord = ord->next) { if (getkeyword(ord) == K_TRANSPORT) { init_order(ord); - if (getunit(ut->region, ut->faction) == u) { + if (getunit_deprecated(ut->region, ut->faction) == u) { return true; } } @@ -1281,7 +1281,7 @@ static void init_transportation(void) unit *ut; init_order(u->thisorder); - ut = getunit(r, u->faction); + ut = getunit_deprecated(r, u->faction); if (ut == NULL) { ADDMSG(&u->faction->msgs, msg_feedback(u, u->thisorder, "feedback_unit_not_found", "")); @@ -1312,14 +1312,14 @@ static void init_transportation(void) if (getkeyword(ord) == K_TRANSPORT) { init_order(ord); for (;;) { - unit *ut = getunit(r, u->faction); + unit *ut = getunit_deprecated(r, u->faction); if (ut == NULL) break; if (getkeyword(ut->thisorder) == K_DRIVE && can_move(ut) && !fval(ut, UFL_NOTMOVING) && !LongHunger(ut)) { init_order(ut->thisorder); - if (getunit(r, ut->faction) == u) { + if (getunit_deprecated(r, ut->faction) == u) { w += weight(ut); } } @@ -2204,7 +2204,7 @@ static const region_list *travel_i(unit * u, const region_list * route_begin, continue; init_order(ord); - ut = getunit(r, u->faction); + ut = getunit_deprecated(r, u->faction); if (ut != NULL) { if (getkeyword(ut->thisorder) == K_DRIVE) { if (ut->building && !can_leave(ut)) { @@ -2219,7 +2219,7 @@ static const region_list *travel_i(unit * u, const region_list * route_begin, if (!fval(ut, UFL_NOTMOVING) && !LongHunger(ut)) { init_order(ut->thisorder); - if (getunit(u->region, ut->faction) == u) { + if (getunit_deprecated(u->region, ut->faction) == u) { const region_list *route_to = travel_route(ut, route_begin, route_end, ord, TRAVEL_TRANSPORTED); diff --git a/src/spy.c b/src/spy.c index 00616a47e..438cc29df 100644 --- a/src/spy.c +++ b/src/spy.c @@ -121,7 +121,7 @@ int spy_cmd(unit * u, struct order *ord) region *r = u->region; init_order(ord); - target = getunit(r, u->faction); + target = getunit_deprecated(r, u->faction); if (!target) { ADDMSG(&u->faction->msgs, msg_feedback(u, u->thisorder, diff --git a/src/study.c b/src/study.c index 6bc889a59..d76fd0353 100644 --- a/src/study.c +++ b/src/study.c @@ -390,7 +390,7 @@ int teach_cmd(unit * u, struct order *ord) init_order(ord); while (!parser_end()) { - unit *u2 = getunit(r, u->faction); + unit *u2 = getunit_deprecated(r, u->faction); bool feedback; ++count; @@ -406,7 +406,7 @@ int teach_cmd(unit * u, struct order *ord) for (j = 0; j != count - 1; ++j) { /* skip over the first 'count' units */ - getunit(r, u->faction); + getunit_deprecated(r, u->faction); } token = getstrtoken();