Possible File I/O Problem

Posted by Trom on Fri 06 Feb 2004 01:29 AM — 22 posts, 81,390 views.

#0
my mud reads in the classes in load_obj_char(), which uses fread_char, fread_obj, fread_pet (fread_class). When it comes to reading the class properties, it works. But when its time to assign the values to the link list in the pfile, it doesn't... I put like log_string (prints to the dos screen) all over the function. It reads the values, assigns them, but when the function is over, it doesn't stick.

Default value for ch->classes->level is -1, after recieving the value from the pfile, it should be "15". The value is retrieved from the pfile, and is assigned to the pointer to the linked list for the ch structure. When the function is done, retrieving information from ch->classes->level shows -1 instead of 15. I've spent a while trying to pinpoint the exact problem, but i can't.

My guess is that it is assigned to the local variable but it doesn't make sense.

this is the function with arguments:
fread_class( CLASS_DATA *class, FILE *fp )

Before the class data is passed off, it finds the last place in the linked list (so its null), then uses that as the argument. fread_class then uses "alloc_mem" so it won't crash the mud, then assigns the values from the pfile.
Canada #1
Is it possible that in the end section of the fread_function its defaulting back to -1? or after its being read in, and thats all done, does it get reset in the load_classes section? It might help if you posted the function itself.
#2

void fread_class ( CLASS_DATA *cls, FILE * fp )
{

    char *word;
  //CLASS_DATA *tempclass = ch->classes;
  char buf[MSL];
  bool fMatch;
  CLASS_DATA *class = new_classes( );
  int temp = 0;

    for ( ;; )
    {
        word = feof ( fp ) ? "End" : fread_word ( fp );
    fMatch = FALSE;

        switch ( UPPER ( word[0] ) )
        {
            case '*':
                fMatch = TRUE;
                fread_to_eol ( fp );
        log_string("fread_char: *");
                break;

            case 'L':
        fMatch = TRUE;
        temp = fread_number ( fp );
        // test
        sprintf(buf, "fread_class, level : %d\n\r", temp);
        log_string(buf);
        // test
        class->level = temp;
        // test
        sprintf(buf, "fread_class, class->level : %d\n\r", class->level);
        log_string(buf);
        // test
        /*if ( !str_cmp( word, "Level" ) ) {
          temp = fread_number ( fp );
          sprintf(buf, "%d", temp);
          log_string(buf);
          tempclass->level = temp < 1 ? 1 : temp;
        }*/
                break;

            case 'I':
        fMatch = TRUE;
        temp = fread_number ( fp );
        // test
        sprintf(buf, "fread_class, id : %d\n\r", temp);
        log_string(buf);
        // test
        class->id = temp;
        // test
        sprintf(buf, "fread_class, class->id : %d\n\r", class->id);
        log_string(buf);
        // test
        /*if ( !str_cmp( word, "ID" ) ) {
          temp = fread_number ( fp );
          sprintf(buf, "%d", temp);
          log_string(buf);
          tempclass->id = temp < 0 ? 0 : temp;
        }*/
                break;

            case 'E':
                if ( !str_cmp ( word, "End" ) )   {
          if (cls == NULL) {
            cls = class;
          }
          else {
            cls->next = class;
          }
           return;
        }

                break;

        }

    if ( !fMatch )
        {
            bug ( "Fread_class: no match.", 0 );
            fread_to_eol ( fp );
        }
    }

  return;
  
}


Edited by Nick to add [code] so we can read it properly.
Amended on Sat 07 Feb 2004 08:01 PM by Nick Gammon
Canada #3
why not use the KEY macro? cause that always works, as far as I know:
	    KEY( "Level",	class->level,		fread_number( fp ) );
Alot more compact, although I realize that your current code if for test purposes. Also, if the second log message, the class->level one if fine, then its getting reset after that somewhere. You may want to read Nick's GDB guide for help something like this.
#4
i expanded it because it does the same thing, i wanted to make sure what was happening. It does assign, but i will have to check out the gdb info thing to see how its being reset, thanks.
Canada #5
Something else you might try, grep for class->level ("grep 'class->level' *.c"), and see if at any point it is being reset when it shouldn't be.
Amended on Fri 06 Feb 2004 04:43 PM by Greven
#6
done that already and its not, do you happen to have the gdb link?
Canada #7
http://www.gammon.com.au/forum/bbshowpost.php?bbsubject_id=3653
#8
I'm still having the same prob, not sure why its resetting. In load_char_obj() it assigns the default values to the class in ch then loads the values from the pfiles. Its acting like its doing that once again at the end. I've checked for resetting using visual c++ 6.0 find in files.
Canada #9
These are changes you've done? You could always rip it all out, make sure that everything else works, and then start again. I've had to do this, and its a real pain. However, another option is the GDB stuff, or you might paste what you get from the search for us to see.
#10
grep 'class->level' *.c

