--- deliantra/server/common/object.C 2008/04/23 07:13:23 1.217 +++ deliantra/server/common/object.C 2008/05/06 20:00:12 1.235 @@ -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 @@ -336,7 +322,7 @@ object * object::visible_to () const { - if (!flag [FLAG_REMOVED]) + if (client_visible () && !flag [FLAG_REMOVED]) { // see if we are in a container of sorts if (env) @@ -360,7 +346,8 @@ // maybe there is a player standing on the same mapspace // this will catch the case where "this" is a player if (object *pl = ms ().player ()) - return pl; + if (!pl->container || this == pl->container) + return pl; } } @@ -369,7 +356,7 @@ // adjust weight per container type ("of holding") static sint32 -weight_adjust (object *op, sint32 weight) +weight_adjust_for (object *op, sint32 weight) { return op->type == CONTAINER ? lerp (weight, 0, 100, 0, 100 - op->stats.Str) @@ -385,7 +372,10 @@ { while (op) { - weight = weight_adjust (op, weight); + // adjust by actual difference to account for rounding errors + // i.e. (w2 - w1) / f != w2 / f - w1 / f and the latter is correct + weight = weight_adjust_for (op, op->carrying) + - weight_adjust_for (op, op->carrying - weight); if (!weight) return; @@ -418,7 +408,7 @@ sum += op->total_weight (); } - sum = weight_adjust (this, sum); + sum = weight_adjust_for (this, sum); if (sum != carrying) { @@ -604,35 +594,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 @@ -642,20 +636,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 (); @@ -687,6 +667,7 @@ { object *neu = create (); copy_to (neu); + neu->map = map; // not copied by copy_to return neu; } @@ -747,14 +728,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. @@ -762,12 +743,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) { @@ -966,6 +941,26 @@ return op; } +static struct freed_map : maptile +{ + freed_map () + { + path = ""; + 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 () { @@ -985,27 +980,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_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) { @@ -1028,6 +1005,13 @@ if (destroyed ()) return; + if (!is_head () && !head->destroyed ()) + { + LOG (llevError | logBacktrace, "tried to destroy the tail of an object"); + head->destroy (destroy_inventory); + return; + } + destroy_inv (!destroy_inventory); if (is_head ()) @@ -1057,9 +1041,6 @@ INVOKE_OBJECT (REMOVE, this); - if (object *pl = visible_to ()) - esrv_del_item (pl->contr, count); - flag [FLAG_REMOVED] = true; if (more) @@ -1071,6 +1052,11 @@ */ if (env) { + flag [FLAG_REMOVED] = false; // hack around the issue of visible_to checking flag_removed + if (object *pl = visible_to ()) + esrv_del_item (pl->contr, count); + flag [FLAG_REMOVED] = true; // hack around the issue of visible_to checking flag_removed + adjust_weight (env, -total_weight ()); *(above ? &above->below : &env->inv) = below; @@ -1098,20 +1084,30 @@ } else if (map) { - if (type == PLAYER) + map->dirty = true; + mapspace &ms = this->ms (); + + if (object *pl = ms.player ()) { - // leaving a spot always closes any open container on the ground - if (container && !container->env) - // this causes spurious floorbox updates, but it ensures - // that the CLOSE event is being sent. - close_container (); + if (type == PLAYER) // this == pl(!) + { + // leaving a spot always closes any open container on the ground + if (container && !container->env) + // this causes spurious floorbox updates, but it ensures + // that the CLOSE event is being sent. + close_container (); - --map->players; - map->touch (); - } + --map->players; + map->touch (); + } + else if (pl->container == this) + { + // removing a container should close it + close_container (); + } - map->dirty = true; - mapspace &ms = this->ms (); + esrv_del_item (pl->contr, count); + } /* link the object above us */ if (above) @@ -1123,16 +1119,7 @@ if (below) below->above = above; else - { - /* Nothing below, which means we need to relink map object for this space - * use translated coordinates in case some oddness with map tiling is - * evident - */ - if (GET_MAP_OB (map, x, y) != this) - LOG (llevError, "remove_ob: GET_MAP_OB does not return object to be removed even though it appears to be on the bottom? %s\n", debug_desc ()); - - ms.bot = above; /* goes on above it. */ - } + ms.bot = above; above = 0; below = 0; @@ -1151,7 +1138,11 @@ */ pl->close_container (); - esrv_del_item (pl, count); + //TODO: the floorbox prev/next might need updating + //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) @@ -1287,8 +1278,6 @@ { assert (!op->flag [FLAG_FREED]); - object *top, *floor = NULL; - op->remove (); /* Ideally, the caller figures this out. However, it complicates a lot @@ -1297,7 +1286,7 @@ */ if (!xy_normalise (m, op->x, op->y)) { - op->destroy (1); + op->head_ ()->destroy (1);// remove head_ once all tail object destroyers found return 0; } @@ -1316,6 +1305,9 @@ for (object *tmp = ms.bot; tmp; tmp = tmp->above) if (object::can_merge (op, tmp)) { + // TODO: we atcually want to update tmp, not op, + // but some caller surely breaks when we return tmp + // from here :/ op->nrof += tmp->nrof; tmp->destroy (1); } @@ -1347,6 +1339,8 @@ } else { + object *top, *floor = NULL; + top = ms.bot; /* If there are other objects, then */ @@ -1455,7 +1449,11 @@ op->map->dirty = true; if (object *pl = ms.player ()) - esrv_send_item (pl, op); + //TODO: the floorbox prev/next might need updating + //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 @@ -1585,7 +1583,7 @@ { decrease (nr); - object *op = object_create_clone (this); + object *op = deep_clone (); op->nrof = nr; return op; } @@ -1816,7 +1814,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; @@ -1890,8 +1888,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; @@ -2310,40 +2308,23 @@ * create clone from object to another */ object * -object_create_clone (object *asrc) +object::deep_clone () { - object *dst = 0, *tmp, *src, *prev, *item; - - if (!asrc) - return 0; + assert (("deep_clone called on non-head object", is_head ())); - 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; } @@ -2362,143 +2343,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) +const shstr & +object::kv_get (const shstr &key) const { - for (key_value *link = ob->key_values; link; link = link->next) - if (link->key == key) - return link; + for (key_value *kv = key_values; kv; kv = kv->next) + if (kv->key == key) + return kv->value; - return 0; + return shstr_null; } -/* - * 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) -{ - key_value *link; - shstr_cmp canonical_key (key); - - 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; -} - -/* - * 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) @@ -2606,7 +2490,9 @@ if (container == new_container) return; - if (object *old_container = container) + object *old_container = container; + + if (old_container) { if (INVOKE_OBJECT (CLOSE, old_container, ARG_OBJECT (this))) return; @@ -2618,10 +2504,15 @@ closer->destroy (); #endif + // make sure the container is available + esrv_send_item (this, old_container); + old_container->flag [FLAG_APPLIED] = false; container = 0; + // client needs item update to make it work, client bug requires this to be separate esrv_update_item (UPD_FLAGS, this, old_container); + new_draw_info_format (NDI_UNIQUE, 0, this, "You close %s.", query_name (old_container)); play_sound (sound_find ("chest_close")); } @@ -2644,13 +2535,19 @@ new_draw_info_format (NDI_UNIQUE, 0, this, "You open %s.", query_name (new_container)); + // make sure the container is available, client bug requires this to be separate + esrv_send_item (this, new_container); + new_container->flag [FLAG_APPLIED] = true; container = new_container; + // client needs flag change esrv_update_item (UPD_FLAGS, this, new_container); esrv_send_inventory (this, new_container); play_sound (sound_find ("chest_open")); } +// else if (!old_container->env && contr && contr->ns) +// contr->ns->floorbox_reset (); } object *