[CF-Devel] How can this have happened? (crashing bug)

Jan Echternach echter at informatik.uni-rostock.de
Tue Apr 10 05:37:25 CDT 2001


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

    
    


More information about the crossfire mailing list