Bug 2606: DRY, share code between buy and sell.

src/economy.c

@@ -1189,6 +1189,18 @@ bool trade_needs_castle(const terrain_type *terrain, const race *rc)
     return rc != rc_insect && (terrain == t_swamp || terrain == t_desert);
 }
 
+static building * first_building(region *r, const struct building_type *btype, int minsize) {
+    building *b = NULL;
+    if (r->buildings) {
+        for (b = r->buildings; b; b = b->next) {
+            if (b->type == btype && b->size >= minsize) {
+                return b;
+            }
+        }
+    }
+    return NULL;
+}
+
 static void buy(unit * u, econ_request ** buyorders, struct order *ord)
 {
     char token[128];
@@ -1223,21 +1235,12 @@ static void buy(unit * u, econ_request ** buyorders, struct order *ord)
     /* Entweder man ist Insekt in Sumpf/Wueste, oder es muss
      * einen Handelsposten in der Region geben: */
     if (trade_needs_castle(r->terrain, u_race(u))) {
-        building *b = NULL;
+        static int cache;
-        if (r->buildings) {
+        static const struct building_type *castle_bt;
-            static int cache;
+        if (bt_changed(&cache)) {
-            static const struct building_type *bt_castle;
+            castle_bt = bt_find("castle");
-            if (bt_changed(&cache)) {
-                bt_castle = bt_find("castle");
-            }
-
-            for (b = r->buildings; b; b = b->next) {
-                if (b->type == bt_castle && b->size >= 2) {
-                    break;
-                }
-            }
         }
-        if (b == NULL) {
+        if (first_building(r, castle_bt, 2) == NULL) {
             cmistake(u, ord, 119, MSG_COMMERCE);
             return;
         }
@@ -1540,29 +1543,14 @@ static bool sell(unit * u, econ_request ** sellorders, struct order *ord)
             return false;
         }
     }
-    /* In der Region muss es eine Burg geben. */
 
-    if (u_race(u) == get_race(RC_INSECT)) {
+    if (trade_needs_castle(r->terrain, u_race(u))) {
-        if (r->terrain != newterrain(T_SWAMP) && r->terrain != newterrain(T_DESERT)
+        /* In der Region muss es eine Burg geben. */
-            && !rbuildings(r)) {
+        if (first_building(r, castle_bt, 2) == NULL) {
             cmistake(u, ord, 119, MSG_COMMERCE);
             return false;
         }
     }
-    else {
-        /* ...oder in der Region muss es eine Burg geben. */
-        building *b = 0;
-        if (r->buildings) {
-            for (b = r->buildings; b; b = b->next) {
-                if (b->type == castle_bt && b->size >= 2) break;
-            }
-        }
-        if (!b) {
-            cmistake(u, ord, 119, MSG_COMMERCE);
-            return false;
-        }
-    }
-
     /* Ein Haendler kann nur 10 Gueter pro Talentpunkt verkaufen. */
 
     i = u->number * 10 * effskill(u, SK_TRADE, NULL);

src/economy.test.c

@@ -237,7 +237,10 @@ static void test_trade_needs_castle(CuTest *tc) {
     /* Handeln ist nur in Regionen mit Burgen möglich. */
     race *rc;
     region *r;
+    unit *u;
+    building *b;
     const terrain_type *t_swamp;
+    const item_type *it_luxury;
 
     test_setup();
     setup_production();
@@ -246,9 +249,32 @@ static void test_trade_needs_castle(CuTest *tc) {
     init_terrains();
     t_swamp = get_terrain("swamp");
     r = setup_trade_region(tc, t_swamp);
-    rc = test_create_race(NULL);
+    it_luxury = r_luxury(r);
 
+    rc = test_create_race(NULL);
     CuAssertTrue(tc, trade_needs_castle(t_swamp, rc));
+
+    u = test_create_unit(test_create_faction(rc), r);
+    unit_addorder(u, create_order(K_BUY, u->faction->locale, "1 %s",
+        LOC(u->faction->locale, resourcename(it_luxury->rtype, 0))));
+    unit_addorder(u, create_order(K_SELL, u->faction->locale, "1 %s",
+        LOC(u->faction->locale, resourcename(it_luxury->rtype, 0))));
+    produce(r);
+    CuAssertIntEquals(tc, 2, test_count_messagetype(u->faction->msgs, "error119"));
+
+    test_clear_messages(u->faction);
+    freset(u, UFL_LONGACTION);
+    b = test_create_building(r, test_create_buildingtype("castle"));
+    b->size = 1;
+    produce(r);
+    CuAssertIntEquals(tc, 2, test_count_messagetype(u->faction->msgs, "error119"));
+
+    test_clear_messages(u->faction);
+    freset(u, UFL_LONGACTION);
+    b->size = 2;
+    test_clear_messages(u->faction);
+    produce(r);
+    CuAssertIntEquals(tc, 0, test_count_messagetype(u->faction->msgs, "error119"));
     test_teardown();
 }
 
@@ -259,7 +285,6 @@ static void test_trade_insect(CuTest *tc) {
     race *rc;
     const terrain_type *t_swamp;
     const item_type *it_luxury;
-    const item_type *it_silver;
 
     test_setup();
     setup_production();
@@ -273,21 +298,13 @@ static void test_trade_insect(CuTest *tc) {
     it_luxury = r_luxury(r);
     CuAssertTrue(tc, !trade_needs_castle(t_swamp, rc));
     CuAssertPtrNotNull(tc, it_luxury);
-    it_silver = get_resourcetype(R_SILVER)->itype;
-
     u = setup_trade_unit(tc, r, rc);
     unit_addorder(u, create_order(K_BUY, u->faction->locale, "1 %s",
         LOC(u->faction->locale, resourcename(it_luxury->rtype, 0))));
-
+    unit_addorder(u, create_order(K_SELL, u->faction->locale, "1 %s",
-    test_set_item(u, it_silver, 10);
+        LOC(u->faction->locale, resourcename(it_luxury->rtype, 0))));
-    CuAssertPtrEquals(tc, r, u->region);
-    CuAssertPtrEquals(tc, (void *)it_luxury, (void *)r_luxury(u->region));
     produce(u->region);
-    CuAssertPtrEquals(tc, NULL, test_find_messagetype(u->faction->msgs, "error119"));
+    CuAssertIntEquals(tc, 0, test_count_messagetype(u->faction->msgs, "error119"));
-    CuAssertIntEquals(tc, 1, get_item(u, it_luxury));
-    CuAssertIntEquals(tc, 5, get_item(u, it_silver));
-
-    terraform_region(r, get_terrain("swamp"));
     test_teardown();
 }
 

src/tests.c

@@ -591,6 +591,19 @@ struct message * test_find_messagetype(struct message_list *msgs, const char *na
     return test_find_messagetype_ex(msgs, name, NULL);
 }
 
+int test_count_messagetype(struct message_list *msgs, const char *name)
+{
+    int count = 0;
+    struct mlist *ml;
+    if (!msgs) return 0;
+    for (ml = msgs->begin; ml; ml = ml->next) {
+        if (strcmp(name, test_get_messagetype(ml->msg)) == 0) {
+            ++count;
+        }
+    }
+    return count;
+}
+
 void test_clear_messagelist(message_list **msgs) {
     if (*msgs) {
         free_messagelist((*msgs)->begin);

src/tests.h

@@ -64,6 +64,7 @@ extern "C" {
 
     void test_translate_param(const struct locale *lang, enum param_t param, const char *text);
     const char * test_get_messagetype(const struct message *msg);
+    int test_count_messagetype(struct message_list *msgs, const char *name);
     struct message * test_find_messagetype_ex(struct message_list *msgs, const char *name, struct message *prev);
     struct message * test_find_messagetype(struct message_list *msgs, const char *name);
     struct message * test_get_last_message(struct message_list *mlist);