--- deliantra/server/common/object.C	2008/04/24 12:06:01	1.221
+++ deliantra/server/common/object.C	2008/05/04 11:12:40	1.231
@@ -141,33 +141,18 @@
 static bool
 compare_ob_value_lists_one (const object *wants, const object *has)
 {
-  key_value *wants_field;
-
-  /* n-squared behaviour (see get_ob_key_link()), but I'm hoping both
+  /* n-squared behaviour (see kv_get), but I'm hoping both
    * objects with lists are rare, and lists stay short. If not, use a
    * different structure or at least keep the lists sorted... 
    */
 
   /* For each field in wants, */
-  for (wants_field = wants->key_values; wants_field; wants_field = wants_field->next)
-    {
-      key_value *has_field;
-
-      /* Look for a field in has with the same key. */
-      has_field = get_ob_key_link (has, wants_field->key);
-
-      if (!has_field)
-        return 0; /* No field with that name. */
-
-      /* Found the matching field. */
-      if (has_field->value != wants_field->value)
-        return 0; /* Values don't match, so this half of the comparison is false. */
-
-      /* If we get here, we found a match. Now for the next field in wants. */
-    }
+  for (key_value *kv = wants->key_values; kv; kv = kv->next)
+    if (has->kv_get (kv->key) != kv->value)
+      return false;
 
   /* If we get here, every field in wants has a matching field in has. */
-  return 1;
+  return true;
 }
 
 /* Returns TRUE if ob1 has the same key_values as ob2. */
@@ -177,7 +162,8 @@
   /* However, there may be fields in has which aren't partnered in wants,
    * so we need to run the comparison *twice*. :(
    */
-  return compare_ob_value_lists_one (ob1, ob2) && compare_ob_value_lists_one (ob2, ob1);
+  return compare_ob_value_lists_one (ob1, ob2)
+      && compare_ob_value_lists_one (ob2, ob1);
 }
 
 /* Function examines the 2 objects given to it, and returns true if
@@ -605,35 +591,39 @@
   op->key_values = 0;
 }
 
-object &
-object::operator =(const object &src)
+/*
+ * copy_to first frees everything allocated by the dst object,
+ * and then copies the contents of itself into the second
+ * object, allocating what needs to be allocated.  Basically, any
+ * data that is malloc'd needs to be re-malloc/copied.  Otherwise,
+ * if the first object is freed, the pointers in the new object
+ * will point at garbage.
+ */
+void
+object::copy_to (object *dst)
 {
-  bool is_freed   = flag [FLAG_FREED];
-  bool is_removed = flag [FLAG_REMOVED];
-
-  *(object_copy *)this = src;
-
-  flag [FLAG_FREED]   = is_freed;
-  flag [FLAG_REMOVED] = is_removed;
+  dst->remove ();
+  *(object_copy *)dst = *this;
+  dst->flag [FLAG_REMOVED] = true;
 
   /* Copy over key_values, if any. */
-  if (src.key_values)
+  if (key_values)
     {
       key_value *tail = 0;
-      key_values = 0;
+      dst->key_values = 0;
 
-      for (key_value *i = src.key_values; i; i = i->next)
+      for (key_value *i = key_values; i; i = i->next)
         {
           key_value *new_link = new key_value;
 
-          new_link->next = 0;
-          new_link->key = i->key;
+          new_link->next  = 0;
+          new_link->key   = i->key;
           new_link->value = i->value;
 
           /* Try and be clever here, too. */
-          if (!key_values)
+          if (!dst->key_values)
             {
-              key_values = new_link;
+              dst->key_values = new_link;
               tail = new_link;
             }
           else
@@ -643,20 +633,6 @@
             }
         }
     }
