refactoring: replace the dead_units list

res/translations/messages.de.po

@@ -1735,7 +1735,7 @@ msgid "santa_m"
 msgstr "'Ho ho ho!' Ein dicker Gnom fliegt auf einem von 8 Jungdrachen gezogenen Schlitten durch die Nacht und vermacht Deiner Partei einen $resource($item,1). (Um diesen Gegenstand einer Einheit zu geben, gib ihr den Befehl 'BEANSPRUCHE 1 $resource($item,1)')."
 
 msgid "error234"
-msgstr "\"$unit($unit) in $region($region): '$order($command)' - Die Einheit ist mit Ausschiffen beschäftigt..\""
+msgstr "\"$unit($unit) in $region($region): '$order($command)' - Die Einheit ist mit Ausschiffen beschäftigt.\""
 
 msgid "spellfail::nolevel"
 msgstr "\"$unit($mage) in $region($region): '$order($command)' - Dieser Zauber kann nicht mit Stufenangabe gezaubert werden.\""

scripts/tests/e2/e2features.lua

@@ -420,8 +420,9 @@ function test_stonegolems()
 -- test that no server crash occurs
   u1:clear_orders()
   u1:add_order("Mache Burg")
+  local uid = u1.id
   process_orders()
-  assert_equal(0, u1.number, "There should be no more stone golems")
+  assert_nil(get_unit(uid), "There should be no more stone golems")
 -- end test server crash
 
 -- test that Stone Golems build for four stones

scripts/tests/e2/recruit.lua

@@ -160,9 +160,9 @@ function test_recruit_empty()
     u11:add_order("GIB 0 ALLES PERSONEN")
     u11:add_order("REKRUTIERE 1")
 
+    local uid = u11.id
     process_orders()
-
-    assert_equal(0, u11.number)
+    assert_nil(get_unit(uid));
 
     local count = 0
     local zero = 0

scripts/tests/e2/ships.lua

@@ -227,21 +227,22 @@ function test_give_ship_all_on_land()
     u1.ship.number = 2
     u1:add_order("GIB 0 2 SCHIFF")
     process_orders()
-    assert_equal(nil, u1.ship)
-    assert_not_equal(nil, r.ships())
+    assert_nil(u1.ship)
+    assert_not_nil(r.ships())
 end
 
 function test_give_ship_no_boat()
-    local r = region.create(1, 0, 'ocean')
+    local r = region.create(1, 0, 'plain')
     local f = faction.create("human")
     local u1 = unit.create(f, r, 1)
     local u2 = unit.create(f, r, 1)
+    u2.name = 'Xolgrim'
     u1.ship = ship.create(r, 'boat')
     u1.ship.number = 1
     u1:add_order("GIB " .. itoa36(u2.id) .. " 1 SCHIFF")
     process_orders()
     assert_equal(1, u1.ship.number)
-    assert_equal(nil, u2.ship)
+    assert_nil(u2.ship)
 end
 
 function test_give_ship_away()
@@ -418,7 +419,7 @@ function test_give_ship_self_only()
     u1:add_order("GIB " .. itoa36(u2.id) .. " 1 SCHIFF")
     process_orders()
     assert_equal(2, u1.ship.number)
-    assert_equal(nil, u2.ship)
+    assert_nil(u2.ship)
 end
 
 function test_give_ship_not_cursed()
@@ -436,7 +437,7 @@ function test_give_ship_not_cursed()
 
     u:add_order("GIB " .. itoa36(u2.id) .. " 1 SCHIFF")
     process_orders()
-    assert_equal(nil, u2.ship)
+    assert_nil(u2.ship)
 end
 
 function test_speedsail_on_ship()
@@ -461,7 +462,7 @@ function test_no_speedsail_on_convoy()
     u:add_item("speedsail", 2)
     u:add_order("BENUTZE 2 Sonnensegel")
     process_orders()