act_wiz.c: class->level = level;
music.c: if ( ((class_table[class->id].tier*10+50) > class->level) )
music.c: count += class->level / 5;
music.c: count += class->level / 3;
music.c: count += class->level;
save.c: sprintf(buf, "fread_class, after read loop, class->level : %d", ch->classes->level);
save.c: class->level = temp;
save.c: sprintf(buf, "fread_class, class->level : %d\n\r", class->level);
save.c: tempclass->level = temp < 1 ? 1 : temp;
skills.c: if ( ch->exp < get_exp_req(class_table[cn].tier, class->level+1)) {
skills.c: else if ( class->level >= 100 ) {
skills.c: ch->exp -= get_exp_req(class_table[cn].tier, class->level+1);
skills.c: class->level += 1;
skills.c: if ( ch->exp < get_exp_req(class_table[class->id].tier, class->level) ) {
skills.c: else if ( class->level >= 100 ) {
skills.c: ch->exp -= get_exp_req(class_table[class->id].tier, class->level);
skills.c: class->level += 1;

As for the entire function working, everything seems to load correctly, just not the classes. As in stats affects and objects return on character load up, but the class levels of a char are set to -1, even the character class id's work, level is the main prob.
Australia Forum Administrator #11
It isn't really clear what your problem is. You are saying when you read the pfile the class is wrong, but you have posted reading the classes, not the pfile.

First thing I would do is check that the classes are read properly, maybe just do a display in the class reading loop.

Next, when loading the *pfile* put a breakpoint around the spot where it loads the pfile, and see what is happening there. Is it assigning a number and then changing it, or assigning -1 in the first place?

Remember in SMAUG there are places where it ignores the level (eg. of objects) and recalculates them based on the area level.
#12
The mud is rot (rom/merc), fread_class(classes, fp), the pointer is reading directly from the pfile. It reads any sections in the pfile that are "#CLASS" then assigns the values to the next empty class in the pfile.

CLASS_DATA *class = ch->classes;
while (class != NULL)
class = class->next;

then class is passed off into fread_class(class, fp). While inside fread_class, the level value is read correctly, and it seems to assign also. But when the function is done reading the class, the value seems to be reset to -1 (which is its initializer in my mud. I don't see in the code were it does this, this is the problem.
Australia Forum Administrator #13
OK, I see what the problem is. This is it ...

Quote:

if (cls == NULL) {
cls = class;
}
else {
cls->next = class;
}


Why is this a problem? Well, you are passing down a *copy* of the class field (cls) to the function. This is not pass-by-reference.

Let me illustrate. Say I had this:


CLASS_DATA *cls = NULL;
FILE * fp = fopen ("somefile", "r");
fread_class (cls, fp );

// cls is still NULL, no matter what fread_class does



Inside fread_class you have changed "cls" to point to something, but you have changed the *copy* of cls, not the original. Thus, after the function exits cls is still NULL.

Probably the simplest thing is to pass down a pointer to the character (from which you can extract cls), because then if you change ch->classes then that will be OK. You are modifying something *inside* the pointer you are passing down, not the pointer itself.


#14
From my understanding i modified the code like this:

- removed the copy class inside of fread_class()

in load_char_obj(), instead of passing the ch->classes copy of the class variable then assigning memory to it, i did that before and gave fread_class() the resulting NULL class pointer with allocation assigned. Inside fread_class i made all assignments to the cls parameter (got rid of *class variable and made it all *cls).

Somehow i get the same problem. I'm planning to post the code that deals with fread_class in load_char_obj and the fread_class updated.
#15
The below are fragments from the functions that deal with the classes in my mud. When it comes to reading a class in load_char_obj, it checks to see if its null so it doesn't re-empty it (since there would be no need too), if its not null it scans to the end of the classes and makes space to add another, than moves to fread_class().

load_char_obj:
ch->classes = new_classes ( );
CLASS_DATA *cls = ch->classes;
sprintf ( strsave, "%s%s", PLAYER_DIR, capitalize ( name ) );
if ( ( fp = file_open ( strsave, "r" ) ) != NULL )
{
int iNest;

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 ( "Load_char_obj: # not found.", 0 );
break;
}

word = fread_word ( fp );
if ( !str_cmp ( word, "PLAYER" ) ) {
fread_char ( ch, fp );
}
else if ( !str_cmp ( word, "OBJECT" ) || !str_cmp ( word, "O" ) ) {
fread_obj ( ch, fp );
}
else if ( !str_cmp ( word, "CLASS" ) ) {
if (cls != NULL) {
while (cls != NULL)
cls = cls->next;
cls = new_classes( );
}
fread_class ( cls, fp );
}
else if ( !str_cmp ( word, "PET" ) ) {
fread_pet ( ch, fp );
}
else if ( !str_cmp ( word, "END" ) )
break;
else
{
bug ( "Load_char_obj: bad section.", 0 );
break;
}
}
file_close ( fp );
}

This is fread_class()

void fread_class ( CLASS_DATA *cls, FILE * fp )
{

char *word;
bool fMatch;


int temp = 0;

for ( ;; )
{
word = feof ( fp ) ? "End" : fread_word ( fp );
fMatch = FALSE;

switch ( UPPER ( word[0] ) )
{
case '*':
fMatch = TRUE;
fread_to_eol ( fp );
log_string("fread_class: *");
break;

case 'L':
fMatch = TRUE;
if ( !str_cmp( word, "Level" ) ) {
temp = fread_number ( fp );
cls->level = temp < 1 ? 1 : temp;
}
break;

case 'I':
fMatch = TRUE;
if ( !str_cmp( word, "Id" ) ) {
temp = fread_number ( fp );
cls->id = temp < 0 ? 0 : temp;
}
break;

case 'E':
if ( !str_cmp ( word, "End" ) )
return;

break;

}

if ( !fMatch )
{
bug ( "Fread_class: no match.", 0 );
fread_to_eol ( fp );
}
}

return;

}

Without the use of temp, the values would be put into the files directly as -1, which would probably crash the mud when it tries to lookup -1 in the class tables (class_table[-1]), until it works i'm going to keep that in.

new_classes() is a function like new_pcdata(), it assigns a default value of -1 to level and id of the class and before that alloc_mem() so it won't crash the mud when its referenced.
Amended on Mon 09 Feb 2004 04:59 PM by Trom
Canada #16
So the only place that its set to -1 ever is in new_classes? Then it looks like its being called somewhere after the class is read. Have you tried to set a breakpoint on it and see when its called?
#17
C:\cygwin\home\doe15\src\finger.c(81): victim->classes = new_classes ( );
C:\cygwin\home\doe15\src\recycle.c(552):CLASS_DATA *new_classes ( void )
C:\cygwin\home\doe15\src\recycle.h(112):CC *new_classes args ( ( void ) );
C:\cygwin\home\doe15\src\save.c(681): ch->classes = new_classes ( );
C:\cygwin\home\doe15\src\save.c(771): cls = new_classes( );

Did that in visual c and checked all of them, nothing that would cause a prob. I'm starting to wonder if this problem is possible to solve.
Australia Forum Administrator #18
You are doing the same thing, but in a different way ...

ch->classes = new_classes ( );
CLASS_DATA *cls = ch->classes;

...

if (cls != NULL) {
while (cls != NULL)
cls = cls->next;
cls = new_classes( );
}
fread_class ( cls, fp );


You are still changing the copy, not the original. This line:

cls = cls->next;

is only changing your local copy of the cls field.

Why bother with a copy? Get rid of the line:

CLASS_DATA *cls = ch->classes;

Then use "ch->classes" instead of "cls" in the function.

However I think it looks complicated. I would have had:

void fread_class ( CHAR_DATA * ch, FILE * fp )
{
...

}

And inside fread_class simply referred to ch->classes.
#19
If i use ch->classes, i won't be able to return to the start of the list. ch->classes is the list, if i traverse that list, then theres no way of returning to the start of it. What i want to do is a make a copy of it so that anything modified to that is modified in the true classes, but since its not actually it, i could use it anytime making pointers.. Basically i don't see how i can get the first ch->classes data if its assigned directly.

ch->classes = new_classes()
ch->classes->level = fread_number(fp);
ch->classes = ch->classes->next;
ch->classes->level = fread_number(fp);

how would you see the first node if you did it that way.. I'm probably misunderstanding.
Australia Forum Administrator #20

CLASS * cls = new_classes ();

if (ch->classes)
  ch->classes->next = cls;
else
  ch->classes = cls;


There is a difference. This method changes the actual class list. Your method changed "cls" but not "ch->classes".


#21
Thanks, i got it working using that method you showed previously.