-}
-
-/*
- * copy_to first frees everything allocated by the dst object,
- * and then copies the contents of itself into the second
- * object, allocating what needs to be allocated.  Basically, any
- * data that is malloc'd needs to be re-malloc/copied.  Otherwise,
- * if the first object is freed, the pointers in the new object
- * will point at garbage.
- */
-void
-object::copy_to (object *dst)
-{
-  *dst = *this;
 
   if (speed < 0)
     dst->speed_left -= rndm ();
@@ -688,6 +664,7 @@
 {
   object *neu = create ();
   copy_to (neu);
+  neu->map = map; // not copied by copy_to
   return neu;
 }
 
@@ -748,14 +725,14 @@
 void
 update_object (object *op, int action)
 {
-  if (op == NULL)
+  if (!op)
     {
       /* this should never happen */
-      LOG (llevDebug, "update_object() called for NULL object.\n");
+      LOG (llevError | logBacktrace, "update_object() called for NULL object.\n");
       return;
     }
 
-  if (op->env)
+  if (!op->is_on_map ())
     {
       /* Animation is currently handled by client, so nothing
        * to do in this case.
@@ -763,12 +740,6 @@
       return;
     }
 
-  /* If the map is saving, don't do anything as everything is
-   * going to get freed anyways.
-   */
-  if (!op->map || op->map->in_memory == MAP_SAVING)
-    return;
-
   /* make sure the object is within map boundaries */
   if (op->x < 0 || op->x >= op->map->width || op->y < 0 || op->y >= op->map->height)
     {
@@ -967,6 +938,26 @@
   return op;
 }
 
+static struct freed_map : maptile
+{
+  freed_map ()
+  {
+    path   = "<freed objects map>";
+    name   = "/internal/freed_objects_map";
+    width  = 3;
+    height = 3;
+    nodrop = 1;
+
+    alloc ();
+    in_memory = MAP_ACTIVE;
+  }
+
+  ~freed_map ()
+  {
+    destroy ();
+  }
+} freed_map; // freed objects are moved here to avoid crashes
+
 void
 object::do_destroy ()
 {
@@ -986,27 +977,9 @@
   flag [FLAG_FREED] = 1;
 
   // hack to ensure that freed objects still have a valid map
-  {
-    static maptile *freed_map; // freed objects are moved here to avoid crashes
-
-    if (!freed_map)
-      {
-        freed_map = new maptile;
-
-        freed_map->path   = "<freed objects map>";
-        freed_map->name   = "/internal/freed_objects_map";
-        freed_map->width  = 3;
-        freed_map->height = 3;
-        freed_map->nodrop = 1;
-
-        freed_map->alloc ();
-        freed_map->in_memory = MAP_ACTIVE;
-      }
-
-    map = freed_map;
-    x = 1;
-    y = 1;
-  }
+  map = &freed_map;
+  x = 1;
+  y = 1;
 
   if (more)
     {
@@ -1033,6 +1006,7 @@
     {
       LOG (llevError | logBacktrace, "tried to destroy the tail of an object");
       head->destroy (destroy_inventory);
+      return;
     }
 
   destroy_inv (!destroy_inventory);
@@ -1171,7 +1145,10 @@
             pl->close_container ();
 
           //TODO: the floorbox prev/next might need updating
-          esrv_del_item (pl->contr, count);
+          //esrv_del_item (pl->contr, count);
+          //TODO: update floorbox to preserve ordering
+          if (pl->contr->ns)
+            pl->contr->ns->floorbox_update ();
         }
 
       for (tmp = ms.bot; tmp; tmp = tmp->above)
@@ -1479,7 +1456,10 @@
 
   if (object *pl = ms.player ())
     //TODO: the floorbox prev/next might need updating
-    esrv_send_item (pl, op);
+    //esrv_send_item (pl, op);
+    //TODO: update floorbox to preserve ordering
+    if (pl->contr->ns)
+      pl->contr->ns->floorbox_update ();
 
   /* If this object glows, it may affect lighting conditions that are
    * visible to others on this map.  But update_all_los is really
@@ -1609,7 +1589,7 @@
     {
       decrease (nr);
 
-      object *op = object_create_clone (this);
+      object *op = deep_clone ();
       op->nrof = nr;
       return op;
     }
@@ -1840,7 +1820,7 @@
     }
 
   for (object *tmp = m->at (x, y).bot; tmp; tmp = tmp->above)
-    if (tmp->arch == at)
+    if (tmp->arch->archname == at->archname)
       return tmp;
 
   return NULL;
@@ -1914,8 +1894,8 @@
 object *
 present_arch_in_ob (const archetype *at, const object *op)
 {
-  for (object *tmp = op->inv; tmp != NULL; tmp = tmp->below)
-    if (tmp->arch == at)
+  for (object *tmp = op->inv; tmp; tmp = tmp->below)
+    if (tmp->arch->archname == at->archname)
       return tmp;
 
   return NULL;
@@ -2334,40 +2314,23 @@
  * create clone from object to another
  */
 object *
-object_create_clone (object *asrc)
+object::deep_clone ()
 {
-  object *dst = 0, *tmp, *src, *prev, *item;
+  assert (("deep_clone called on non-head object", is_head ()));
 
-  if (!asrc)
-    return 0;
-
-  src = asrc->head_ ();
+  object *dst = clone ();
 
-  prev = 0;
-  for (object *part = src; part; part = part->more)
+  object *prev = dst;
+  for (object *part = this->more; part; part = part->more)
     {
-      tmp = part->clone ();
-      tmp->x -= src->x;
-      tmp->y -= src->y;
-
-      if (!part->head)
-        {
-          dst = tmp;
-          tmp->head = 0;
-        }
-      else
-        tmp->head = dst;
-
-      tmp->more = 0;
-
-      if (prev)
-        prev->more = tmp;
-
+      object *tmp = part->clone ();
+      tmp->head = dst;
+      prev->more = tmp;
       prev = tmp;
     }
 
-  for (item = src->inv; item; item = item->below)
-    insert_ob_in_ob (object_create_clone (item), dst);
+  for (object *item = inv; item; item = item->below)
+    insert_ob_in_ob (item->deep_clone (), dst);
 
   return dst;
 }
@@ -2386,143 +2349,46 @@
   return 0;
 }
 
-/* If ob has a field named key, return the link from the list,
- * otherwise return NULL. 
- *
- * key must be a passed in shared string - otherwise, this won't
- * do the desired thing.
- */
-key_value *
-get_ob_key_link (const object *ob, const char *key)
-{
-  for (key_value *link = ob->key_values; link; link = link->next)
-    if (link->key == key)
-      return link;
-
-  return 0;
-}
-
-/* 
- * Returns the value of op has an extra_field for key, or NULL.
- *
- * The argument doesn't need to be a shared string.
- *
- * The returned string is shared.
- */
-const char *
-get_ob_key_value (const object *op, const char *const key)
+const shstr &
+object::kv_get (const shstr &key) const
 {
-  key_value *link;
-  shstr_cmp canonical_key (key);
+  for (key_value *kv = key_values; kv; kv = kv->next)
+    if (kv->key == key)
+      return kv->value;
 
-  if (!canonical_key)
-    {
-      /* 1. There being a field named key on any object
-       *    implies there'd be a shared string to find.
-       * 2. Since there isn't, no object has this field.
-       * 3. Therefore, *this* object doesn't have this field.
-       */
-      return 0;
-    }
-
-  /* This is copied from get_ob_key_link() above -
-   * only 4 lines, and saves the function call overhead.
-   */
-  for (link = op->key_values; link; link = link->next)
-    if (link->key == canonical_key)
-      return link->value;
-
-  return 0;
+  return shstr_null;
 }
 
-/*
- * Updates the canonical_key in op to value.
- *
- * canonical_key is a shared string (value doesn't have to be).
- *
- * Unless add_key is TRUE, it won't add fields, only change the value of existing
- * keys.
- *
- * Returns TRUE on success.
- */
-int
-set_ob_key_value_s (object *op, const shstr & canonical_key, const char *value, int add_key)
+void
+object::kv_set (const shstr &key, const shstr &value)
 {
-  key_value *field = NULL, *last = NULL;
-
-  for (field = op->key_values; field != NULL; field = field->next)
-    {
-      if (field->key != canonical_key)
-        {
-          last = field;
-          continue;
-        }
-
-      if (value)
-        field->value = value;
-      else
-        {
-          /* Basically, if the archetype has this key set,
-           * we need to store the null value so when we save
-           * it, we save the empty value so that when we load,
-           * we get this value back again.
-           */
-          if (get_ob_key_link (op->arch, canonical_key))
-            field->value = 0;
-          else
-            {
-              if (last)
-                last->next = field->next;
-              else
-                op->key_values = field->next;
-
-              delete field;
-            }
-        }
-      return TRUE;
-    }
-  /* IF we get here, key doesn't exist */
-
-  /* No field, we'll have to add it. */
-
-  if (!add_key)
-    return FALSE;
-
-  /* There isn't any good reason to store a null
-   * value in the key/value list.  If the archetype has
-   * this key, then we should also have it, so shouldn't
-   * be here.  If user wants to store empty strings,
-   * should pass in ""
-   */
-  if (value == NULL)
-    return TRUE;
+  for (key_value *kv = key_values; kv; kv = kv->next)
+    if (kv->key == key)
+      {
+        kv->value = value;
+        return;
+      }
 
-  field = new key_value;
+  key_value *kv = new key_value;
 
-  field->key = canonical_key;
-  field->value = value;
-  /* Usual prepend-addition. */
-  field->next = op->key_values;
-  op->key_values = field;
+  kv->next  = key_values;
+  kv->key   = key;
+  kv->value = value;
 
-  return TRUE;
+  key_values = kv;
 }
 
-/*
- * Updates the key in op to value.
- *
- * If add_key is FALSE, this will only update existing keys,
- * and not add new ones.
- * In general, should be little reason FALSE is ever passed in for add_key
- *
- * Returns TRUE on success.
- */
-int
-set_ob_key_value (object *op, const char *key, const char *value, int add_key)
+void
+object::kv_del (const shstr &key)
 {
-  shstr key_ (key);
-
-  return set_ob_key_value_s (op, key_, value, add_key);
+  for (key_value **kvp = &key_values; *kvp; kvp = &(*kvp)->next)
+    if ((*kvp)->key == key)
+      {
+        key_value *kv = *kvp;
+        *kvp = (*kvp)->next;
+        delete kv;
+        return;
+      }
 }
 
 object::depth_iterator::depth_iterator (object *container)