Problem with Look


#1

Hello:
So, I’ve uncovered a problem I didn’t realize I had. When someone does a do_look, it often times does not look at the proper item or person. This is not limited to just looking, but also when trying to use object X with object Y. Given that my code is a Rom2.4 base, does anyone have idea on where I could start looking as to the problem or what the problem might be? I’m not sure if I should be looking in do_look or the other function is_name or something else entirely.

Thanks,
Arholly


#2

If you can give more specifics or show some examples, that might help.


#3

Not a problem. Here is one:
>
> inv
> You are carrying:
> a pair of black leather dress shoes
> a long cloak
>
>
> exa shoes
> A pair of black leather dress shoes.
>
>
> exa cloak
> An impossibly descriptive individual
> Rayal is at full health.
> Something’s strange about Rayal. For starters he has:
>
> Rayal is using:
> < around the neck > a black bow tie
> < on the body > a dress shirt
> < on the legs > black trousers
> < about the shoulders > a dinner jacket
> < around the waist > a cummerbund
> < held in the hands > a sleek little mobile phone
>
>
Another example:
>
> inv
> You are carrying:
> a shotgun speed loader
> a pistol-grip shotgun
> a pair of black leather dress shoes
> a long cloak
>
>
> exa shotgun
> Somewhat disheveled, Paul squats here, fiddling with the stacks
> of newspapers and magazines.
> Paul is at full health.
> Something’s strange about Paul. For starters he has:


#4

Could you paste your entire do_examine and do_look functions?


#5

I sure can:

