update key attribute api

prepare to replace at_key. do not return the internals. add basic test.

src/attributes/CMakeLists.txt

@@ -1,6 +1,7 @@
 PROJECT(attributes C)
 SET(_TEST_FILES
 stealth.test.c
+key.test.c
 otherfaction.test.c
 )
 

src/attributes/iceberg.c

@@ -30,6 +30,7 @@ attrib_type at_iceberg = {
     NULL,
     a_writeint,
     a_readint,
+    NULL,
     ATF_UNIQUE
 };
 

src/attributes/key.c

@@ -22,6 +22,68 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 #include <kernel/save.h>
 #include <util/attrib.h>
+#include <storage.h>
+
+#include <stdlib.h>
+#include <assert.h>
+
+static void a_writekeys(const attrib *a, const void *o, storage *store) {
+    int i, *keys = (int *)a->data.v;
+    for (i = 0; i != keys[0]; ++i) {
+        WRITE_INT(store, keys[i]);
+    }
+}
+
+static int a_readkeys(attrib * a, void *owner, struct storage *store) {
+    int i, *p = 0;
+    READ_INT(store, &i);
+    assert(i < 4096 && i>0);
+    a->data.v = p = malloc(sizeof(int)*(i + 1));
+    *p++ = i;
+    while (--i) {
+        READ_INT(store, p++);
+    }
+    return AT_READ_OK;
+}
+
+static int a_readkey(attrib *a, void *owner, struct storage *store) {
+    int res = a_readint(a, owner, store);
+    return (res != AT_READ_FAIL) ? AT_READ_DEPR : res;
+}
+
+attrib_type at_keys = {
+    "keys",
+    NULL,
+    NULL,
+    NULL,
+    a_writekeys,
+    a_readkeys,
+    NULL
+};
+
+void a_upgradekeys(attrib **alist, attrib *abegin) {
+    int n = 0, *keys = 0;
+    int i = 0, val[4];
+    attrib *a, *ak = a_find(*alist, &at_keys);
+    if (!ak) {
+        ak = a_add(alist, a_new(&at_keys));
+        keys = (int *)ak->data.v;
+        n = keys ? keys[0] : 0;
+    }
+    for (a = abegin; a && a->type == abegin->type; a = a->next) {
+        val[i++] = a->data.i;
+        if (i == 4) {
+            keys = realloc(keys, sizeof(int) * (n + i + 1));
+            memcpy(keys + n + 1, val, sizeof(int)*i);
+            n += i;
+        }
+    }
+    if (i > 0) {
+        keys = realloc(keys, sizeof(int) * (n + i + 1));
+        memcpy(keys + n + 1, val, sizeof(int)*i);
+    }
+    keys[0] = n + i;
+}
 
 attrib_type at_key = {
     "key",
@@ -29,29 +91,51 @@ attrib_type at_key = {
     NULL,
     NULL,
     a_writeint,
-    a_readint,
+    a_readkey,
+    a_upgradekeys
 };
 
-attrib *add_key(attrib ** alist, int key)
+static attrib *make_key(int key)
-{
-    attrib *a = find_key(*alist, key);
-    if (a == NULL)
-        a = a_add(alist, make_key(key));
-    return a;
-}
-
-attrib *make_key(int key)
 {
     attrib *a = a_new(&at_key);
+    assert(key != 0);
     a->data.i = key;
     return a;
 }
 
-attrib *find_key(attrib * alist, int key)
+static attrib *find_key(attrib * alist, int key)
 {
     attrib *a = a_find(alist, &at_key);
+    assert(key != 0);
     while (a && a->type == &at_key && a->data.i != key) {
         a = a->next;
     }
     return (a && a->type == &at_key) ? a : NULL;
 }
+
+void key_set(attrib ** alist, int key)
+{
+    attrib *a;
+    assert(key != 0);
+    a = find_key(*alist, key);
+    if (!a) {
+        a = a_add(alist, make_key(key));
+    }
+}
+
+void key_unset(attrib ** alist, int key)
+{
+    attrib *a;
+    assert(key != 0);
+    a = find_key(*alist, key);
+    if (a) {
+        a_remove(alist, a);
+    }
+}
+
+bool key_get(attrib *alist, int key) {
+    attrib *a;
+    assert(key != 0);
+    a = find_key(alist, key);
+    return a != NULL;
+}

src/attributes/key.h

@@ -24,9 +24,9 @@ extern "C" {
 
     extern struct attrib_type at_key;
 
-    struct attrib *make_key(int key);
+    void key_set(struct attrib **alist, int key);
-    struct attrib *find_key(struct attrib *alist, int key);
+    void key_unset(struct attrib **alist, int key);
-    struct attrib *add_key(struct attrib **alist, int key);
+    bool key_get(struct attrib *alist, int key);
 
 #ifdef __cplusplus
 }

src/attributes/key.test.c

@@ -0,0 +1,20 @@
+#include <platform.h>
+#include "key.h"
+
+#include <util/attrib.h>
+#include <CuTest.h>
+
+static void test_get_set_keys(CuTest *tc) {
+    attrib *a = 0;
+    key_set(&a, 42);
+    CuAssertTrue(tc, key_get(a, 42));
+    key_unset(&a, 42);
+    CuAssertTrue(tc, !key_get(a, 42));
+}
+
+CuSuite *get_key_suite(void)
+{
+    CuSuite *suite = CuSuiteNew();
+    SUITE_ADD_TEST(suite, test_get_set_keys);
+    return suite;
+}

src/attributes/matmod.c

@@ -28,6 +28,7 @@ attrib_type at_matmod = {
     NULL,
     NULL,
     NULL,
+    NULL,
     ATF_PRESERVE
 };
 

src/attributes/orcification.c

@@ -28,7 +28,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
 attrib_type at_orcification = {
-    "orcification", NULL, NULL, NULL, a_writeint, a_readint, ATF_UNIQUE
+    "orcification", NULL, NULL, NULL, a_writeint, a_readint, NULL, ATF_UNIQUE
 };
 
 attrib *make_orcification(int orcification)

src/attributes/otherfaction.c

@@ -53,7 +53,7 @@ int read_of(struct attrib *a, void *owner, struct storage *store)
 }
 
 attrib_type at_otherfaction = {
-    "otherfaction", NULL, NULL, NULL, write_of, read_of, ATF_UNIQUE
+    "otherfaction", NULL, NULL, NULL, write_of, read_of, NULL, ATF_UNIQUE
 };
 
 struct faction *get_otherfaction(const struct attrib *a)

