[Samson]

I'm having a problem I can't quite pin down. Against many peoples' better judgement I've started down a C++ conversion of the AFKMud codebase. So far that's gone pretty well, even though it's still in the earlier stages. But just today I noticed a pretty serious problem. When doing a hotboot, reloading the object files that were saved prior to the boot creates an infinite loop as it boots up. I believe I've traced it to the function that loads the objects ( logs stop there and go no farther ). I think the obj_to_room conversion is to blame, and as I was examining it, I began to wonder. Just WTF does this thing do anyway? Why does obj_to_room return an OBJ_DATA pointer? It worked perfectly before the conversion, but obviously it's now now. So maybe I'm doing something wrong?

Alternately, maybe I just don't grasp how the obj_to_room code works, so I'm posting what I have here, including the obj_group function as well since that seems to play a key part in what's going on and maybe someone can spot what I've done wrong.

What calls this stuff in the hotboot code:

[code]
obj_from_char( tobj );
tobj = obj_to_room( tobj, room, supermob );
[/quote]

The replacement for the above is indicated below:

[code]
void read_obj_file( char *dirname, char *filename )
{
room_index *room;
FILE *fp;
char fname[256];
int vnum;

vnum = atoi( filename );
snprintf( fname, 256, "%s%s", dirname, filename );

if( !( room = get_room_index( vnum ) ) )
{
bug( "read_obj_file; ARGH! Missing room index for %d!", vnum );
unlink( fname );
return;
}

if ( ( fp = fopen( fname, "r" ) ) != NULL )
{
sh_int iNest;
bool found;
obj_data *tobj, *tobj_next;

rset_supermob( room );
for ( iNest = 0; iNest < MAX_NEST; iNest++ )
rgObjNest[iNest] = NULL;

found = true;
for(;; )
{
char letter;
char *word;

letter = fread_letter( fp );
if( letter == '*' )
{
fread_to_eol( fp );
continue;
}

if( letter != '#' )
{
bug( "%s", "read_obj_file; # not found." );
break;
}

word = fread_word( fp );
if( !str_cmp( word, "OBJECT" ) ) /* Objects */
fread_obj( supermob, fp, OS_CARRY );
else if( !str_cmp( word, "END" ) ) /* Done */
break;
else
{
bug( "read_obj_file; bad section; %s", word );
break;
}
}
FCLOSE( fp );
unlink( fname );
for( tobj = supermob->first_carrying; tobj; tobj = tobj_next )
{
tobj_next = tobj->next_content;
if( tobj->HAS_FLAG( ITEM_ONMAP ) )
{
supermob->SET_ACT_FLAG( ACT_ONMAP );
supermob->map = tobj->map;
supermob->mx = tobj->mx;
supermob->my = tobj->my;
}
tobj->from_char(); <---------- REPLACEMENT
tobj = tobj->to_room( room, supermob ); <------------ REPLACEMENT
supermob->REMOVE_ACT_FLAG( ACT_ONMAP );
supermob->map = -1;
supermob->mx = -1;
supermob->my = -1;
}
release_supermob();
}
else
log_string( "Cannot open obj file" );

return;
}
[/quote]

The old code:

[code]
/*
* If possible group obj2 into obj1 -Thoric
* This code, along with clone_object, obj->count, and special support
* for it implemented throughout handler.c and save.c should show improved
* performance on MUDs with players that hoard tons of potions and scrolls
* as this will allow them to be grouped together both in memory, and in
* the player files.
*/
OBJ_DATA *group_object( OBJ_DATA *obj1, OBJ_DATA *obj2 )
{
if ( !obj1 || !obj2 )
return NULL;
if ( obj1 == obj2 )
return obj1;

if( obj1->pIndexData->vnum == OBJ_VNUM_TREASURE || obj2->pIndexData->vnum == OBJ_VNUM_TREASURE )
return obj2;

if ( obj1->pIndexData == obj2->pIndexData
&& QUICKMATCH( obj1->name, obj2->name )
&& QUICKMATCH( obj1->short_descr, obj2->short_descr )
&& QUICKMATCH( obj1->objdesc, obj2->objdesc )
&& ( obj1->action_desc && obj2->action_desc && QUICKMATCH( obj1->action_desc, obj2->action_desc ) )
&& QUICKMATCH( obj1->socket[0], obj2->socket[0] )
&& QUICKMATCH( obj1->socket[1], obj2->socket[1] )
&& QUICKMATCH( obj1->socket[2], obj2->socket[2] )
&& obj1->item_type == obj2->item_type
&& xSAME_BITS( obj1->extra_flags, obj2->extra_flags )
&& obj1->magic_flags == obj2->magic_flags
&& obj1->wear_flags == obj2->wear_flags
&& obj1->wear_loc == obj2->wear_loc
&& obj1->weight == obj2->weight
&& obj1->cost == obj2->cost
&& obj1->level == obj2->level
&& obj1->timer == obj2->timer
&& obj1->value[0] == obj2->value[0]
&& obj1->value[1] == obj2->value[1]
&& obj1->value[2] == obj2->value[2]
&& obj1->value[3] == obj2->value[3]
&& obj1->value[4] == obj2->value[4]
&& obj1->value[5] == obj2->value[5]
&& obj1->value[6] == obj2->value[6]
&& obj1->value[7] == obj2->value[7]
&& obj1->value[8] == obj2->value[8]
&& obj1->value[9] == obj2->value[9]
&& obj1->value[10] == obj2->value[10]
&& !obj1->first_extradesc && !obj2->first_extradesc
&& !obj1->first_affect && !obj2->first_affect
&& !obj1->first_content && !obj2->first_content
&& obj1->count + obj2->count > 0
&& obj1->map == obj2->map
&& obj1->x == obj2->x
&& obj1->y == obj2->y
&& QUICKMATCH( obj1->seller, obj2->seller )
&& QUICKMATCH( obj1->buyer, obj2->buyer )
&& obj1->bid == obj2->bid ) /* prevent count overflow */
{
obj1->count += obj2->count;
obj1->pIndexData->count += obj2->count; /* to be decremented in */
numobjsloaded += obj2->count; /* extract_obj */
extract_obj( obj2 );
return obj1;
}
return obj2;
}

