[Home] [Downloads] [Search] [Help/forum]


Register forum user name Search FAQ

Gammon Forum

[Folder]  Entire forum
-> [Folder]  Programming
. -> [Folder]  General
. . -> [Subject]  [ROM] Reworked some code. Would like a second opinion.

[ROM] Reworked some code. Would like a second opinion.

It is now over 60 days since the last post. This thread is closed.     [Refresh] Refresh page


Posted by Rapture   USA  (13 posts)  [Biography] bio
Date Tue 31 Aug 2010 10:43 PM (UTC)
Message
Hello all,

I just got back into coding and found a nice base to start from ,QuickMUD, which is a cleaned up version of Rom2.4b6 with some basic additions. I also applied some bug fixes. In any event, I am working on redoing the fight code. Starting from the top of fight.c, I cleaned up the check_assist() function. I wanted to run it by you all to see if I foobared anything in the process. I found some issues with the stock code and hopefully made it more efficient as well as put in some comments for future reference.

Here is what I've got:
/* 
 * for auto assisting 
 * Commented, adjusted, and modified by Rapture (30Aug2010)
 */
void check_assist (CHAR_DATA * ch, CHAR_DATA * victim)
{
	CHAR_DATA *rch, *rch_next;

	/* rch = next person in the room with ch */
	for (rch = ch->in_room->people; rch != NULL; rch = rch_next)
	{
		rch_next = rch->next_in_room;

		/*
		 * Don't make victim to attempt to attack themselves OR
		 * Skip rch if they are already fighting
		 */
		if ( rch_next == victim || rch->fighting != NULL )
			continue;

		/* Skip rch if they are sleeping or blind */
		if ( !IS_AWAKE(rch) || IS_AFFECTED (rch, AFF_BLIND) )
			continue;

		/* quick check for ASSIST_PLAYER */
		/*
		 * If ch is a PLAYER and 
		 * rch is a MOBILE, is set to assist players, and the MOBILE's level is 7+ levels over the victim
		 * then the MOBILE(rch) will attack the victim
		 * For example, city guards
		 * skip rest of loop and check next person in room
		 */
		if ( !IS_NPC (ch) && ( IS_NPC (rch) && IS_SET(rch->off_flags, ASSIST_PLAYERS) && (rch->level + 6 > victim->level) ) )
		{
			do_function (rch, &do_emote, "screams and attacks!");
			multi_hit (rch, victim, TYPE_UNDEFINED);
			continue;
		}

		/* PCs next */
		/* 
		 * If ch is a PLAYER or ch is a MOBILE and is charmed 
		 */  //who has charmed ch????
		if ( !IS_NPC (ch) || IS_AFFECTED (ch, AFF_CHARM) )
		{
			/*
			 * If rch is a PLAYER and is set to auto-assist players OR rch is a MOBILE and is charmed AND
			 * rch is in the same group as ch, and can attack the victim
			 * then rch will attack the victim
			 * skip the rest of loop and check next person in room
			 */
			if ( ( ( !IS_NPC (rch) && IS_SET (rch->act, PLR_AUTOASSIST) ) || IS_AFFECTED (rch, AFF_CHARM) ) 
				&& is_same_group (ch, rch) && !is_safe (rch, victim) )
			{
				multi_hit (rch, victim, TYPE_UNDEFINED);
			}

			continue;
		} // end if (!IS_NPC (ch) || IS_AFFECTED (ch, AFF_CHARM))

		/* now check the NPC cases */
		/*
		 * If ch is a MOBILE and is not charmed AND
		 * if rch is a MOBILE
		 */
		if ( IS_NPC (ch) && !IS_AFFECTED (ch, AFF_CHARM) && IS_NPC(rch) )
		{
			/*
			 * if rch is set to assist all OR
			 * if rch is in the same group as ch OR
			 * if rch is the same race as ch and is set to assist same race as ch OR
			 * if rch is set to assist same alignment and alignment is same as ch OR
			 * if rch is the same mobile VNUM as ch and rch is set to assist same VNUM
			 */
			if ( IS_SET (rch->off_flags, ASSIST_ALL) || ( rch->group && rch->group == ch->group ) || 
				( rch->race == ch->race && IS_SET (rch->off_flags, ASSIST_RACE) ) ||
				( IS_SET (rch->off_flags, ASSIST_ALIGN) && 
				( ( IS_GOOD (rch) && IS_GOOD (ch) ) || ( IS_EVIL (rch) && IS_EVIL (ch) ) || 
				( IS_NEUTRAL (rch) && IS_NEUTRAL (ch) ) ) ) || 
				( rch->pIndexData == ch->pIndexData && IS_SET (rch->off_flags, ASSIST_VNUM) ) )
			{
				CHAR_DATA *vch;
				CHAR_DATA *target;
				int number;

				/*
				 * Flip a coin to see if MOBILE(rch) is going to actually do something
				 * otherwise, skip the rest of the loop and go on to next person in room.
				 */
				if (number_bits (1) == 0)
				{
					continue;
				}

				target = NULL;
				number = 0;

				/* Now, we are going to loop through all the people in the room again */
				for (vch = ch->in_room->people; vch; vch = vch->next)
				{
					/*
					 * If MONSTER(rch) can see next person (vch) in room AND
					 * if vch is in the same group as victim AND
					 * MONSTER(rch) has picked this person's random number
					 * then set MONSTER(rch)'s target as vch then repeat this 
					 * until we get through all the people in the room.
					 * NOTE: The first person in the victim's group is always the victim!
					 */
					if (can_see (rch, vch)
					    && is_same_group (vch, victim)
					    && number_range (0, number) == 0)
					{
						target = vch;
						number++;
					}
				}

				/* If MONSTER(rch) was able to pick someone then attack them! Go to next person in room. */
				if (target != NULL)
				{
					do_function (rch, &do_emote, "screams and attacks!");
					multi_hit (rch, target, TYPE_UNDEFINED);
				}
			}
		}
	}
}