> void do_look( CHAR_DATA *ch, char *argument )
> {
> 	EXIT_DATA *pexit;
> 	CHAR_DATA *victim;
> 	OBJ_DATA *obj;
> 	ROOM_INDEX_DATA *original;
> 	ROOM_INDEX_DATA *looking;
> 	char arg1 [MIL]={'\0'};
> 	char arg2 [MIL]={'\0'};
> 	char arg3 [MIL]={'\0'};
> 	char *pdesc;
> 	int door = 0;
> 	int number = 0,count = 0;
> 
> 	CheckCH(ch);
> 
> 	if ( ch->desc == NULL )
> 		return;
> 
> 	/**********************************
> 	* Check if the person looking is  *
> 	* sleeping.                       *
> 	***********************************/
> 	if ( ch->position < P_SLEEP )
> 	{
> 		send_to_char( "You can't see anything but stars!\n\r", ch );
> 		return;
> 	}
> 
> 	/**********************************
> 	* Check if the person looking is  *
> 	* sleeping and ACT2_ASTRAL.       *
> 	***********************************/
> 	if ( ch->position == P_SLEEP && !IS_SET(ch->act2, ACT2_ASTRAL))
> 	{
> 		send_to_char("Your dreams are filled with nightmarish images!\n\r",ch);
> 		return;
> 	}
> 
> 	/**********************************
> 	* Check if the person looking is  *
> 	* currently Earthmelded.          *
> 	***********************************/
> 	if (IS_SET(ch->affected_by2, AFF2_EARTHMELD))
> 	{
> 		send_to_char("It's dark... really dark...\n\r", ch);
> 		return;
> 	}
> 
> 	/**********************************
> 	* Check if the person looking is  *
> 	* blind.                          *
> 	***********************************/
> 	if ( !check_blind( ch ) )
> 		return;
> 
> 	if(IS_SET(ch->act2, ACT2_ASTRAL))
> 		looking = ch->listening;
> 	else
> 		looking = ch->in_room;
> 
> 	/**********************************
> 	* Check if the person looking is  *
> 	* can see in the darkness.        *
> 	***********************************/
> 	if ( !IS_NPC(ch)
> 			&&   !IS_SET(ch->plr_flags, PLR_HOLYLIGHT)
> 			&&   !IS_AFFECTED(ch, AFF_DARK_VISION)
> 			&&   room_is_dark( looking ) )
> 	{
> 		send_to_char( "It is pitch black ... \n\r", ch );
> 		show_char_to_char( looking->people, ch );
> 		return;
> 	}
> 
> 	argument = one_argument( argument, arg1 );
> 	argument = one_argument( argument, arg2 );
> 	number = number_argument(arg1,arg3);
> 	count = 0;
> 
> 	if ( IS_NULLSTR(arg1) || !str_cmp( arg1, "auto" ) )
> 	{
> 
> 		/*************************************
> 		 * If an Admin, show the room number *
> 		 *************************************/
> 		if ((IS_ADMIN(ch)
> 				&& ((IS_NPC(ch) && ch->desc->original && IS_ADMIN(ch->desc->original))
> 						|| IS_SET(ch->plr_flags,PLR_HOLYLIGHT)))
> 						|| ch->pcdata->security > 0)
> 		{
> 			send_to_char(Format(" [\tORoom %d\tn] ",looking->vnum),ch);
> 		}
> 
> 		/*************************************
> 		 * If the character is in the Umbra, *
> 		 * show the Umbral room name.        *
> 		 *************************************/
> 		if(IS_SET(ch->act,ACT_UMBRA))
> 		{
> 			send_to_char( Format("\tW%s\tn", looking->uname), ch );
> 
> 			send_to_char( "\n\r", ch );
> 
> 			if(!IS_NPC(ch) && !IS_SET(ch->comm, COMM_BRIEF))
> 			{
> 				send_to_char( "\tB", ch);
> 				send_to_char( "  ",ch);
> 				send_to_char( looking->udescription, ch );
> 				send_to_char( "\tn", ch);
> 			}
> 		}
> 		else
> 		{
> 			send_to_char( Format("\tW%s\tn", looking->name), ch );
> 
> 			send_to_char( "\n\r", ch );
> 
> 			// Show the room description. Make the description text white.
> 			if(!IS_NPC(ch) && !IS_SET(ch->comm, COMM_BRIEF))
> 			{
> 				send_to_char( "\tW", ch);
> 				send_to_char( "  ",ch);
> 				send_to_char( looking->description, ch );
> 				send_to_char( "\tn", ch);
> 			}
> 		}
> 
> 		// Show the exits. Need to colorize.
> 		if ( IS_SET(ch->plr_flags, PLR_AUTOEXIT) )
> 		{
> 			send_to_char("\n\r",ch);
> 			do_function(ch, &do_exits, "auto");
> 		}
> 
> 		// Show objects in the room.
> 		send_to_char("\tW---\tYObjects\tW---\tn\n\r", ch);
> 		show_list_to_char( ch->in_room->contents, ch, FALSE, FALSE );
> 		send_to_char("\n\r", ch);
> 
> 		// Show people and mobs in the room.
> 		send_to_char("\tW---\tYPeople\tW---\tn\n\r", ch);
> 		show_char_to_char( ch->in_room->people, ch );
> 
> 		/*  if(IS_SET(ch->act, ACT_UMBRA))
> 		show_char_to_char( looking->people,   ch ); */
> 		return;
> 	}
> 
> 	/************************************************
> 	* Look in or on an object.                      *
> 	*************************************************/
> 	if ( !str_cmp(arg1, "in")  || !str_cmp(arg1,"on"))
> 	{
> 		/* 'look in' */
> 		if ( IS_NULLSTR(arg2) )
> 		{
> 			send_to_char( "Look in what?\n\r", ch );
> 			return;
> 		}
> 
> 		if ( ( obj = get_obj_here( ch, arg2 ) ) == NULL )
> 		{
> 			send_to_char( "You do not see that here.\n\r", ch );
> 			return;
> 		}
> 
> 		switch ( obj->item_type )
> 		{
> 		default:
> 			send_to_char( "That is not a container.\n\r", ch );
> 			break;
> 
> 		case ITEM_DRINK_CON:
> 			if ( obj->value[1] <= 0 )
> 			{
> 				send_to_char( "It is empty.\n\r", ch );
> 				break;
> 			}
> 
> 			send_to_char( Format("It's %s filled with a %s liquid.\n\r",
> 					obj->value[1] <     obj->value[0] / 4 ? "less than half-" : obj->value[1] < 3 * obj->value[0] / 4 ? "about half-"     : "more than half-",
> 					liq_table[obj->value[2]].liq_color), ch );
> 			break;
> 
> 		case ITEM_WEAPON:
> 			if(obj->value[0] == WEAPON_FIREARM)
> 			{
> 				send_to_char(Format("%s contains %d rounds.\n\r", obj->short_descr, obj->value[3]), ch);
> 			}
> 			break;
> 
> 		case ITEM_CONTAINER:
> 		case ITEM_CORPSE_NPC:
> 		case ITEM_CORPSE_PC:
> 			if ( IS_SET(obj->value[1], CONT_CLOSED) )
> 			{
> 				send_to_char( "It is closed.\n\r", ch );
> 				break;
> 			}
> 
> 			act( "$p holds:", ch, obj, NULL, TO_CHAR, 1 );
> 			show_list_to_char( obj->contains, ch, TRUE, TRUE );
> 			break;
> 
> 		case ITEM_FURNITURE:
> 			act( "$p holds:", ch, obj, NULL, TO_CHAR, 1 );
> 			show_list_to_char( obj->contains, ch, TRUE, TRUE );
> 			break;
> 		}
> 		return;
> 	}
> 
> 	/*************************************************
> 	* Look at a particular person                    *
> 	**************************************************/
> 	if ( ( victim = get_char_room( ch, arg1 ) ) != NULL && SAME_PLANE(victim,ch) )
> 	{
> 		show_char_to_char_1( victim, ch );
> 		return;
> 	}
> 
> 
> 	// if ( (victim = get_char_room(ch, arg1) ) != NULL && SAME_PLANE(victim, ch) )
> 	// {
> 	// 	if(IS_NULLSTR(arg2)) 
> 	// 	{
> 	// 		show_char_to_char_1( victim, ch );
> 	// 	} 
> 	// 	else if ( can_see( ch, victim ) && SAME_PLANE(victim, ch) 
> 	// 		&& (pdesc = get_extra_descr( arg2, victim->extra_descr )) != NULL)
> 	// 	{
> 	// 		if (++count == number)
> 	// 		{
> 	// 			send_to_char( pdesc, ch );
> 	// 			return;
> 	// 		}
> 	// 	} 
> 	// 	else 
> 	// 		act("You don't see anything special on $N like that.", ch, NULL, victim, TO_CHAR, 1);
> 	// 	if(!IS_SET(ch->act2, ACT2_ASTRAL))
> 	// 		act("$n looks at you.", ch, NULL, victim, TO_VICT, 1);
> 
> 	// 	return;
> 	// }
> 
> 	/**********************************
> 	* Look at objects                 *
> 	***********************************/
> 	for ( obj = ch->carrying; obj != NULL; obj = obj->next_content )
> 	{
> 		if ( can_see_obj( ch, obj ) )
> 		{
> 		    /* player can see object */
> 			pdesc = get_extra_descr( arg3, obj->extra_descr );
> 			if ( pdesc != NULL )
> 			{
> 				if (++count == number)
> 				{
> 					send_to_char( pdesc, ch );
> 					return;
> 				}
> 				else continue;
> 			}
> 
> 			pdesc = get_extra_descr( arg3, obj->pIndexData->extra_descr );
> 			if ( pdesc != NULL )
> 			{
> 				if (++count == number)
> 				{
> 					send_to_char( pdesc, ch );
> 					return;
> 				}
> 				else continue;
> 			}
> 
> 			if (!str_prefix(arg3, "package")
> 					&& IS_SET(obj->extra2, OBJ_PACKAGED))
> 				if (++count == number)
> 				{
> 					send_to_char( "An unobtrusively wrapped package sits here.", ch );
> 					send_to_char( "\n\r", ch);
> 					state_obj_cond(obj,ch);
> 					return;
> 				}
> 
> 			if ( is_name( arg3, obj->name ))
> 				if (++count == number)
> 				{
> 					send_to_char( obj->full_desc, ch );
> 					send_to_char( "\n\r", ch);
> 					state_obj_cond(obj,ch);
> 					return;
> 				}
> 		}
> 	}
> 
> 	for ( obj = looking->contents; obj != NULL; obj = obj->next_content )
> 	{
> 		if ( can_see_obj( ch, obj ) )
> 		{
> 			pdesc = get_extra_descr( arg3, obj->extra_descr );
> 			if ( pdesc != NULL )
> 				if (++count == number)
> 				{
> 					send_to_char( pdesc, ch );
> 					return;
> 				}
> 
> 			pdesc = get_extra_descr( arg3, obj->pIndexData->extra_descr );
> 			if ( pdesc != NULL )
> 				if (++count == number)
> 				{
> 					send_to_char( pdesc, ch );
> 					return;
> 				}
> 
> 			if (!str_prefix(arg3, "package")
> 					&& IS_SET(obj->extra2, OBJ_PACKAGED))
> 				if (++count == number)
> 				{
> 					send_to_char( "An unobtrusively wrapped package sits here.", ch );
> 					send_to_char( "\n\r", ch);
> 					state_obj_cond(obj,ch);
> 					return;
> 				}
> 
> 			if ( is_name( arg3, obj->name ) )
> 				if (++count == number)
> 				{
> 					send_to_char( obj->full_desc, ch );
> 					send_to_char( "\n\r", ch);
> 					state_obj_cond(obj,ch);
> 					return;
> 				}
> 		}
> 	}
> 
> 	pdesc = get_extra_descr(arg3,looking->extra_descr);
> 	if (pdesc != NULL)
> 	{
> 		if (++count == number)
> 		{
> 			send_to_char(pdesc,ch);
> 			return;
> 		}
> 	}
> 
> 	if (count > 0 && count != number)
> 	{
> 		if (count == 1)
> 		{
> 			send_to_char( Format("You only see one %s here.\n\r",arg3), ch);
> 		}
> 		else
> 		{
> 			send_to_char( Format("You only see %d of those here.\n\r",count), ch);
> 		}
> 
> 		return;
> 	}
> 
> 	if ( !str_cmp( arg1, "n" ) || !str_cmp( arg1, "north" ) ) door = 0;
> 	else if ( !str_cmp( arg1, "e" ) || !str_cmp( arg1, "east"  ) ) door = 1;
> 	else if ( !str_cmp( arg1, "s" ) || !str_cmp( arg1, "south" ) ) door = 2;
> 	else if ( !str_cmp( arg1, "w" ) || !str_cmp( arg1, "west"  ) ) door = 3;
> 	else if ( !str_cmp( arg1, "u" ) || !str_cmp( arg1, "up"    ) ) door = 4;
> 	else if ( !str_cmp( arg1, "d" ) || !str_cmp( arg1, "down"  ) ) door = 5;
> 	else if ( (door = get_door(ch, arg1)) == -1 )
> 	{
> 		send_to_char( "You do not see that here.\n\r", ch );
> 		return;
> 	}
> 
> 	/* 'look direction' */
> 	if ( ( pexit = looking->exit[door] ) == NULL )
> 	{
> 		send_to_char( "Nothing special there.\n\r", ch );
> 		return;
> 	}
> 
> 	if(IS_SET(pexit->exit_info, EX_WINDOW))
> 	{
> 		send_to_char(Format("%s\n\r", pexit->description == NULL ? "Through the window you see...\n\r" : pexit->description), ch);
> 		original = looking;
> 		obj = ch->on;
> 		char_from_room( ch );
> 		char_to_room( ch, pexit->u1.to_room );
> 		do_function(ch, &do_look, "");
> 		char_from_room( ch );
> 		char_to_room( ch, original );
> 		ch->on = obj;
> 	}
> 	else if ( pexit->description != NULL && !IS_NULLSTR(pexit->description) )
> 		send_to_char( pexit->description, ch );
> 	else
> 		send_to_char( "Nothing special there.\n\r", ch );
> 
> 	if ( pexit->keyword    != NULL && !IS_NULLSTR(pexit->keyword) && pexit->keyword[0] != ' ' )
> 	{
> 		if ( IS_SET(pexit->exit_info, EX_CLOSED) )
> 		{
> 			act( "The $d is closed.", ch, NULL, pexit->keyword, TO_CHAR, 1 );
> 		}
> 		else if ( IS_SET(pexit->exit_info, EX_ISDOOR) )
> 		{
> 			act( "The $d is open.",   ch, NULL, pexit->keyword, TO_CHAR, 1 );
> 		}
> 	}
> 
> 	return;
> }

