refactor: eliminate the getunitpeasants global variable.

bugfix: it was possible to uncover stealth units with GIVE CONTROL.

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" */

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);
+    getunit(r, u->faction, &u2);
-    s = getstrtoken();
-    p = findparam(s, u->faction->locale);
 
-    /* first, do all the ones that do not require HELP_GIVE or CONTACT */
+    s = getstrtoken();
-    if (p == P_CONTROL) {
+    if (isparam(s, u->faction->locale, P_CONTROL)) {
         message *msg = 0;
 
-        if (!u2 || u2->number == 0) {
+        if (!can_give_to(u, u2)) {
-            msg = msg_feedback(u, ord, "feedback_unit_not_found", "");
+            ADDMSG(&u->faction->msgs,
-            ADDMSG(&u->faction->msgs, msg);
+                msg_feedback(u, ord, "feedback_unit_not_found", ""));
+            return;
         }
         else if (!u->building && !u->ship) {
             msg = cmistake(u, ord, 140, MSG_EVENT);

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) {
+    if (err == GET_NOTFOUND || (err != GET_PEASANTS && !can_give_to(u, u2))) {
-        ADDMSG(&u->faction->msgs, msg_feedback(u, ord, "feedback_unit_not_found",
+        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;
     }
 

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
 }

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();

src/kernel/config.c

@@ -1015,19 +1015,38 @@ int read_unitid(const faction * f, const region * r)
     return atoi36((const char *)s);
 }
 
-/* exported symbol */
+int getunit(const region * r, const faction * f, unit **uresult)
-bool getunitpeasants;
+{
-
+    int n = read_unitid(f, r);
-unit *getunit(const region * r, const faction * f)
+    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;
 

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];

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,

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);

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,

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();