[crossfire] MF Crash/Arena petmode

[Alex Schultz]

Mark Wedel wrote:

>
     
      I'd personally suggest cleaning that 'if' statement at all possible - 
     
     >
     
      break it into smaller pieces or something.
     
     >
     
     
     >
     
       Anyways, hard to know exactly the logic of the if statement.  But 
     
     >
     
      looking at the core files, and at the statements, that big problem I 
     
     >
     
      see is that you are looking at owner->contr->... without knowing in 
     
     >
     
      fact that owner->contr is in fact valid.
     
     >
     
     
     >
     
       owner does not have to be a player, and looking at the two crashes, 
     
     >
     
      in fact, owner is not a player, but a Balrog.  I'd also be wary of any 
     
     >
     
      other ->contr checks here, unless you really know 100% sure that they 
     
     >
     
      are valid.  The if statement is too complex for me to see that at a 
     
     >
     
      glance if the checks are there.
     
     >
     
     
     >
     
       The other problem with such complex if statements is that the crash 
     
     >
     
      point is really 'someplace' in that statement - breaking it in smaller 
     
     >
     
      pieces gives you a bit finer control.
     
     >
     
     
     >
     
       The cleanest thing is if you can do some basic checks like:
     
     >
     
     
     >
     
       if (simpler expression) continue;
     
     >
     
       if (other simpler expression) continue;
     
     >
     
       ...
     
     >
     
     
     >
     
       and then perhaps the if statement that is executed could perhaps be 
     
     >
     
      readable.
     
     
Ok, because I've used the same logic block in a couple places I've 
separated into a separate function, and I've now split it into many if 
statements and such which should be much much more readable. I've also 
added many safeguards in the process that should get rid of the bug that 
caused the crash on metalforge and should hopefully prevent other 
similar bugs from cropping up (including some sanity checks that would 
indicate bugs in other places if they end up being triggered). I've 
tested this update and committed it to CVS. It isn't necessary, but it 
would be convenient if metalforge was updated to the latest CVS 
including this for more extensive testing before the upcoming release.

       Thanks,
          Alex Schultz