Merge pull request #586 from ennorehling/bug-2234-maintenance-msg

Bug 2234: building maintenance messages

res/core/common/buildings.xml

@@ -23,7 +23,7 @@
   </building>
 
   <building name="inn" capacity="1">
-    <maintenance type="money" amount="5" variable="yes" vital="yes"/>
+    <maintenance type="money" amount="5" variable="yes"/>
     <construction skill="building" minskill="2" reqsize="10">
       <requirement type="iron" quantity="10"/>
       <requirement type="log" quantity="30"/>
@@ -34,7 +34,7 @@
 
   <building name="tunnel" capacity="1" maxsize="100">
     <maintenance type="stone" amount="2"/>
-    <maintenance type="money" amount="100" vital="yes"/>
+    <maintenance type="money" amount="100"/>
     <construction skill="building" minskill="6" reqsize="100" maxsize="100">
       <requirement type="iron" quantity="100"/>
       <requirement type="log" quantity="500"/>
@@ -45,7 +45,7 @@
 
   <building name="caravan" capacity="1" maxsize="10">
     <maintenance type="horse" amount="2"/>
-    <maintenance type="money" amount="3000" vital="yes"/>
+    <maintenance type="money" amount="3000"/>
     <construction skill="building" minskill="2" reqsize="10" maxsize="10">
       <requirement type="iron" quantity="10"/>
       <requirement type="log" quantity="50"/>
@@ -56,7 +56,7 @@
 
   <building name="dam" capacity="1" maxsize="50">
     <maintenance type="log" amount="3"/>
-    <maintenance type="money" amount="1000" vital="yes"/>
+    <maintenance type="money" amount="1000"/>
     <construction skill="building" minskill="4" reqsize="50" maxsize="50">
       <requirement type="iron" quantity="50"/>
       <requirement type="log" quantity="500"/>
@@ -75,7 +75,7 @@
   </building>
 
   <building name="stables" capacity="1">
-    <maintenance type="money" amount="150" vital="yes"/>
+    <maintenance type="money" amount="150"/>
     <construction skill="building" minskill="2">
       <requirement type="log" quantity="4"/>
       <requirement type="stone" quantity="2"/>
@@ -85,7 +85,7 @@
   </building>
 
   <building name="sawmill" capacity="1">
-    <maintenance type="money" amount="250" vital="yes"/>
+    <maintenance type="money" amount="250"/>
     <construction skill="building" minskill="3">
       <requirement type="log" quantity="5"/>
       <requirement type="stone" quantity="5"/>
@@ -96,7 +96,7 @@
 
   <building name="smithy" capacity="1">
     <function name="init" value="init_smithy"/>
-    <maintenance type="money" amount="300" vital="yes"/>
+    <maintenance type="money" amount="300"/>
     <maintenance type="log" amount="1"/>
     <construction skill="building" minskill="3">
       <requirement type="log" quantity="5"/>
@@ -107,7 +107,7 @@
   </building>
 
   <building name="magictower" maxcapacity="2" maxsize="50" magic="yes" magres="40" fumblebonus="10" auraregen="1.75">
-    <maintenance type="money" amount="1000" vital="yes"/>
+    <maintenance type="money" amount="1000"/>
     <construction skill="building" minskill="5" reqsize="50" maxsize="50">
       <requirement type="log" quantity="150"/>
       <requirement type="stone" quantity="250"/>
@@ -119,7 +119,7 @@
   </building>
 
   <building name="academy" maxcapacity="25" maxsize="25">
-    <maintenance type="money" amount="1000" vital="yes"/>
+    <maintenance type="money" amount="1000"/>
     <construction skill="building" minskill="3" reqsize="25" maxsize="25">
       <requirement type="log" quantity="125"/>
       <requirement type="stone" quantity="125"/>
@@ -129,7 +129,7 @@
   </building>
 
   <building name="harbour" capacity="1" maxcapacity="25" maxsize="25" unique="yes">
-    <maintenance type="money" amount="250" vital="yes"/>
+    <maintenance type="money" amount="250"/>
     <construction skill="building" minskill="3" reqsize="25" maxsize="25">
       <requirement type="log" quantity="125"/>
       <requirement type="stone" quantity="125"/>