The only part of this function I haven't messed with is the last for loop which picks a random person in the victim's group to attack. I'm not too confident in the original coder's choice of randomness, but I didn't want to introduce another for loop into the mix.

Does everything look copasetic?
[Go to top] top

Posted by Nick Gammon   Australia  (22,975 posts)  [Biography] bio   Forum Administrator
Date Reply #1 on Wed 01 Sep 2010 03:52 AM (UTC)
Message
Well it's hard to say if it is copasetic or not (especially when I had to look that word up). :P

Did you delete that thread and re-add it? It looks familiar.

Anyway, the proof of the pudding is in the eating, as they say. If it gives good results it is probably OK.

- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top

Posted by Twisol   USA  (2,257 posts)  [Biography] bio
Date Reply #2 on Wed 01 Sep 2010 04:26 AM (UTC)
Message
Nick Gammon said:
Did you delete that thread and re-add it? It looks familiar.

That's what I thought, too.

'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
[Go to top] top

Posted by Rapture   USA  (13 posts)  [Biography] bio
Date Reply #3 on Wed 01 Sep 2010 04:51 AM (UTC)
Message
Yes, I moved it from MUDs->Design concepts to programming. I figured it was more appropriate here.

It does work just fine in a closed environment, but I at least wanted a pro's opinion.
[Go to top] top

Posted by David Haley   USA  (3,881 posts)  [Biography] bio
Date Reply #4 on Wed 01 Sep 2010 05:20 AM (UTC)
Message
It's hard to tell without a before/after comparison. A side-by-side diff would be useful, or at the very least a "unified context" diff (try diff -C).

If you messed around with whitespace, use diff -w so that it doesn't report spurious whitespace changes.

David Haley aka Ksilyan
Head Programmer,
Legends of the Darkstone

http://david.the-haleys.org
[Go to top] top

Posted by Rapture   USA  (13 posts)  [Biography] bio
Date Reply #5 on Wed 01 Sep 2010 06:12 AM (UTC)
Message
Not sure if this will be readable, but here is a diff -cw:

This will be multiple posts....

Change 1:
***************
*** 101,122 ****
      return;
  }
  
