On Mon, 1 Apr 2002, Anarchangel wrote:
> On Mon, 1 Apr 2002, Dennis wrote:
>
> > On Mon, 1 Apr 2002, Anarchangel wrote:
> >
> > > Hello everyone,
> > >
> > > Every couple of weeks or so we'll crash with a problem in curse. Usually
> > > the problem is vo is NULL. It always happened when curse was either an
> > > affect that would be added after eating a pill, or when a scroll of curse
> > > was recited on an object. In trying to squash this bug, I may have found
> > > a problem in obj_cast_spell. Attached is the stock problematic(?) portion
> > > of the code.
> >
> > Wrong. Attached is your version of the code. It is no longer the stock
> > code, or even logically equivalent to it.
>
> I'm going to have to disagree here. Take a look at the stock code.
After adding the {} where you did, your code is no longer equivalent
to the stock code. It is completely different, and has a completely
different effect. In the stock code, the if(victim != NULL) ... code
is run whether (victim == NULL && obj == NULL) is true or false. In
your version, it is only run if (victim == NULL && obj == NULL) is true.
So I am going to have to disagree with your disagreement. :)
> if ( victim == NULL && obj == NULL)
> if (ch->fighting != NULL)
> victim = ch->fighting;
> else
> {
> send_to_char("You can't do that.\n\r",ch);
> return;
> }
>
> if (victim != NULL)
> {
> if (is_safe_spell(ch,victim,FALSE) && ch != victim)
> {
> send_to_char("Somehting isn't right...\n\r",ch);
> return;
> }
>
> vo = (void *) victim;
> target = TARGET_CHAR;
> }
> else
> {
> vo = (void *) obj;
> target = TARGET_OBJ;
> }
> break;
>
>
> Take a look at those indentions. The first line, if ( victim == NULL &&
> obj == NULL) lines up with only one thing, the break
> statement. The other else's are, therefore, meant to be else statements
> for if (ch->fighting != NULL). Notice how they line up.
The indention in this particular example sucks.
(As does most of the indention in ROM code.. Why can't people just use tabs
instead of mixing tabs and spaces and be nice to those of us who like
to be able to change our tab spacing!)
> Yes, I realize that the original intent of the code was to make else
> conditions for the first line. But it's written (or formatted, I should
> say) in such a way that it leads the reader to believe otherwise.
I agree. It is formatted very poorly, to make it look like it should have
a different meaning. But I propose that it is the formatting that is
wrong, and that the logic in the stock code is correct.
If you'll look closely, you'll see that your code (*after adding the else
statements*) is more or less logically equivalent to the stock code.
The step between, with the {} added, and the extra elses not there,
is NOT equivalent, and contain bugs.
Dennis