@@ -138,7 +138,7 @@
   </building>
 
   <building name="quarry" capacity="1">
-    <maintenance type="money" amount="250" vital="yes"/>
+    <maintenance type="money" amount="250"/>
     <construction skill="building" minskill="2">
       <requirement type="iron" quantity="1"/>
       <requirement type="log" quantity="5"/>
@@ -148,7 +148,7 @@
   </building>
 
   <building name="mine" capacity="1">
-    <maintenance type="money" amount="500" vital="yes"/>
+    <maintenance type="money" amount="500"/>
     <construction skill="building" minskill="4">
       <requirement type="iron" quantity="1"/>
       <requirement type="log" quantity="10"/>
@@ -158,7 +158,7 @@
   </building>
 
   <building name="lighthouse" capacity="1" maxcapacity="4">
-    <maintenance type="money" amount="100" vital="yes"/>
+    <maintenance type="money" amount="100"/>
     <construction skill="building" minskill="3">
       <requirement type="iron" quantity="1"/>
       <requirement type="log" quantity="1"/>

res/e3a/buildings.xml

@@ -19,7 +19,7 @@
   </building>
 
   <building name="market" capacity="1" maxsize="10">
-    <maintenance type="money" amount="200" vital="yes"/>
+    <maintenance type="money" amount="200"/>
     <construction skill="building" minskill="3">
       <requirement type="log" quantity="1"/>
       <requirement type="stone" quantity="1"/>

src/economy.c

@@ -717,7 +717,7 @@ static int maintain(building * b)
 {
     int c;
     region *r = b->region;
-    bool paid = true, work = true;
+    bool paid = true;
     unit *u;
 
     if (fval(b, BLD_MAINTAINED) || b->type == NULL || b->type->maintenance == NULL) {
@@ -738,78 +738,40 @@ static int maintain(building * b)
             return 0;
         }
     }
-    for (c = 0; b->type->maintenance[c].number; ++c) {
+    for (c = 0; b->type->maintenance[c].number && paid; ++c) {
         const maintenance *m = b->type->maintenance + c;
         int need = m->number;
 
         if (fval(m, MTF_VARIABLE))
             need = need * b->size;
-        if (u) {
+        need -= get_pooled(u, m->rtype, GET_DEFAULT, need);
-            /* first ist im ersten versuch true, im zweiten aber false! Das
-             * bedeutet, das in der Runde in die Region geschafften Resourcen
-             * nicht genutzt werden können, weil die reserviert sind! */
-            need -= get_pooled(u, m->rtype, GET_DEFAULT, need);
-        }
         if (need > 0) {
-            if (!fval(m, MTF_VITAL))
+            paid = false;
-                work = false;
-            else {
-                paid = false;
-                break;
-            }
         }
     }
     if (fval(b, BLD_DONTPAY)) {
+        ADDMSG(&r->msgs, msg_message("maintenance_nowork", "building", b));
         return 0;
     }
-    u = building_owner(b);
+    if (!paid) {
-    if (!u) {
+        ADDMSG(&u->faction->msgs, msg_message("maintenancefail", "unit building", u, b));
+        ADDMSG(&r->msgs, msg_message("maintenance_nowork", "building", b));
         return 0;
     }
     for (c = 0; b->type->maintenance[c].number; ++c) {
         const maintenance *m = b->type->maintenance + c;
-        int need = m->number;
+        int cost = m->number;
 
-        if (fval(m, MTF_VARIABLE))
+        if (fval(m, MTF_VARIABLE)) {
-            need = need * b->size;
+            cost = cost * b->size;
-        if (u) {
-            need -= get_pooled(u, m->rtype, GET_DEFAULT, need);
-            if (need > 0) {
-                work = false;
-                if (fval(m, MTF_VITAL)) {
-                    paid = false;
-                    break;
-                }
-            }
         }
+        cost -=
+            use_pooled(u, m->rtype, GET_SLACK | GET_RESERVE | GET_POOLED_SLACK,
+                cost);
+        assert(cost == 0);
     }
-    if (paid && c > 0) {
+    ADDMSG(&u->faction->msgs, msg_message("maintenance", "unit building", u, b));
-        if (!work) {
+    return BLD_MAINTAINED;
-            ADDMSG(&u->faction->msgs, msg_message("maintenancefail", "unit building", u, b));
-            return 0;
-        }
-
-        for (c = 0; b->type->maintenance[c].number; ++c) {
-            const maintenance *m = b->type->maintenance + c;
-            int cost = m->number;
-
-            if (!fval(m, MTF_VITAL) && !work)
-                continue;
-            if (fval(m, MTF_VARIABLE))
-                cost = cost * b->size;
-
-            cost -=
-                use_pooled(u, m->rtype, GET_SLACK | GET_RESERVE | GET_POOLED_SLACK,
-                    cost);
-            assert(cost == 0);
-        }
-        if (work) {
-            ADDMSG(&u->faction->msgs, msg_message("maintenance", "unit building", u, b));
-            return BLD_MAINTAINED;
-        }
-    }
-    ADDMSG(&u->faction->msgs, msg_message("maintenancefail", "unit building", u, b));
-    return 0;
 }
 
 void maintain_buildings(region * r)
@@ -824,19 +786,6 @@ void maintain_buildings(region * r)
             flags = maintain(b);
         }
         fset(b, flags);
-
-        if (!fval(b, BLD_MAINTAINED)) {
-            unit *u = building_owner(b);
-            struct message *msg = msg_message("maintenance_nowork", "building", b);
-            if (u) {
-                add_message(&u->faction->msgs, msg);
-                r_addmessage(r, u->faction, msg);
-            }
-            else {
-                add_message(&r->msgs, msg);
-            }
-            msg_release(msg);
-        }
         bp = &b->next;
     }
 }