! /* for auto assisting */
  void check_assist (CHAR_DATA * ch, CHAR_DATA * victim)
  {
      CHAR_DATA *rch, *rch_next;
  
      for (rch = ch->in_room->people; rch != NULL; rch = rch_next)
      {
          rch_next = rch->next_in_room;
  
!         if (IS_AWAKE (rch) && rch->fighting == NULL)
!         {
  
              /* quick check for ASSIST_PLAYER */
!             if (!IS_NPC (ch) && IS_NPC (rch)
!                 && IS_SET (rch->off_flags, ASSIST_PLAYERS)
!                 && rch->level + 6 > victim->level)
              {
                  do_function (rch, &do_emote, "screams and attacks!");
                  multi_hit (rch, victim, TYPE_UNDEFINED);
--- 101,139 ----
      return;
  }
  
! /* 
!  * for auto assisting 
!  * Commented, adjusted, and modified by Rapture (30Aug2010)
!  */
  void check_assist (CHAR_DATA * ch, CHAR_DATA * victim)
  {
  	CHAR_DATA *rch, *rch_next;
  
+ 	/* rch = next person in the room with ch */
  	for (rch = ch->in_room->people; rch != NULL; rch = rch_next)
  	{
  		rch_next = rch->next_in_room;
  
! 		/*
! 		 * Don't make victim to attempt to attack themselves OR
! 		 * Skip rch if they are already fighting
! 		 */
! 		if ( rch_next == victim || rch->fighting != NULL )
! 			continue;
! 
! 		/* Skip rch if they are sleeping or blind */
! 		if ( !IS_AWAKE(rch) || IS_AFFECTED (rch, AFF_BLIND) )
! 			continue;
  
  		/* quick check for ASSIST_PLAYER */
! 		/*
! 		 * If ch is a PLAYER and 
! 		 * rch is a MOBILE, is set to assist players, and the MOBILE's level is 7+ levels over the victim
! 		 * then the MOBILE(rch) will attack the victim
! 		 * For example, city guards
! 		 * skip rest of loop and check next person in room
! 		 */
! 		if ( !IS_NPC (ch) && ( IS_NPC (rch) && IS_SET(rch->off_flags, ASSIST_PLAYERS) && (rch->level + 6 > victim->level) ) )
  		{
  			do_function (rch, &do_emote, "screams and attacks!");
  			multi_hit (rch, victim, TYPE_UNDEFINED);


Basically, I moved some checks to the top of room-list loop to expedite code for blindness and a couple other things. Other than that, I think I spent more time sorting through the logic and commenting it for future reference.
[Go to top] top

Posted by Rapture   USA  (13 posts)  [Biography] bio
Date Reply #6 on Wed 01 Sep 2010 06:13 AM (UTC)
Message
Change 2:
***************
*** 124,165 ****
              }
  
              /* PCs next */
              if (!IS_NPC (ch) || IS_AFFECTED (ch, AFF_CHARM))
              {
!                 if (((!IS_NPC (rch) && IS_SET (rch->act, PLR_AUTOASSIST))
!                      || IS_AFFECTED (rch, AFF_CHARM))
                      && is_same_group (ch, rch) && !is_safe (rch, victim))
                      multi_hit (rch, victim, TYPE_UNDEFINED);
  
                  continue;
!             }
  
              /* now check the NPC cases */
! 
!             if (IS_NPC (ch) && !IS_AFFECTED (ch, AFF_CHARM))
              {
!                 if ((IS_NPC (rch) && IS_SET (rch->off_flags, ASSIST_ALL))
!                     || (IS_NPC (rch) && rch->group && rch->group == ch->group)
!                     || (IS_NPC (rch) && rch->race == ch->race
!                         && IS_SET (rch->off_flags, ASSIST_RACE))
!                     || (IS_NPC (rch) && IS_SET (rch->off_flags, ASSIST_ALIGN)
!                         && ((IS_GOOD (rch) && IS_GOOD (ch))
!                             || (IS_EVIL (rch) && IS_EVIL (ch))
!                             || (IS_NEUTRAL (rch) && IS_NEUTRAL (ch))))
!                     || (rch->pIndexData == ch->pIndexData
!                         && IS_SET (rch->off_flags, ASSIST_VNUM)))
                  {
                      CHAR_DATA *vch;
                      CHAR_DATA *target;
                      int number;
  
                      if (number_bits (1) == 0)
                          continue;
  
                      target = NULL;
                      number = 0;
                      for (vch = ch->in_room->people; vch; vch = vch->next)
                      {
                          if (can_see (rch, vch)
                              && is_same_group (vch, victim)
                              && number_range (0, number) == 0)
--- 141,214 ----
  		}
  
  		/* PCs next */
+ 		/* 
+ 		 * If ch is a PLAYER or ch is a MOBILE and is charmed 
+ 		 */  //who has charmed ch????
  		if ( !IS_NPC (ch) || IS_AFFECTED (ch, AFF_CHARM) )
  		{
! 			/*
! 			 * If rch is a PLAYER and is set to auto-assist players OR rch is a MOBILE and is charmed AND
! 			 * rch is in the same group as ch, and can attack the victim
! 			 * then rch will attack the victim
! 			 * skip the rest of loop and check next person in room
! 			 */
! 			if ( ( ( !IS_NPC (rch) && IS_SET (rch->act, PLR_AUTOASSIST) ) || IS_AFFECTED (rch, AFF_CHARM) ) 
  				&& is_same_group (ch, rch) && !is_safe (rch, victim) )
+ 			{
  				multi_hit (rch, victim, TYPE_UNDEFINED);
+ 			}
  
  			continue;
! 		} // end if (!IS_NPC (ch) || IS_AFFECTED (ch, AFF_CHARM))
  
  		/* now check the NPC cases */
! 		/*
! 		 * If ch is a MOBILE and is not charmed AND
! 		 * if rch is a MOBILE
! 		 */
! 		if ( IS_NPC (ch) && !IS_AFFECTED (ch, AFF_CHARM) && IS_NPC(rch) )
  		{
! 			/*
! 			 * if rch is set to assist all OR
! 			 * if rch is in the same group as ch OR
! 			 * if rch is the same race as ch and is set to assist same race as ch OR
! 			 * if rch is set to assist same alignment and alignment is same as ch OR
! 			 * if rch is the same mobile VNUM as ch and rch is set to assist same VNUM
! 			 */
! 			if ( IS_SET (rch->off_flags, ASSIST_ALL) || ( rch->group && rch->group == ch->group ) || 
! 				( rch->race == ch->race && IS_SET (rch->off_flags, ASSIST_RACE) ) ||
! 				( IS_SET (rch->off_flags, ASSIST_ALIGN) && 
! 				( ( IS_GOOD (rch) && IS_GOOD (ch) ) || ( IS_EVIL (rch) && IS_EVIL (ch) ) || 
! 				( IS_NEUTRAL (rch) && IS_NEUTRAL (ch) ) ) ) || 
! 				( rch->pIndexData == ch->pIndexData && IS_SET (rch->off_flags, ASSIST_VNUM) ) )
  			{
  				CHAR_DATA *vch;
  				CHAR_DATA *target;
  				int number;
  
+ 				/*
+ 				 * Flip a coin to see if MOBILE(rch) is going to actually do something
+ 				 * otherwise, skip the rest of the loop and go on to next person in room.
+ 				 */
  				if (number_bits (1) == 0)
+ 				{
  					continue;
+ 				}
  
  				target = NULL;
  				number = 0;
+ 
+ 				/* Now, we are going to loop through all the people in the room again */
  				for (vch = ch->in_room->people; vch; vch = vch->next)
  				{
+ 					/*
+ 					 * If MONSTER(rch) can see next person (vch) in room AND
+ 					 * if vch is in the same group as victim AND
+ 					 * MONSTER(rch) has picked this person's random number
+ 					 * then set MONSTER(rch)'s target as vch then repeat this 
+ 					 * until we get through all the people in the room.
+ 					 * NOTE: The first person in the victim's group is always the victim!
+ 					 */
  					if (can_see (rch, vch)
  					    && is_same_group (vch, victim)
  					    && number_range (0, number) == 0)
[Go to top] top

Posted by Rapture   USA  (13 posts)  [Biography] bio
Date Reply #7 on Wed 01 Sep 2010 06:13 AM (UTC)
Message
Change 3 (Last one):
***************
*** 169,174 ****
--- 218,224 ----
  					}
  				}
  
+ 				/* If MONSTER(rch) was able to pick someone then attack them! Go to next person in room. */
  				if (target != NULL)
  				{
  					do_function (rch, &do_emote, "screams and attacks!");
***************
*** 178,185 ****
              }
          }
      }
- }
-
[Go to top] top

Posted by David Haley   USA  (3,881 posts)  [Biography] bio
Date Reply #8 on Wed 01 Sep 2010 05:55 PM (UTC)
Message
It looks like your changes are mostly adding comments, which seem reasonable enough. There's that one big block of if statement conditions that got reformatted which I didn't check in detail, but I'm assuming you just reformatted/rearranged it.

However, I'm unsure why you added this:

if ( rch_next == victim


Your comment says that it's to avoiding having rch attack itself, but you're testing if the next character in the room is the victim. Did you mean rch == victim?

David Haley aka Ksilyan
Head Programmer,
Legends of the Darkstone

http://david.the-haleys.org
[Go to top] top

Posted by Rapture   USA  (13 posts)  [Biography] bio
Date Reply #9 on Wed 01 Sep 2010 08:11 PM (UTC)
Message
Yep, I overlooked that. Oops. Thanx!
[Go to top] top

The dates and times for posts above are shown in Universal Co-ordinated Time (UTC).

To show them in your local time you can join the forum, and then set the 'time correction' field in your profile to the number of hours difference between your location and UTC time.


23,792 views.

It is now over 60 days since the last post. This thread is closed.     [Refresh] Refresh page

Go to topic:           Search the forum


[Go to top] top

Quick links: MUSHclient. MUSHclient help. Forum shortcuts. Posting templates. Lua modules. Lua documentation.

Information and images on this site are licensed under the Creative Commons Attribution 3.0 Australia License unless stated otherwise.

[Home]


Written by Nick Gammon - 5K   profile for Nick Gammon on Stack Exchange, a network of free, community-driven Q&A sites   Marriage equality

Comments to: Gammon Software support
[RH click to get RSS URL] Forum RSS feed ( https://gammon.com.au/rss/forum.xml )

[Best viewed with any browser - 2K]    [Hosted at HostDash]