and then do_examine:
void do_examine( CHAR_DATA *ch, char *argument )
{
char arg[MIL]={’\0’};
OBJ_DATA *obj;

	CheckCH(ch);

	one_argument( argument, arg );

	if ( IS_NULLSTR(arg) )
	{
		send_to_char( "Examine what?\n\r", ch );
		return;
	}

	do_function(ch, &do_look, arg);

	if ( ( obj = get_obj_here( ch, arg ) ) != NULL )
	{
		switch ( obj->item_type )
		{
		default:
			break;

		case ITEM_MONEY:
			if (obj->value[0] == 0)
			{
				if (obj->value[1] == 0)
				{
					send_to_char("Odd...there's no coins in the pile.\n\r", ch);
				}
				else if (obj->value[1] == 1)
				{
					send_to_char("Wow. One whole dollar.\n\r", ch);
				}
				else
				{
					send_to_char(Format("There are %d dollars in the pile.\n\r", obj->value[1]), ch);
				}
			}
			else if (obj->value[1] == 0)
			{
				if (obj->value[0] == 1)
				{
					send_to_char( "Wow. One cent.\n\r", ch);
				}
				else
				{
					send_to_char(Format("There are %d cents in the pile.\n\r", obj->value[0]), ch);
				}
			}
			else
				send_to_char(Format("There are %d dollars and %d cents in the pile.\n\r", obj->value[1],obj->value[0]),ch);
			break;

		case ITEM_WEAPON:
		case ITEM_DRINK_CON:
		case ITEM_CONTAINER:
		case ITEM_CORPSE_NPC:
		case ITEM_CORPSE_PC:
			do_function(ch, &do_look, (char *)Format("in %s", argument));
		}
	}

	return;
}