src/attributes/raceprefix.c

@@ -28,7 +28,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 #include <stdlib.h>
 
 attrib_type at_raceprefix = {
-    "raceprefix", NULL, a_finalizestring, NULL, a_writestring, a_readstring,
+    "raceprefix", NULL, a_finalizestring, NULL, a_writestring, a_readstring, NULL,
     ATF_UNIQUE
 };
 

src/attributes/reduceproduction.c

@@ -49,6 +49,7 @@ attrib_type at_reduceproduction = {
     age_reduceproduction,
     a_writeshorts,
     a_readshorts,
+    NULL,
     ATF_UNIQUE
 };
 

src/attributes/targetregion.c

@@ -52,6 +52,7 @@ attrib_type at_targetregion = {
     NULL,
     write_targetregion,
     read_targetregion,
+    NULL,
     ATF_UNIQUE
 };
 

src/bind_region.c

@@ -542,11 +542,9 @@ static int tolua_region_getkey(lua_State * L)
 {
     region *self = (region *)tolua_tousertype(L, 1, 0);
     const char *name = tolua_tostring(L, 2, 0);
-
     int flag = atoi36(name);
-    attrib *a = find_key(self->attribs, flag);
-    lua_pushboolean(L, a != NULL);
 
+    lua_pushboolean(L, key_get(self->attribs, flag));
     return 1;
 }
 
@@ -555,14 +553,13 @@ static int tolua_region_setkey(lua_State * L)
     region *self = (region *)tolua_tousertype(L, 1, 0);
     const char *name = tolua_tostring(L, 2, 0);
     int value = tolua_toboolean(L, 3, 0);
-
     int flag = atoi36(name);
-    attrib *a = find_key(self->attribs, flag);
+
-    if (a == NULL && value) {
+    if (value) {
-        add_key(&self->attribs, flag);
+        key_set(&self->attribs, flag);
     }
-    else if (a != NULL && !value) {
+    else {
-        a_remove(&self->attribs, a);
+        key_unset(&self->attribs, flag);
     }
     return 0;
 }

src/bind_unit.c

@@ -807,8 +807,7 @@ static int tolua_unit_get_flag(lua_State * L)
     unit *self = (unit *)tolua_tousertype(L, 1, 0);
     const char *name = tolua_tostring(L, 2, 0);
     int flag = atoi36(name);
-    attrib *a = find_key(self->attribs, flag);
+    lua_pushboolean(L, key_get(self->attribs, flag));
-    lua_pushboolean(L, (a != NULL));
     return 1;
 }
 