/*
* Move an obj into a room.
*/
OBJ_DATA *obj_to_room( OBJ_DATA *obj, ROOM_INDEX_DATA *pRoomIndex, CHAR_DATA *ch )
{
OBJ_DATA *otmp, *oret;
sh_int count = obj->count;
sh_int item_type = obj->item_type;
AFFECT_DATA *paf;

for( paf = obj->first_affect; paf; paf = paf->next )
room_affect( pRoomIndex, paf, TRUE );

for( paf = obj->pIndexData->first_affect; paf; paf = paf->next )
room_affect( pRoomIndex, paf, TRUE );

for( otmp = pRoomIndex->first_content; otmp; otmp = otmp->next_content )
if( ( oret = group_object( otmp, obj ) ) == otmp )
{
if( item_type == ITEM_FIRE )
pRoomIndex->light += count;
return oret;
}

LINK( obj, pRoomIndex->first_content, pRoomIndex->last_content, next_content, prev_content );
obj->in_room = pRoomIndex;
obj->carried_by = NULL;
obj->in_obj = NULL;
obj->room_vnum = pRoomIndex->vnum; /* hotboot tracker */
if( item_type == ITEM_FIRE )
pRoomIndex->light += count;
falling++;
obj_fall( obj, FALSE );
falling--;

/* Hoping that this will cover all instances of objects from character to room - Samson 8-22-99 */
if( ch != NULL )
{
if( IS_ACT_FLAG( ch, ACT_ONMAP ) || IS_PLR_FLAG( ch, PLR_ONMAP ) )
{
SET_OBJ_FLAG( obj, ITEM_ONMAP );
obj->map = ch->map;
obj->x = ch->x;
obj->y = ch->y;
}
else
{
REMOVE_OBJ_FLAG( obj, ITEM_ONMAP );
obj->map = -1;
obj->x = -1;
obj->y = -1;
}
}

if( obj->pIndexData->vnum == OBJ_VNUM_CORPSE_PC && falling < 1 )
write_corpses( NULL, obj->short_descr+14, NULL );
return obj;
}
[/quote]

