On Mon, Apr 09, 2001 at 10:11:54PM -0700, Mark Wedel wrote: > Peter Mardahl wrote: > > I take it a better implementation would be to create the whole > > thing and then insert it into the map? I think I tried that at one > > point with rather annoying results: I suppose the attempt where you got one head, two pieces of the second part, three the third and so on was almost right - the only problem was that you called insert_ob_in_map() in a loop for all parts inserting all parts except the head multiple times. One insert_ob_in_map() for the head would have been enough. The problem with only the head being on the map was that you didn't link the parts together before calling insert_ob_in_map(), i.e. you called insert_ob_in_map() too early. > > Hmm, the random map code always places a floor and therefore assumes > > that there is a floor. Given that, would you still say these changes > > are necessary? It isn't necessary, but I prefer code that makes fewer assumptions (i.e. that doesn't assume that there is always a floor object) because it doesn't break as easily. > nuke_map_region looks ok. after the call to get_map_ob in both areas, we do a > check to see if tmp is NULL, and do the proper thing. Though the loop is a bit complicated. > remove_monsters is missing a tmp==NULL check after the tmp=get_map_ob... call. > That should probably be added - you never know when someone may try to re-use > your code. Both functions could be coded as simply as for (tmp = get_map_ob(...); tmp != NULL; ) if (want_to_remove_this (tmp)) { ... tmp = get_map_ob(...); } else { tmp = tmp->above; } but I don't see any reason to make that change before 1.0, and even after that I'd give it a very low priority. -- Jan