@@ -818,12 +817,11 @@ static int tolua_unit_set_flag(lua_State * L)
     const char *name = tolua_tostring(L, 2, 0);
     int value = (int)tolua_tonumber(L, 3, 0);
     int flag = atoi36(name);
-    attrib *a = find_key(self->attribs, flag);
+    if (value) {
-    if (a == NULL && value) {
+        key_set(&self->attribs, flag);
-        add_key(&self->attribs, flag);
     }
-    else if (a != NULL && !value) {
+    else {
-        a_remove(&self->attribs, a);
+        key_unset(&self->attribs, flag);
     }
     return 0;
 }

src/bindings.c

@@ -186,8 +186,8 @@ static int tolua_getkey(lua_State * L)
 {
     const char *name = tolua_tostring(L, 1, 0);
     int flag = atoi36(name);
-    attrib *a = find_key(global.attribs, flag);
+
-    lua_pushboolean(L, a != NULL);
+    lua_pushboolean(L, key_get(global.attribs, flag));
     return 1;
 }
 
@@ -209,12 +209,12 @@ static int tolua_setkey(lua_State * L)
     const char *name = tolua_tostring(L, 1, 0);
     int value = tolua_toboolean(L, 2, 0);
     int flag = atoi36(name);
-    attrib *a = find_key(global.attribs, flag);
+    bool isset = key_get(global.attribs, flag);
-    if (a == NULL && value) {
+    if (value) {
-        add_key(&global.attribs, flag);
+        key_set(&global.attribs, flag);
     }
-    else if (a != NULL && !value) {
+    else {
-        a_remove(&global.attribs, a);
+        key_unset(&global.attribs, flag);
     }
     return 0;
 }

src/chaos.c

@@ -50,6 +50,7 @@ attrib_type at_chaoscount = {
     DEFAULT_AGE,
     a_writeint,
     a_readint,
+    NULL,
     ATF_UNIQUE
 };
 

src/guard.c

@@ -38,6 +38,7 @@ attrib_type at_guard = {
     DEFAULT_AGE,
     a_writeint,
     a_readint,
+    NULL,
     ATF_UNIQUE
 };
 

src/kernel/alliance.c

@@ -480,7 +480,7 @@ int victorycondition(const alliance * al, const char *name)
 
         for (qi = 0; flist; ql_advance(&flist, &qi, 1)) {
             faction *f = (faction *)ql_get(flist, qi);
-            if (find_key(f->attribs, atoi36("phnx"))) {
+            if (key_get(f->attribs, atoi36("phnx"))) {
                 return 1;
             }
         }
@@ -509,7 +509,7 @@ int victorycondition(const alliance * al, const char *name)
 
         for (qi = 0; flist; ql_advance(&flist, &qi, 1)) {
             faction *f = (faction *)ql_get(flist, qi);
-            if (find_key(f->attribs, atoi36("pyra"))) {
+            if (key_get(f->attribs, atoi36("pyra"))) {
                 return 1;
             }
         }

src/kernel/ally.c

@@ -121,6 +121,7 @@ attrib_type at_npcfaction = {
     NULL,
     a_writeint,
     a_readint,
+    NULL,
     ATF_UNIQUE
 };
 

src/kernel/building.c

@@ -141,7 +141,7 @@ int buildingcapacity(const building * b)
 }
 
 attrib_type at_building_generic_type = {
-    "building_generic_type", NULL, NULL, NULL, a_writestring, a_readstring,
+    "building_generic_type", NULL, NULL, NULL, a_writestring, a_readstring, NULL,
     ATF_UNIQUE
 };
 

src/kernel/curse.c

@@ -285,6 +285,7 @@ attrib_type at_curse = {
     curse_age,
     curse_write,
     curse_read,
+    NULL,
     ATF_CURSE
 };
 

src/kernel/faction.c

@@ -798,6 +798,7 @@ attrib_type at_maxmagicians = {
     NULL,
     a_writeint,
     a_readint,
+    NULL,
     ATF_UNIQUE
 };
 

src/kernel/group.c

@@ -119,7 +119,7 @@ write_group(const attrib * a, const void *owner, struct storage *store)
 attrib_type at_group = {        /* attribute for units assigned to a group */
     "grp",
     DEFAULT_INIT,
-    DEFAULT_FINALIZE, DEFAULT_AGE, write_group, read_group, ATF_UNIQUE };
+    DEFAULT_FINALIZE, DEFAULT_AGE, write_group, read_group, NULL, ATF_UNIQUE };
 
 void free_group(group * g)
 {

src/kernel/item.c

@@ -926,14 +926,14 @@ struct order *ord)
             "unit item region command", user, itype->rtype, user->region, ord));
         return -1;
     }