#6

Does this happen with the “look” command as well, or just when it gets passed through “examine”?


#7

is the object actually in the room?


#8

Just so we don’t overlook the obvious… can you confirm that Rayal doesn’t have “cloak” in his keywords and “Paul” doesn’t have “shotgun” in his keywords?

In the code you pasted, when looking, mobs get precedence over objects when not looking “in” or “on”.


#9

If you haven’t tampered with functions such as number_argument, get_obj_list, get_obj_carry, get_obj_here and so on, then I’m afraid you may have memory corruption problems there. For example, if your name strings are not zero terminated then it may happen that it finds matching keywords from memory regions where you should not read from.


#10

You function number_argument() is broken, and because of that your do_look function will always show the description of the character whenever you look at something that begins with the letter ‘c’.

You’ve got a line in number_argument that says: strncpy( arg, pdot+1, sizeof(*arg) )

sizeof(*arg) in this case is always 1 byte, since the size of a de-referenced char * is going to be the size of a single char. So, the strncpy always copies just the first letter of the number stripped argument into the space pointed to by arg. You probably meant it to copy the full number stripped argument.

The confusion here is that the function is given two string pointers, but no lengths, and sizeof() gives the byte size of the type- not the length of a string or of the array or memory block a pointer might be pointing into.

The reason it always shows the wrong thing when you look at something that begins with ‘c’ is because get_char_room is going to return a pointer to the ch whenever the arg returned after the call to number_argument() is a prefix of the string “ch->name”. number_argument(“cloak”,arg) sets arg to “c”, and that matches as a prefix of “ch->name”.