src/economy.test.c

@@ -241,11 +241,15 @@ static void test_tax_cmd(CuTest *tc) {
     test_cleanup();
 }
 
+/** 
+ * see https://bugs.eressea.de/view.php?id=2234
+ */
 static void test_maintain_buildings(CuTest *tc) {
     region *r;
     building *b;
     building_type *btype;
     unit *u;
+    faction *f;
     maintenance *req;
     item_type *itype;
 
@@ -253,7 +257,8 @@ static void test_maintain_buildings(CuTest *tc) {
     btype = test_create_buildingtype("Hort");
     btype->maxsize = 10;
     r = test_create_region(0, 0, 0);
-    u = test_create_unit(test_create_faction(0), r);
+    f = test_create_faction(0);
+    u = test_create_unit(f, r);
     b = test_create_building(r, btype);
     itype = test_create_itemtype("money");
     b->size = btype->maxsize;
@@ -263,6 +268,8 @@ static void test_maintain_buildings(CuTest *tc) {
     b->flags = 0;
     maintain_buildings(r);
     CuAssertIntEquals(tc, BLD_MAINTAINED, fval(b, BLD_MAINTAINED));
+    CuAssertPtrEquals(tc, 0, f->msgs);
+    CuAssertPtrEquals(tc, 0, r->msgs);
 
     req = calloc(2, sizeof(maintenance));
     req[0].number = 100;
@@ -273,19 +280,30 @@ static void test_maintain_buildings(CuTest *tc) {
     b->flags = 0;
     maintain_buildings(r);
     CuAssertIntEquals(tc, 0, fval(b, BLD_MAINTAINED));
-
+    CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "maintenancefail"));
+    CuAssertPtrNotNull(tc, test_find_messagetype(r->msgs, "maintenance_nowork"));
+    test_clear_messagelist(&f->msgs);
+    test_clear_messagelist(&r->msgs);
+    
     // we can afford to pay:
     i_change(&u->items, itype, 100);
     b->flags = 0;
     maintain_buildings(r);
     CuAssertIntEquals(tc, BLD_MAINTAINED, fval(b, BLD_MAINTAINED));
     CuAssertIntEquals(tc, 0, i_get(u->items, itype));
+    CuAssertPtrEquals(tc, 0, r->msgs);
+    CuAssertPtrEquals(tc, 0, test_find_messagetype(f->msgs, "maintenance_nowork"));
+    CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "maintenance"));
+    test_clear_messagelist(&f->msgs);
 
     // this building has no owner, it doesn't work:
     u_set_building(u, NULL);
     b->flags = 0;
     maintain_buildings(r);
     CuAssertIntEquals(tc, 0, fval(b, BLD_MAINTAINED));