-    if (!is_mage(user) || find_key(f->attribs, atoi36("mbst")) != NULL) {
+    if (!is_mage(user) || key_get(f->attribs, atoi36("mbst"))) {
         cmistake(user, user->thisorder, 214, MSG_EVENT);
         return -1;
     }
     use_pooled(user, itype->rtype, GET_SLACK | GET_RESERVE | GET_POOLED_SLACK,
         user->number);
 
-    a_add(&f->attribs, make_key(atoi36("mbst")));
+    key_set(&f->attribs, atoi36("mbst"));
     set_level(user, SK_MAGIC, 3);
 
     ADDMSG(&user->faction->msgs, msg_message("use_item",

src/kernel/region.c

@@ -483,6 +483,7 @@ attrib_type at_horseluck = {
     DEFAULT_AGE,
     NO_WRITE,
     NO_READ,
+    NULL,
     ATF_UNIQUE
 };
 
@@ -496,6 +497,7 @@ attrib_type at_peasantluck = {
     DEFAULT_AGE,
     NO_WRITE,
     NO_READ,
+    NULL,
     ATF_UNIQUE
 };
 
@@ -509,6 +511,7 @@ attrib_type at_deathcount = {
     DEFAULT_AGE,
     a_writeint,
     a_readint,
+    NULL,
     ATF_UNIQUE
 };
 
@@ -522,6 +525,7 @@ attrib_type at_woodcount = {
     DEFAULT_AGE,
     NO_WRITE,
     a_readint,
+    NULL,
     ATF_UNIQUE
 };
 

src/kernel/save.test.c

@@ -70,8 +70,8 @@ static void test_readwrite_attrib(CuTest *tc) {
     test_cleanup();
     data = gamedata_open(path, "wb");
     CuAssertPtrNotNull(tc, data);
-    add_key(&a, 41);
+    key_set(&a, 41);
-    add_key(&a, 42);
+    key_set(&a, 42);
     write_attribs(data->store, a, NULL);
     gamedata_close(data);
     a_removeall(&a, NULL);
@@ -81,8 +81,8 @@ static void test_readwrite_attrib(CuTest *tc) {
     CuAssertPtrNotNull(tc, data);
     read_attribs(data->store, &a, NULL);
     gamedata_close(data);
-    CuAssertPtrNotNull(tc, find_key(a, 41));
+    CuAssertTrue(tc, key_get(a, 41));
-    CuAssertPtrNotNull(tc, find_key(a, 42));
+    CuAssertTrue(tc, key_get(a, 42));
     a_removeall(&a, NULL);
 
     CuAssertIntEquals(tc, 0, remove(path));

src/kernel/skills.c

@@ -59,6 +59,7 @@ attrib_type at_skillmod = {
     NULL,
     NULL,                         /* can't write function pointers */
     NULL,                         /* can't read function pointers */
+    NULL,
     ATF_PRESERVE
 };
 

src/laws.c

@@ -486,6 +486,7 @@ attrib_type at_germs = {
     DEFAULT_AGE,
     a_writeshorts,
     a_readshorts,
+    NULL,
     ATF_UNIQUE
 };
 
@@ -2749,7 +2750,7 @@ void sinkships(struct region * r)
 
 static attrib_type at_number = {
     "faction_renum",
-    NULL, NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL, NULL, NULL,
     ATF_UNIQUE
 };
 

src/magic.c

@@ -335,6 +335,7 @@ attrib_type at_mage = {
     NULL,
     write_mage,
     read_mage,
+    NULL,
     ATF_UNIQUE
 };
 
@@ -2416,6 +2417,7 @@ attrib_type at_familiarmage = {
     age_unit,
     a_write_unit,
     read_magician,
+    NULL,
     ATF_UNIQUE
 };
 
@@ -2426,6 +2428,7 @@ attrib_type at_familiar = {
     age_unit,
     a_write_unit,
     read_familiar,
+    NULL,
     ATF_UNIQUE
 };
 
@@ -2436,6 +2439,7 @@ attrib_type at_clonemage = {
     age_unit,
     a_write_unit,
     read_magician,
+    NULL,
     ATF_UNIQUE
 };
 
@@ -2446,6 +2450,7 @@ attrib_type at_clone = {
     age_unit,
     a_write_unit,
     read_clone,
+    NULL,
     ATF_UNIQUE
 };
 

src/market.c

@@ -63,7 +63,7 @@ static void free_market(attrib * a)
 attrib_type at_market = {
     "script",
     NULL, free_market, NULL,
-    NULL, NULL, ATF_UNIQUE
+    NULL, NULL, NULL, ATF_UNIQUE
 };
 
 static int rc_luxury_trade(const struct race *rc)

src/spells/alp.c

@@ -96,6 +96,7 @@ static attrib_type at_alp = {
     alp_verify,
     alp_write,
     alp_read,
+    NULL,
     ATF_UNIQUE
 };
 

src/spells/borders.c

@@ -112,6 +112,7 @@ static attrib_type at_cursewall = {
     curse_age,
     cw_write,
     cw_read,
+    NULL,
     ATF_CURSE
 };
 

src/study.c

@@ -157,7 +157,7 @@ static void done_learning(struct attrib *a)
 
 const attrib_type at_learning = {
     "learning",
-    init_learning, done_learning, NULL, NULL, NULL,
+    init_learning, done_learning, NULL, NULL, NULL, NULL,
     ATF_UNIQUE
 };
 

src/test_eressea.c

@@ -118,6 +118,7 @@ int RunAllTests(int argc, char *argv[])
     ADD_SUITE(monsters);
     ADD_SUITE(move);
     ADD_SUITE(piracy);
+    ADD_SUITE(key);
     ADD_SUITE(stealth);
     ADD_SUITE(otherfaction);
     ADD_SUITE(upkeep);

src/util/attrib.c

@@ -292,7 +292,7 @@ void at_deprecate(const char * name, int(*reader)(attrib *, void *, struct stora
 }
 
 static int a_read_i(struct storage *store, attrib ** attribs, void *owner, unsigned int key) {
-    int retval = AT_READ_FAIL;
+    int retval = AT_READ_OK;
     int(*reader)(attrib *, void *, struct storage *) = 0;
     attrib_type *at = at_find(key);
     attrib * na = 0;
@@ -313,11 +313,13 @@ static int a_read_i(struct storage *store, attrib ** attribs, void *owner, unsig
         }
     }
     if (reader) {
-        retval = reader(na, owner, store);
+        int ret = reader(na, owner, store);
         if (na) {
-            switch (retval) {
+            switch (ret) {
+            case AT_READ_DEPR:
             case AT_READ_OK:
                 a_add(attribs, na);
+                retval = ret;
                 break;
             case AT_READ_FAIL:
                 a_free(na);
@@ -341,11 +343,24 @@ int a_read(struct storage *store, attrib ** attribs, void *owner) {
     zText[0] = 0;
     key = -1;
     READ_INT(store, &key);
-    while (key > 0 && retval == AT_READ_OK) {
+    while (key > 0) {
-        retval = a_read_i(store, attribs, owner, key);
+        int ret = a_read_i(store, attribs, owner, key);
+        if (ret == AT_READ_DEPR) {
+            retval = AT_READ_DEPR;
+        }
         READ_INT(store, &key);
     }
-    return retval;
+    if (retval == AT_READ_DEPR) {
+        /* handle deprecated attributes */
+        attrib *a = *attribs;
+        while (a) {
+            if (a->type->upgrade) {
+                a->type->upgrade(attribs, a);
+            }
+            a = a->nexttype;
+        }
+    }
+    return AT_READ_OK;
 }
 
 int a_read_orig(struct storage *store, attrib ** attribs, void *owner)

src/util/attrib.h

@@ -56,6 +56,7 @@ extern "C" {
         /* age returns 0 if the attribute needs to be removed, !=0 otherwise */
         void(*write) (const struct attrib *, const void *owner, struct storage *);
         int(*read) (struct attrib *, void *owner, struct storage *);       /* return AT_READ_OK on success, AT_READ_FAIL if attrib needs removal */
+        void(*upgrade) (struct attrib **alist, struct attrib *a);
         unsigned int flags;
         /* ---- internal data, do not modify: ---- */
         struct attrib_type *nexthash;
@@ -90,6 +91,7 @@ extern "C" {
 
 #define AT_READ_OK 0
 #define AT_READ_FAIL -1
+#define AT_READ_DEPR 1 /* a deprecated attribute was read, should run a_upgrade */
 
 #define AT_AGE_REMOVE 0         /* remove the attribute after calling age() */
 #define AT_AGE_KEEP 1           /* keep the attribute for another turn */

src/wormhole.c

@@ -134,6 +134,7 @@ static attrib_type at_wormhole = {
     wormhole_age,
     wormhole_write,
     wormhole_read,
+    NULL,
     ATF_UNIQUE
 };