Function improvement

Posted by Zeno on Sat 11 Dec 2004 10:20 PM — 5 posts, 17,413 views.

Zeno USA Sat 11 Dec 2004 10:20 PM #0

I recently did a function to handle berserk (uncontrollable demon form), so they attack random things in the room, or travel elsewhere to find others. I am looking for comments on how to improve this function, problems it could cause, and so on. Basically, I'd like some criticism on it, so I can learn more.

void hanyou_dform_randoms( CHAR_DATA *ch )
{
    int chars=0;
    CHAR_DATA *rch;
    sh_int door;
    EXIT_DATA *pexit;

    if ( IS_NPC( ch ) || ch->race != 2 || ch->fighting || (ch->race == 2 && !xIS_SET(ch->act, PLR_TFORMED) ) )
        return;

    if ( IS_SET(ch->in_room->room_flags, ROOM_SAFE ) )
        return;

    if ( number_percent() < 30 )
        return;

    for ( rch = ch->in_room->first_person; rch; rch = rch->next_in_room )
    {
        if ( ch == rch || xIS_SET(rch->act, ACT_PACIFIST) )
           continue;
        if ( can_see( ch, rch ) )
        {
          chars++;
          if ( number_percent() < 20 )
          {
               act( AT_FIRE, "In a frenzy, you jump at $N!",  ch, NULL, rch, TO_CHAR    );
               act( AT_FIRE, "In a frenzy, $n jumps at you!", ch, NULL, rch, TO_VICT    );
               act( AT_FIRE, "In a frenzy, $n jumps at $N!",  ch, NULL, rch, TO_NOTVICT );
                if ( IS_NPC(rch) )
                  do_kill( ch, rch->name );
                else if ( !IS_NPC(rch) )
                  do_murder( ch, rch->name );
          }
        }
    }
    if ( chars < 1 )
    {
      door = number_door();
      pexit = get_exit( ch->in_room, door );
      if ( pexit != NULL && pexit->to_room )
      {
        act( AT_PLAIN, "Finding no victims, you wander elsewhere.",  ch, NULL, NULL, TO_CHAR    );
        move_char( ch, get_exit( ch->in_room, door ), 0, FALSE );
      }
    }


   return;
}

Yes there are some obvious things, such as putting all the ifchecks in one ifcheck near the top.

Greven Canada Sat 11 Dec 2004 11:26 PM #1

Well, I would use the RACE_* name instead of the number two, if it ever changed you would have to manually edit it, and people, yourself included, would understand with a quick look about which race your refereing too.

I would also look at your calls to do_kill and do_murder, have you tested that, should that one attack kill them, will rch still be valid, or will it have been cleared?

Also, with your call to do_kill for npc's, I think you need to pass the short_descr to it instead of name, I think it get_char checks against that instead of name, can't remember off hand though.

Also, just for clarity, I know its small and maybe its just me, I would check chars == 0 rather than < 1, same thing but it better expresses your purpose, IMO.

Also, you don't technically need the return; at the bottom, as any void when hitting the end of the function will return by default.

Zeno USA Sat 11 Dec 2004 11:36 PM #2

Good point on the race part.

Well I have tested it, and it works fine. But perhaps that could be a problem in the future.

Not the short desc, (unless I one_arg it) because they are the keywords for the NPC.

*shrug* If somehow it turns out to be negative, this is why I did is less than, instead of 0.

True, might as well remove that return.

David Haley USA Sun 12 Dec 2004 08:33 PM #3

The chars variable cannot go negative from the code you have; but comparing it against 0 is just better stylistically.

You should also do checks for closed doors and the like when the character wanders looking for victims.

Edit:
You should also probably break out of the for loop when you find a victim, because I don't believe you can do_kill on multiple people anyhow. This would also solve that rch problem Greven brought up.

Zeno USA Sun 12 Dec 2004 09:08 PM #4

Ah yes, since I only increase chars. Whoops, makes sense. Here is the redone function thanks to your suggestions.

void hanyou_dform_randoms( CHAR_DATA *ch )
{
    int chars=0;
    CHAR_DATA *rch;
    sh_int door;
    EXIT_DATA *pexit;
    char buf[MAX_STRING_LENGTH];

    if ( IS_NPC( ch ) || ch->race != RACE_HANYOU || ch->fighting || (ch->race == RACE_HANYOU && !xIS_SET(ch->act, PLR_TFORME$
        return;

    if ( IS_SET(ch->in_room->room_flags, ROOM_SAFE ) )
        return;

    if ( number_percent() < 30 )
        return;

    for ( rch = ch->in_room->first_person; rch; rch = rch->next_in_room )
    {
        if ( ch == rch || xIS_SET(rch->act, ACT_PACIFIST) )
           continue;
        if ( can_see( ch, rch ) )
        {
          chars++;
          if ( number_percent() < 20 )
          {
               act( AT_FIRE, "In a frenzy, you jump at $N!",  ch, NULL, rch, TO_CHAR    );
               act( AT_FIRE, "In a frenzy, $n jumps at you!", ch, NULL, rch, TO_VICT    );
               act( AT_FIRE, "In a frenzy, $n jumps at $N!",  ch, NULL, rch, TO_NOTVICT );
                  sprintf(buf, "hyoukaiform: %s atk %s.", ch->name, rch->name );
                  to_channel( buf, CHANNEL_DEBUG, "Debug", LEVEL_IMMORTAL );
                if ( IS_NPC(rch) )
                {
                  do_kill( ch, rch->name );
                  break;
                }
                else if ( !IS_NPC(rch) )
                {
                  do_murder( ch, rch->name );
                  break;
                }
          }
        }
    }

    if ( chars == 0 )
    {
      door = number_door();
      pexit = get_exit( ch->in_room, door );
      if ( pexit != NULL && pexit->to_room && !IS_SET(pexit->exit_info, EX_CLOSED ) )
      {
        act( AT_PLAIN, "Finding no victims, you wander elsewhere.",  ch, NULL, NULL, TO_CHAR    );
         sprintf(buf, "hyoukaiform: %s moves %s.", ch->name,  dir_name[door] );
         to_channel( buf, CHANNEL_DEBUG, "Debug", LEVEL_IMMORTAL );
        move_char( ch, get_exit( ch->in_room, door ), 0, FALSE );
      }
    }

}

Do not mind the debugs. For my use. Does it look all good now?