+    CuAssertPtrEquals(tc, 0, f->msgs);
+    CuAssertPtrNotNull(tc, test_find_messagetype(r->msgs, "maintenance_noowner"));
+    test_clear_messagelist(&r->msgs);
 
     test_cleanup();
 }

src/kernel/building.h

@@ -31,7 +31,6 @@ extern "C" {
     /* maintenance::flags */
 #define MTF_NONE     0x00
 #define MTF_VARIABLE 0x01       /* resource usage scales with size */
-#define MTF_VITAL    0x02       /* if resource missing, building may crash */
 
     typedef struct maintenance {
         const struct resource_type *rtype;  /* type of resource required */

src/kernel/jsonconf.c

@@ -106,7 +106,7 @@ static void json_maintenance_i(cJSON *json, maintenance *mt) {
             break;
         case cJSON_Array:
             if (strcmp(child->string, "flags") == 0) {
-                const char * flags[] = { "variable", "required", 0 };
+                const char * flags[] = { "variable", 0 };
                 mt->flags = json_flags(child, flags);
             }
             else {

src/kernel/jsonconf.test.c

@@ -319,7 +319,7 @@ static void test_spells(CuTest * tc)
 static const char * building_data = "{\"buildings\": { "
 "\"house\" : { "
 "\"maintenance\" : "
-"{ \"type\" : \"iron\", \"amount\" : 1, \"flags\" : [ \"required\", \"variable\" ] }"
+"{ \"type\" : \"iron\", \"amount\" : 1, \"flags\" : [ \"variable\" ] }"
 ","
 "\"construction\" : {"
 "\"maxsize\" : 20,"
@@ -363,7 +363,7 @@ static void test_buildings(CuTest * tc)
     CuAssertPtrNotNull(tc, bt->maintenance);
     CuAssertIntEquals(tc, 1, bt->maintenance[0].number);
     CuAssertPtrEquals(tc, (void *)get_resourcetype(R_IRON), (void *)bt->maintenance[0].rtype);
-    CuAssertIntEquals(tc, MTF_VARIABLE | MTF_VITAL, bt->maintenance[0].flags);
+    CuAssertIntEquals(tc, MTF_VARIABLE, bt->maintenance[0].flags);
     CuAssertIntEquals(tc, 0, bt->maintenance[1].number);
 
     CuAssertPtrNotNull(tc, bt->construction);

src/kernel/xmlreader.c

@@ -339,9 +339,6 @@ static int parse_buildings(xmlDocPtr doc)
 
                 if (xml_bvalue(node, "variable", false))
                     mt->flags |= MTF_VARIABLE;
-                if (xml_bvalue(node, "vital", false))
-                    mt->flags |= MTF_VITAL;
-
             }
             xmlXPathFreeObject(result);
 

src/tests.c

@@ -452,6 +452,14 @@ struct message * test_find_messagetype(struct message_list *msgs, const char *na
     return test_find_messagetype_ex(msgs, name, NULL);
 }
 
+void test_clear_messagelist(message_list **msgs) {
+    if (*msgs) {
+        free_messagelist((*msgs)->begin);
+        free(*msgs);
+        *msgs = NULL;
+    }
+}
+
 void test_clear_messages(faction *f) {
     if (f->msgs) {
         free_messagelist(f->msgs->begin);

src/tests.h

@@ -61,6 +61,7 @@ extern "C" {
     struct message * test_find_messagetype(struct message_list *msgs, const char *name);
     struct message * test_get_last_message(struct message_list *mlist);
     void test_clear_messages(struct faction *f);
+    void test_clear_messagelist(struct message_list **msgs);
     void assert_message(struct CuTest * tc, struct message *msg, char *name, int numpar);
 
     void assert_pointer_parameter(struct CuTest * tc, struct message *msg, int index, void *arg);