-    assert_equal(nil, sh:get_curse('shipspeedup'))
+    assert_nil(sh:get_curse('shipspeedup'))
 end
 
 function test_build_new_ship()

scripts/tests/e2/spells.lua

@@ -231,8 +231,9 @@ function test_bug_2480()
   u1.number = 30
   u1.hp = u1.hp_max * u1.number
   monster:add_order("ATTACK " .. itoa36(u1.id))
+  local uid = u1.id
   process_orders()
-  assert_equal(0, u1.number);
+  assert_nil(get_unit(uid));
 end
 
 function test_bug_2517()

scripts/tests/laws.lua

@@ -10,13 +10,14 @@ function setup()
     eressea.free_game()
     conf = [[{
         "races": {
-            "human" : { "flags" : [ "player", "giveperson" ] }
+            "human" : { "flags" : [ "player", "giveperson", "getitem" ] }
         },
         "terrains" : {
             "plain": { "flags" : [ "land", "walk", "sail" ] }
         },
         "parameters" : {
             "de" : {
+                "ALLES": "ALLES",
                 "EINHEIT": "EINHEIT",
                 "PERSONEN": "PERSONEN"
             }
@@ -25,6 +26,7 @@ function setup()
             "de": {
                 "attack" : "ATTACKIERE",
                 "guard" : "BEWACHE",
+                "steal" : "BEKLAUE",
                 "maketemp" : "MACHETEMP",
                 "end" : "ENDE",
                 "use" : "BENUTZEN",
@@ -168,15 +170,60 @@ function test_give_and_forget()
     local r = region.create(0, 0, "plain")
     local f = faction.create("human")
     local u1 = unit.create(f, r, 1)
+    local uno = u1.id
     local u2 = unit.create(f, r, 1)
-    u1.name = 'Xolgrim'
     u1:set_skill('alchemy', 1)
     u1:set_skill('crossbow', 1)
     u2:set_skill('alchemy', 1)
     u1:set_orders("GIB " .. itoa36(u2.id) .. " 1 PERSON\nVERGISS Armbrust")
     process_orders()
-    assert_equal(0, u1.number)
+    assert_nil(get_unit(uno))
     assert_equal(2, u2.number)
     assert_equal(0, u2:get_skill('crossbow'))
     assert_equal(1, u2:get_skill('alchemy'))
 end
+
+-- u1 is poor, so we steal from u2
+function test_steal_from_pool()
+    local r = region.create(0, 0, "plain")
+    local f = faction.create("human")
+    local u1 = unit.create(f, r)
+    local u2 = unit.create(f, r)
+    u2:add_item('money', 100)
+    local u = unit.create(faction.create("human"), r)
+    u:set_skill('stealth', 1)
+    u:set_orders("BEKLAUE " .. itoa36(u1.id))
+    u.name = 'Xolgrim'
+    process_orders()
+    assert_equal(50, u:get_item('money'))
+    assert_equal(50, u2:get_item('money'))
+end
+
+function test_thief_must_see_target()
+    local r = region.create(0, 0, "plain")
+    local f = faction.create("human")
+    local u1 = unit.create(f, r)
+    u1:set_skill('stealth', 1)
+    u1:add_item('money', 100)
+    local u = unit.create(faction.create("human"), r)
+    u:set_skill('stealth', 1)
+    u:set_orders("BEKLAUE " .. itoa36(u1.id))
+    process_orders()
+    assert_equal(0, u:get_item('money'))
+    assert_equal(100, u1:get_item('money'))
+end
+
+function test_temp_cannot_avoid_theft()
+    local r = region.create(0, 0, "plain")
+    local f = faction.create("human")
+    local u1 = unit.create(f, r)
+    u1:add_item('money', 100)
+    u1:set_orders("MACHETEMP 1\nENDE\nGIB TEMP 1 ALLES PERSON\nGIB TEMP 1 ALLES")
+    local u = unit.create(faction.create("human"), r)
+    u:set_skill('stealth', 1)
+    u:set_orders("BEKLAUE " .. itoa36(u1.id))
+    process_orders()
+    assert_equal(50, u:get_item('money'))
+    local u2 = f.units()
+    assert_equal(50, u2:get_item('money'))
+end

src/kernel/unit.c

@@ -318,17 +318,15 @@ int gift_items(unit * u, int flags)
  * returns 0 on success, or -1 if unit could not be removed.
  */
 
-static unit *deleted_units = NULL;
+typedef struct dead_faction {
+    int key;
+    faction* value;
+} dead_faction;
 
-struct faction *dfindhash(int no) {
-    unit *u;
+static dead_faction* dead_hash;
 
-    for (u = deleted_units; u; u = u->next) {
-        if (u->no == no) {
-            return u->faction;
-        }
-    }
-    return NULL;
+struct faction *dfindhash(int no) {
+    return hmget(dead_hash, no);
 }
 
 void erase_unit(unit** ulist, unit* u)
@@ -367,10 +365,11 @@ void erase_unit(unit** ulist, unit* u)
     u->nextF = 0;
     u->prevF = 0;
 
-    u->next = deleted_units;
-    deleted_units = u;
-
-    u->region = NULL;
+    if (u->faction) {
+        hmput(dead_hash, u->no, u->faction);
+    }
+    free_unit(u);
+    free(u);
 }
 
 int remove_unit(unit ** ulist, unit * u)
@@ -510,12 +509,7 @@ void usetprivate(unit * u, const char *str)
 
 void free_units(void)
 {
-    while (deleted_units) {
-        unit *u = deleted_units;
-        deleted_units = deleted_units->next;
-        free_unit(u);
-        free(u);
-    }
+    hmfree(dead_hash);
 }
 
 void write_unit_reference(const unit * u, struct storage *store)

src/kernel/unit.test.c

@@ -49,52 +49,67 @@ static void test_remove_empty_units(CuTest *tc) {
 }
 
 static void test_remove_empty_units_in_region(CuTest *tc) {
-    unit *u;
+    unit *u1, *u2;
+    faction* f;
+    region* r;
     int uid;
 
     test_setup();
 
-    u = test_create_unit(test_create_faction(), test_create_plain(0, 0));
-    u = test_create_unit(u->faction, u->region);
-    CuAssertPtrNotNull(tc, u->nextF);
-    uid = u->no;
-    remove_empty_units_in_region(u->region);
-    CuAssertPtrNotNull(tc, findunit(uid));
-    u->number = 0;
-    remove_empty_units_in_region(u->region);
+    u2 = test_create_unit(f = test_create_faction(), r = test_create_plain(0, 0));
+    u1 = test_create_unit(f, r);
+    uid = u1->no;
+    CuAssertPtrEquals(tc, u1, f->units);
+    CuAssertPtrEquals(tc, u2, u1->nextF);
+    remove_empty_units_in_region(r);
+    CuAssertPtrEquals(tc, u1, findunit(uid));
+    u1->number = 0;
+    remove_empty_units_in_region(r);
     CuAssertPtrEquals(tc, NULL, findunit(uid));
-    CuAssertPtrEquals(tc, NULL, u->nextF);
-    CuAssertPtrEquals(tc, NULL, u->region);
+    CuAssertPtrEquals(tc, u2, f->units);
+    CuAssertPtrEquals(tc, NULL, u2->nextF);
+    CuAssertPtrEquals(tc, u2, r->units);
+    CuAssertPtrEquals(tc, NULL, u2->next);
     test_teardown();
 }
 
 static void test_remove_units_without_faction(CuTest *tc) {
     unit *u;
+    faction* f;
+    region* r;
     int uid;
 
     test_setup();
 
-    u = test_create_unit(test_create_faction(), test_create_plain(0, 0));
+    u = test_create_unit(f = test_create_faction(), r = test_create_plain(0, 0));
     uid = u->no;
-    u_setfaction(u, 0);
-    remove_empty_units_in_region(u->region);
+    u_setfaction(u, NULL);
+    remove_empty_units_in_region(r);
+    /* unit is deleted: */
+    CuAssertPtrEquals(tc, NULL, r->units);
+    CuAssertPtrEquals(tc, NULL, f->units);
     CuAssertPtrEquals(tc, NULL, findunit(uid));
-    CuAssertIntEquals(tc, 0, u->number);
+    CuAssertPtrEquals(tc, NULL, dfindhash(uid));
     test_teardown();
 }
 
 static void test_remove_units_with_dead_faction(CuTest *tc) {
     unit *u;
+    faction* f;
+    region* r;
     int uid;
 
     test_setup();
 
-    u = test_create_unit(test_create_faction(), test_create_plain(0, 0));
+    u = test_create_unit(f = test_create_faction(), r = test_create_plain(0, 0));
     uid = u->no;
-    u->faction->_alive = false;
-    remove_empty_units_in_region(u->region);
+    f->_alive = false;
+    remove_empty_units_in_region(r);
+    /* unit is deleted: */
+    CuAssertPtrEquals(tc, NULL, r->units);
+    CuAssertPtrEquals(tc, NULL, f->units);
     CuAssertPtrEquals(tc, NULL, findunit(uid));
-    CuAssertIntEquals(tc, 0, u->number);
+    CuAssertPtrEquals(tc, f, dfindhash(uid));
     test_teardown();
 }
 
@@ -520,25 +535,21 @@ static void test_remove_unit(CuTest *tc) {
     region_setresource(r, rtype, 0);
     i_change(&u1->items, rtype->itype, 100);
     remove_unit(&r->units, u1);
-    CuAssertIntEquals(tc, 0, u1->number);
-    CuAssertPtrEquals(tc, NULL, u1->region);
+
+    /* unit is deleted: */
+    CuAssertPtrEquals(tc, NULL, findunit(uno));
+    CuAssertPtrEquals(tc, f, dfindhash(uno));
+
     /* money is given to a survivor: */
-    CuAssertPtrEquals(tc, NULL, u1->items);
     CuAssertIntEquals(tc, 0, region_getresource(r, rtype));
     CuAssertIntEquals(tc, 100, i_get(u2->items, rtype->itype));
 
     /* unit is removed from f->units: */
-    CuAssertPtrEquals(tc, NULL, u1->nextF);
     CuAssertPtrEquals(tc, u2, f->units);
     CuAssertPtrEquals(tc, NULL, u2->nextF);
     CuAssertPtrEquals(tc, NULL, u2->prevF);
     /* unit is no longer in r->units: */
     CuAssertPtrEquals(tc, u2, r->units);
-    CuAssertPtrEquals(tc, NULL, u2->next);
-
-    /* unit is in deleted_units: */
-    CuAssertPtrEquals(tc, NULL, findunit(uno));
-    CuAssertPtrEquals(tc, f, dfindhash(uno));
 
     remove_unit(&r->units, u2);
     /* no survivor, give money to peasants: */
@@ -724,6 +735,7 @@ static void test_transfermen_peasants(CuTest *tc) {
     /* demons don't transfer to peasants */
     rc = test_create_race("undead");
     rc->ec_flags |= ECF_REC_ETHEREAL;
+    rc->ec_flags |= ECF_REC_ETHEREAL;
     u_setrace(u, rc);
     transfermen(u, NULL, 1);
     CuAssertIntEquals(tc, 7, u->number);

src/steal.c

@@ -107,7 +107,7 @@ message * steal_message(const unit * u, struct order *ord) {
     if (pl && fval(pl, PFL_NOATTACK)) {
         return msg_feedback(u, ord, "error270", "");
     }
-    return 0;
+    return NULL;
 }
 
 void steal_cmd(unit * u, struct order *ord, econ_request ** stealorders)