The new code:
[code]
/*
* If possible group obj2 into obj1 -Thoric
* This code, along with clone_object, obj->count, and special support
* for it implemented throughout handler.c and save.c should show improved
* performance on MUDs with players that hoard tons of potions and scrolls
* as this will allow them to be grouped together both in memory, and in
* the player files.
*/
obj_data *obj_data;;group( obj_data *obj2 )
{
if ( !this || !obj2 )
return NULL;

if ( this == obj2 )
return this;

if( pIndexData->vnum == OBJ_VNUM_TREASURE || obj2->pIndexData->vnum == OBJ_VNUM_TREASURE )
return obj2;

if( pIndexData == obj2->pIndexData
&& QUICKMATCH( name, obj2->name )
&& QUICKMATCH( short_descr, obj2->short_descr )
&& QUICKMATCH( objdesc, obj2->objdesc )
&& ( action_desc && obj2->action_desc && QUICKMATCH( action_desc, obj2->action_desc ) )
&& QUICKMATCH( socket[0], obj2->socket[0] )
&& QUICKMATCH( socket[1], obj2->socket[1] )
&& QUICKMATCH( socket[2], obj2->socket[2] )
&& item_type == obj2->item_type
&& xSAME_BITS( extra_flags, obj2->extra_flags )
&& magic_flags == obj2->magic_flags
&& wear_flags == obj2->wear_flags
&& wear_loc == obj2->wear_loc
&& weight == obj2->weight
&& cost == obj2->cost
&& level == obj2->level
&& timer == obj2->timer
&& value[0] == obj2->value[0]
&& value[1] == obj2->value[1]
&& value[2] == obj2->value[2]
&& value[3] == obj2->value[3]
&& value[4] == obj2->value[4]
&& value[5] == obj2->value[5]
&& value[6] == obj2->value[6]
&& value[7] == obj2->value[7]
&& value[8] == obj2->value[8]
&& value[9] == obj2->value[9]
&& value[10] == obj2->value[10]
&& !first_extradesc && !obj2->first_extradesc
&& !first_affect && !obj2->first_affect
&& !first_content && !obj2->first_content
&& count + obj2->count > 0
&& map == obj2->map
&& mx == obj2->mx
&& my == obj2->my
&& QUICKMATCH( seller, obj2->seller )
&& QUICKMATCH( buyer, obj2->buyer )
&& bid == obj2->bid ) /* prevent count overflow */
{
count += obj2->count;
pIndexData->count += obj2->count; /* to be decremented in */
numobjsloaded += obj2->count; /* extract_obj */
obj2->extract();
return this;
}
return obj2;
}

/*
* Move an obj into a room.
*/
obj_data *obj_data;;to_room( room_index *pRoomIndex, char_data *ch )
{
obj_data *otmp, *oret;
sh_int ocount = count;
sh_int oitem_type = item_type;
AFFECT_DATA *paf;

for( paf = first_affect; paf; paf = paf->next )
pRoomIndex->room_affect( paf, true );

for( paf = pIndexData->first_affect; paf; paf = paf->next )
pRoomIndex->room_affect( paf, true );

for( otmp = pRoomIndex->first_content; otmp; otmp = otmp->next_content )
if( ( oret = otmp->group( this ) ) == otmp )
{
if( oitem_type == ITEM_FIRE )
pRoomIndex->light += ocount;
return oret;
}

LINK( this, pRoomIndex->first_content, pRoomIndex->last_content, next_content, prev_content );
in_room = pRoomIndex;
carried_by = NULL;
in_obj = NULL;
room_vnum = pRoomIndex->vnum; /* hotboot tracker */
if( oitem_type == ITEM_FIRE )
pRoomIndex->light += count;
falling++;
fall( false );
falling--;

/* Hoping that this will cover all instances of objects from character to room - Samson 8-22-99 */
if( ch != NULL )
{
if( ch->IS_ACT_FLAG( ACT_ONMAP ) || ch->IS_PLR_FLAG( PLR_ONMAP ) )
{
SET_FLAG( ITEM_ONMAP );
map = ch->map;
mx = ch->mx;
my = ch->my;
}
else
{
REMOVE_FLAG( ITEM_ONMAP );
map = -1;
mx = -1;
my = -1;
}
}

if( pIndexData->vnum == OBJ_VNUM_CORPSE_PC && falling < 1 )
write_corpses( NULL, short_descr+14, NULL );
return this;
}
[/quote]

The obj_data class I've converted to, in case it helps:

