Merge pull request #66 from badgerman/getunit-refactor

refactoring getunit

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 0;
         }
         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

@@ -1013,32 +1013,32 @@ 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;
-
-unit *getunit(const region * r, const faction * f)
 {
     int n = read_unitid(f, r);
-    unit *u2;
+    int result = GET_NOTFOUND;
+    unit *u2 = NULL;
 
     if (n == 0) {
-        getunitpeasants = 1;
+        result = GET_PEASANTS;
-        return NULL;
     }
-    getunitpeasants = 0;
+    else if (n>0) {
-    if (n < 0)
-        return 0;
-
         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 u2;
+            result = GET_UNIT;
         }
-
+        else {
-    return NULL;
+            u2 = NULL;
+        }
+    }
+    if (uresult) {
+        *uresult = u2;
+    }
+    return result;
 }
 
 /* - String Listen --------------------------------------------- */

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,7 @@ 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);
+    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 +376,6 @@ extern "C" {
 
     extern int turn;
     extern int verbosity;
-    extern bool getunitpeasants;
 
     /** report options **/
     extern const char *options[MAXOPTIONS];

src/kernel/config.test.c

@@ -2,10 +2,68 @@
 #include <stdlib.h>
 
 #include <kernel/config.h>
+#include <kernel/terrain.h>
+#include <kernel/unit.h>
+#include <kernel/order.h>
+#include <util/language.h>
+#include <util/base36.h>
+#include <util/attrib.h>
 
 #include <CuTest.h>
 #include <tests.h>
 
+struct critbit_tree;
+
+static void test_getunit(CuTest *tc) {
+    unit *u, *u2;
+    order *ord;
+    attrib *a;
+    struct region *r;
+    struct locale *lang;
+    struct terrain_type *t_plain;
+    struct critbit_tree ** cb;
+
+    test_cleanup();
+    lang = get_or_create_locale("de");
+    cb = (struct critbit_tree **)get_translations(lang, UT_PARAMS);
+    add_translation(cb, "TEMP", P_TEMP);
+    /* note that the english order is FIGHT, not COMBAT, so this is a poor example */
+    t_plain = test_create_terrain("plain", LAND_REGION);
+    u = test_create_unit(test_create_faction(0), test_create_region(0, 0, t_plain));
+    a = a_add(&u->attribs, a_new(&at_alias));
+    a->data.i = atoi36("42"); /* this unit is also TEMP 42 */
+    r = test_create_region(1, 0, t_plain);
+
+    ord = create_order(K_GIVE, lang, itoa36(u->no));
+    init_order(ord);
+    CuAssertIntEquals(tc, GET_UNIT, getunit(u->region, u->faction, &u2));
+    CuAssertPtrEquals(tc, u, u2);
+    init_order(ord);
+    CuAssertIntEquals(tc, GET_NOTFOUND, getunit(r, u->faction, &u2));
+    CuAssertPtrEquals(tc, NULL, u2);
+    free_order(ord);
+
+    ord = create_order(K_GIVE, lang, itoa36(u->no+1));
+    init_order(ord);
+    CuAssertIntEquals(tc, GET_NOTFOUND, getunit(u->region, u->faction, &u2));
+    CuAssertPtrEquals(tc, NULL, u2);
+    free_order(ord);
+
+    ord = create_order(K_GIVE, lang, "0");
+    init_order(ord);
+    CuAssertIntEquals(tc, GET_PEASANTS, getunit(u->region, u->faction, &u2));
+    CuAssertPtrEquals(tc, NULL, u2);
+    free_order(ord);
+
+    ord = create_order(K_GIVE, lang, "TEMP 42");
+    init_order(ord);
+    CuAssertIntEquals(tc, GET_UNIT, getunit(u->region, u->faction, &u2));
+    CuAssertPtrEquals(tc, u, u2);
+    free_order(ord);
+
+    test_cleanup();
+}
+
 static void test_get_set_param(CuTest * tc)
 {
     struct param *par = 0;
@@ -42,6 +100,7 @@ static void test_param_flt(CuTest * tc)
 CuSuite *get_config_suite(void)
 {
   CuSuite *suite = CuSuiteNew();
+  SUITE_ADD_TEST(suite, test_getunit);
   SUITE_ADD_TEST(suite, test_get_set_param);
   SUITE_ADD_TEST(suite, test_param_int);
   SUITE_ADD_TEST(suite, test_param_flt);

src/laws.c

@@ -1828,8 +1828,9 @@ int name_cmd(struct unit *u, struct order *ord)
 
     case P_UNIT:
         if (foreign) {
-            unit *u2 = getunit(r, u->faction);
+            unit *u2 = 0;
 
+            getunit(r, u->faction, &u2);
             if (!u2 || !cansee(u->faction, r, u2, 0)) {
                 ADDMSG(&u->faction->msgs, msg_feedback(u, ord,
                     "feedback_unit_not_found", ""));

src/move.c

@@ -1247,8 +1247,10 @@ static bool transport(unit * ut, unit * u)
 
     for (ord = ut->orders; ord; ord = ord->next) {
         if (getkeyword(ord) == K_TRANSPORT) {
+            unit *u2;
             init_order(ord);
-            if (getunit(ut->region, ut->faction) == u) {
+            getunit(ut->region, ut->faction, &u2);
+            if (u2 == u) {
                 return true;
             }
         }
@@ -1278,11 +1280,10 @@ static void init_transportation(void)
         for (u = r->units; u; u = u->next) {
             if (getkeyword(u->thisorder) == K_DRIVE && can_move(u)
                 && !fval(u, UFL_NOTMOVING) && !LongHunger(u)) {
-                unit *ut;
+                unit *ut = 0;
 
                 init_order(u->thisorder);
-                ut = getunit(r, u->faction);
+                if (getunit(r, u->faction, &ut) != GET_UNIT) {
-                if (ut == NULL) {
                     ADDMSG(&u->faction->msgs, msg_feedback(u, u->thisorder,
                         "feedback_unit_not_found", ""));
                     continue;
@@ -1312,14 +1313,18 @@ static void init_transportation(void)
                     if (getkeyword(ord) == K_TRANSPORT) {
                         init_order(ord);
                         for (;;) {
-                            unit *ut = getunit(r, u->faction);
+                            unit *ut = 0;
 
-                            if (ut == NULL)
+                            if (getunit(r, u->faction, &ut) != GET_UNIT) {
                                 break;
-                            if (getkeyword(ut->thisorder) == K_DRIVE && can_move(ut)
+                            }
-                                && !fval(ut, UFL_NOTMOVING) && !LongHunger(ut)) {
+                            if (getkeyword(ut->thisorder) == K_DRIVE &&
+                                    can_move(ut) && !fval(ut, UFL_NOTMOVING) &&
+                                    !LongHunger(ut)) {
+                                unit *u2;
                                 init_order(ut->thisorder);
-                                if (getunit(r, ut->faction) == u) {
+                                getunit(r, ut->faction, &u2);
+                                if (u2 == u) {
                                     w += weight(ut);
                                 }
                             }
@@ -2198,14 +2203,13 @@ static const region_list *travel_i(unit * u, const region_list * route_begin,
 
     /* transportation */
     for (ord = u->orders; ord; ord = ord->next) {
-        unit *ut;
+        unit *ut = 0;
 
         if (getkeyword(ord) != K_TRANSPORT)
             continue;
 
         init_order(ord);
-        ut = getunit(r, u->faction);
+        if (getunit(r, u->faction, &ut) == GET_UNIT) {
-        if (ut != NULL) {
             if (getkeyword(ut->thisorder) == K_DRIVE) {
                 if (ut->building && !can_leave(ut)) {
                     cmistake(ut, ut->thisorder, 150, MSG_MOVE);
@@ -2218,8 +2222,10 @@ static const region_list *travel_i(unit * u, const region_list * route_begin,
                     bool found = false;
 
                     if (!fval(ut, UFL_NOTMOVING) && !LongHunger(ut)) {
+                        unit *u2;
                         init_order(ut->thisorder);
-                        if (getunit(u->region, ut->faction) == u) {
+                        getunit(u->region, ut->faction, &u2);
+                        if (u2 == u) {
                             const region_list *route_to =
                                 travel_route(ut, route_begin, route_end, ord,
                                 TRAVEL_TRANSPORTED);

src/spy.c

@@ -91,7 +91,8 @@ void spy_message(int spy, const unit * u, const unit * target)
                 found++;
                 if (first == 1) {
                     first = 0;
-        } else {
+                }
+                else {
                     strncat(buf, ", ", sizeof(buf) - 1);
                 }
                 strncat(buf, (const char *)skillname((skill_t)sv->id, u->faction->locale),
@@ -121,11 +122,11 @@ int spy_cmd(unit * u, struct order *ord)
     region *r = u->region;
 
     init_order(ord);
-  target = getunit(r, u->faction);
+    getunit(r, u->faction, &target);
 
     if (!target) {
-    ADDMSG(&u->faction->msgs, msg_feedback(u, u->thisorder,
+        ADDMSG(&u->faction->msgs,
-        "feedback_unit_not_found", ""));
+            msg_feedback(u, u->thisorder, "feedback_unit_not_found", ""));
         return 0;
     }
     if (!can_contact(r, u, target)) {
@@ -145,7 +146,8 @@ int spy_cmd(unit * u, struct order *ord)
     if (chance(spychance)) {
         produceexp(u, SK_SPY, u->number);
         spy_message(spy, u, target);
-  } else {
+    }
+    else {
         ADDMSG(&u->faction->msgs, msg_message("spyfail", "spy target", u, target));
     }
 
@@ -289,7 +291,8 @@ int setstealth_cmd(unit * u, struct order *ord)
             if (!s || *s == 0) {
                 fset(u, UFL_ANON_FACTION);
                 break;
-        } else if (findparam(s, u->faction->locale) == P_NOT) {
+            }
+            else if (findparam(s, u->faction->locale) == P_NOT) {
                 freset(u, UFL_ANON_FACTION);
                 break;
             }
@@ -305,12 +308,14 @@ int setstealth_cmd(unit * u, struct order *ord)
                 if (!s2 || *s2 == 0 || nr == u->faction->no) {
                     a_removeall(&u->attribs, &at_otherfaction);
                     break;
-          } else {
+                }
+                else {
                     struct faction *f = findfaction(nr);
                     if (f == NULL) {
                         cmistake(u, ord, 66, MSG_EVENT);
                         break;
-            } else {
+                    }
+                    else {
                         set_factionstealth(u, f);
                         break;
                     }
@@ -327,7 +332,8 @@ int setstealth_cmd(unit * u, struct order *ord)
     default:
         if (u_race(u)->flags & RCF_SHAPESHIFTANY) {
             set_racename(&u->attribs, s);
-      } else {
+        }
+        else {
             cmistake(u, ord, 289, MSG_EVENT);
         }
     }
@@ -365,7 +371,8 @@ static int try_destruction(unit * u, unit * u2, const ship * sh, int skilldiff)
             ADDMSG(&u2->faction->msgs, msg_message(detect_failure_msg, "ship", sh));
         }
         return 0;
-  } else if (skilldiff < 0) {
+    }
+    else if (skilldiff < 0) {
         /* tell the unit that the attempt was detected: */
         ADDMSG(&u2->faction->msgs, msg_message(destruction_detected_msg,
             "ship unit", sh, u));
@@ -374,7 +381,8 @@ static int try_destruction(unit * u, unit * u2, const ship * sh, int skilldiff)
             ADDMSG(&u2->faction->msgs, msg_message(detect_failure_msg, "ship", sh));
         }
         return 0;
-  } else {
+    }
+    else {
         /* tell the unit that the attempt succeeded */
         ADDMSG(&u->faction->msgs, msg_message(destruction_success_msg, "ship unit",
             sh, u));
@@ -406,7 +414,8 @@ static void sink_ship(region * r, ship * sh, const char *name, unit * saboteur)
     /* figure out what a unit's chances of survival are: */
     if (!fval(r->terrain, SEA_REGION)) {
         probability = CANAL_SWIMMER_CHANCE;
-  } else {
+    }
+    else {
         for (d = 0; d != MAXDIRECTIONS; ++d) {
             region *rn = rconnect(r, d);
             if (!fval(rn->terrain, SEA_REGION) && !move_blocked(NULL, r, rn)) {
@@ -442,7 +451,8 @@ static void sink_ship(region * r, ship * sh, const char *name, unit * saboteur)
                 if (dead > 0) {
                     msg =
                         msg_message("sink_lost_msg", "dead region unit", dead, safety, u);
-        } else {
+                }
+                else {
                     msg = msg_message("sink_saved_msg", "region unit", safety, u);
                 }
                 leave_ship(u);
@@ -453,7 +463,8 @@ static void sink_ship(region * r, ship * sh, const char *name, unit * saboteur)
                     i_remove(&u->items, u->items);
                 }
                 move_unit(u, safety, NULL);
-      } else {
+            }
+            else {
                 msg = msg_message("sink_lost_msg", "dead region unit", dead, NULL, u);
             }
             add_message(&u->faction->msgs, msg);

src/study.c

@@ -380,8 +380,10 @@ int teach_cmd(unit * u, struct order *ord)
         init_order(ord);
 
         while (!parser_end()) {
-            unit *u2 = getunit(r, u->faction);
+            unit *u2;
             bool feedback;
+
+            getunit(r, u->faction, &u2);
             ++count;
 
             /* Falls die Unit nicht gefunden wird, Fehler melden */
@@ -396,7 +398,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(r, u->faction, NULL);
                 }
 
                 token = getstrtoken();