Change the strcpy in number_argument() to strcpy(arg,pdot+1) and it will probably work. (and fix the other strncpy in that function similarly)


#11

Was that in the ROM 2.4 base? Otherwise curious how you knew it was in number_argument() since I can’t see that it was posted here :slight_smile:


#12

No, @arholly posted a link to his git repo in a previous thread, so I was looking there.


#13

Thanks everyone for the input.
@RahjIII - That did the trick. Is there a better way to make use of the strncpy function then to do that or should I just leave it with the strcpy?

Thanks again.


#14

A misunderstanding that a lot of people run into when programming in C is to assume that there is some kind of built in data type for strings, and that the compiler provides basic runtime read/write protections with awareness of a string’s length via sizeof(). There isn’t, and it doesn’t.

In C, there are only single characters and pointers to memory locations that contain a single character. Every string datatype is built on top of that, and provides varying levels of speed, safety, and simplicity. The standard library <string.h> provides a string datatype that leans very heavily on the side of speed and simplicity. Safety wasn’t a big concern at all back when it was first designed, so a programmer who uses it has to be particularly aware of how it works to keep things safe.

The strcpy(dst,src) function copies bytes starting at the memory location pointed to by src into the memory location pointed to by dst, and stops when it encounters a terminating zero byte.

The big assumption that strcpy() makes is that the allocated segment of memory that dst points into will be long enough to hold all of the bytes that need to be copied between src and the first zero byte it finds. There is no additional safety net. If the programmer screws up and tells it to copy a string of bytes from a src that won’t fit into the segment pointed to by dst, the OS will throw you a Segmentation Fault, and you’re all done.

The strncpy(dst,src,n) puts kind of a safety valve on the function with that n parameter. It says “no matter what, always write exactly n bytes to dst”. If the length of the sequence of non-zero bytes starting at src is less than n, it will write additional zero bytes to dst to ensure that n bytes are written.

IF you know how much space you have for storing a string starting at src, using that for the n in strncpy() means it’ll never write more than that many bytes. It won’t overwrite and corrupt adjacent data within the same segment, and it won’t write outside of the segment and trigger a fault.

BUT if you don’t know how much space you have starting at src in the first place, there’s no way to know what value of n to use, and you are left in the same situation you were in with plain old strcpy().

So the way to use strncpy() is to know how much space you have available at dst (call that ‘len’), always write a ‘\0’ at dst+len, and then call strncpy() with len-1 as the size for n. Then no mater what kind of garbage you find at dst, you won’t try to write outside of the space allocated for dst, and you’ll always leave a null terminated string starting at dst.

In your case… it might be fun to try and refactor it, but I’d probably just leave it with a strcpy() rather than try to retrofit a way to pass in the length with every call.