[code]
/*
* One object.
*/
class obj_data
{
public;
obj_data() { init_obj_data(); }
~obj_data();

void init_obj_data() { init_memory( &next, &map, sizeof( map ) ); }

/* Internal refs in object.c */
void fall( bool through );
int item_ego();
sh_int get_resistance();
char *oshort();
char *format_to_char( char_data *ch, bool fShort );
void show_list_to_char( char_data *ch, bool fShort, bool fShowNothing );
obj_data *to_char( char_data *ch );
void from_char();
int apply_ac( int iWear );
void from_room();
obj_data *to_room( room_index *pRoomIndex, char_data *ch );
obj_data *to_obj( obj_data *obj_to );
void from_obj();
void extract();
int get_number();
int get_weight();
int get_real_weight();
char *item_type_name();
bool is_trapped();
obj_data *get_trap();
bool extracted();
obj_data *clone();
obj_data *group( obj_data *obj2 );
void split( int num );
void separate();
bool empty( obj_data *destobj, room_index *destroom );
void remove_portal();
char_data *who_carrying();
bool in_magic_container();
void make_scraps();
int hitroll();
char *myobj();
EXTRA_DESCR_DATA *SetOExtra( char *keywords );
bool DelOExtra( char *keywords );

/* External refs in other files */
void armorgen();
void weapongen();

obj_data *next;
obj_data *prev;
obj_data *next_content;
obj_data *prev_content;
obj_data *first_content;
obj_data *last_content;
obj_data *in_obj;
obj_index *pIndexData;
room_index *in_room;
char_data *carried_by;
EXTRA_DESCR_DATA *first_extradesc;
EXTRA_DESCR_DATA *last_extradesc;
AFFECT_DATA *first_affect;
AFFECT_DATA *last_affect;
EXT_BV extra_flags;
struct mob_prog_act_list *mpact; /* mudprogs */
int mpactnum; /* mudprogs */
char *name;
char *short_descr;
char *objdesc;
char *action_desc;
char *owner; /* Who owns this item? Used with personal flag for Sindhae prizes. */
char *seller; /* Who put the item up for auction? */
char *buyer; /* Who made the final bid on the item? */
char *socket[3]; /* Name of rune/gem the item has in each socket - Samson 3-31-02 */
int bid; /* What was the amount of the final bid? */
sh_int day; /* What day of the week was it offered or sold? */
sh_int month; /* What month? */
sh_int year; /* What year? */
sh_int item_type;
unsigned short mpscriptpos;
int magic_flags; /*Need more bitvectors for spells - Scryn*/
int wear_flags;
sh_int wear_loc;
sh_int weight;
int cost;
sh_int level;
sh_int timer;
int value[11]; /* Raised to 11 by Samson on 12-14-02 */
sh_int count; /* support for object grouping */
int rent; /* Oh, and yes, this is being used ;) */
int room_vnum; /* Track it's room vnum for hotbooting and such */
sh_int mx; /* Object coordinates on overland maps - Samson 8-21-99 */
sh_int my;
sh_int map; /* Which map is it on? - Samson 8-21-99 */

bool HAS_FLAG( int bit ) { return( xIS_SET( extra_flags, bit ) ); }
void SET_FLAG( int bit ) { xSET_BIT( extra_flags, bit ); }
void REMOVE_FLAG( int bit ) { xREMOVE_BIT( extra_flags, bit ); }

bool HAS_MFLAG( int flag ) { return( IS_SET( magic_flags, flag ) ); }
void SET_MFLAG( int flag ) { SET_BIT( magic_flags, flag ); }
void REMOVE_MFLAG( int flag ) { REMOVE_BIT( magic_flags, flag ); }

bool CAN_WEAR( int bit ) { return( IS_SET( wear_flags, bit ) ); }
void SET_WEAR( int bit ) { SET_BIT( wear_flags, bit ); }
void REMOVE_WEAR( int bit ) { REMOVE_BIT( wear_flags, bit ); }
};
[/quote]

Originally Posted by (Samson @ May 09 2004,16:24) Why does obj_to_room return an OBJ_DATA pointer?

AFKMud, being a SMAUG derivative, has a feature whereby multiple copies of an object will coalesce to save system resources if they're grouped  (thus if you have a sword in inventory and someone gives you an identical sword, the new sword is purged and the old sword gets its "count" parameter incremented).  Obviously, if you want to move an object to the floor and still operate on it afterwards, you must anticipate such behavior.  Code such as
[code] obj_to_room( obj, room);
do_get( ch, obj );[/quote]
will crash if the obj pointer becomes invalid due to the coalescing;  the proper code would be
[code] obj = obj_to_room( obj, room );
do_get( ch, obj );[/quote]
or, if you want, even simply
[code] do_get( ch, obj_to_room( obj, room ) );
[/quote](although this slightly simpler still leaves the obj pointer possibly invalid and thus would have to be the last thing using that pointer)

[note that do_get obviously is misused above; this is done on purpose for the sake of making the results clearer]

[Samson]

Yeah, the grouping thing is basically what I got from it. I think I have that part pretty much figured out. I'm just not sure why the changeover to the C++ code is now causing the infinite loop problem. No doubt it'll turn out to be something horribly obvious too.

Originally Posted by (Samson @ May 10 2004,20:18) Yeah, the grouping thing is basically what I got from it. I think I have that part pretty much figured out. I'm just not sure why the changeover to the C++ code is now causing the infinite loop problem. No doubt it'll turn out to be something horribly obvious too.

Why are you trying to OOify obj_to_room?  Given its destructive nature (see above), it does not lend itself well to object oriented programming;  somewhere youve probably got some loop which is getting screwed up by invalid pointers due to a clumsy use of this very clumsy OOP.

...
Actually, why are you making SMAUG OO at all?  It is utterly pointless and a massive waste of time.

EDIT:  Oops, sorry, I meant to say "AFKMud", not "SMAUG" :P   (read:  SMAUG with a couple pages worth of buggy snippets added)

[Samson]

Right. I could have done without the cheap shot. Thanks anyway for at least taking the time to answer.