Sign in to follow this  
Kada2k6

Problem with variable being set to 0

Recommended Posts

fscanf(fp, "%hu %c %c %c %c\n", &Player.condition.health, &Player.condition.action, &Player.condition.hunger, &Player.condition.thirst, &Player.condition.fatigue);

Player.condition.action, Player.condition.hunger, Player.condition.thirst and Player.condition.fatigue are chars, not ints.


EDIT: No, that won't work, it'll try and read characters rather than numbers. This is why I hate the C I/O functions. Try this:
unsigned int action;
unsigned int hunger;
unsigned int thirst;
unsigned int fatigue;
fscanf(fp, "%hu %u %u %u %u\n", &Player.condition.health, &action, &hunger, &thirst, &fatigue);
Player.condition.action = action;
Player.condition.hunger = hunger;
Player.condition.thirst = thirst;
Player.condition.fatigue = fatigue;
Σnigma

Share this post


Link to post
Share on other sites
Hidden
Thanks Enigma, I'll try that. Feels a bit lame to have to do it that way though, but maybe it's the only solution? It's also weird, because I didn't have this problem earlier. It just started happening one day when I was coding. Memory leak?

Share this post


Link to post
This exactly what data breakpoints are for - you get told when data changes. I'd suggest setting a data breakpoint on the variable that gets set to 0 so you can what line of code causes it to be set to 0.

Share this post


Link to post
Share on other sites
Sorry, I scribbled out my (brief) explanation along with my incorrect answer earlier. The issue is your scanf format string says to read an unsigned short followed by four unsigned ints. The problem is, your last four parameters aren't ints, they're chars. Assuming a normal x86 platform your struct memory layout (unpacked) will be something like:
 +------Player.condition------+  +------Player.position-------+
/ \/ \
[00][01][02][03][04][05][06][07][08][09][10][11][12][13][14][15]
\ /\ / | | | | | | \ / | | | |
health padding |hunger |fatigue| | room_no | ypos |padding
| | | | | |
action thirst |padding xpos rotation
|
position
The first format code will cause scanf to read an unsigned short and write it starting at byte zero (&Player.condition.health), so this will write to bytes zero and one. The second format code will cause scanf to read an unsigned int and write it starting at byte 4 (&Player.condition.action). The problem is that the thing at byte 4 is not an unsigned int, but an unsigned char, so this is undefined behaviour. What will happen in practice on x86 is that bytes 4, 5, 6 and 7 will be interpreted as being an unsigned int and the value will be written there. The same problem happens with the next three format codes, causing bytes 5, 6, 7 and 8 to be written (which overwrites Player.position.position for the first time), then bytes 6, 7, 8 and 9 (overwriting Player.position.position again) and finally bytes 7, 8, 9 and 10 (overwriting Player.position.position a final time).

Σnigma

Share this post


Link to post
Share on other sites
May we ask why all those struct members are declared as unsigned char or unsigned short?

It looks like you've chosen to use the smallest type you can as an optimisation. Many of us here at one point or another have once done that. However we've since learned that unless you have many thousands of these in memory at once, you'll actually get better performance by simply using ints. The reason is that in order to do anything with those variables, e.g. add 5 to health, the short first gets extended to an int, and then it adds 5, and then it gets cut back down to a short for storage. If it was an int to begin with then the compiled app would just add 5 and that's it.

Note also that if you do store types of mixed sizes in a struct, that you'll get better performance, or less wasted space due to padding, if you declare your variables in the order of largest to smallest. In this case, putting room_number before position is all you'd do. But as mentioned, you're probably better off making them all ints anyway.

Share this post


Link to post
Share on other sites
The problem is that you are reading a bunch of values that are not of type `unsigned int' using fscanf("%u",...). fscanf will go ahead and write a whole unsigned int in the memory location you give it, even if it doesn't fit in your variable. On some platforms (e.g., Sparc) your program would crash because of a misaligned memory access.

Make everything int, as has been suggested already and which I also recommend, or read into temporary int variables that can then be explicitly casted down to the smaller types.

EDIT: I know I didn't add much new to the conversation, but I think the reason why the code wasn't working needed to be stated a little more clearly.

Share this post


Link to post
Share on other sites
Hidden
Thanks everyone for your replies.

Yeah, the reason I choose the smallest size is because this is a server application for an online game I'm writing (an "mmorpg" if you will), so I wanted it to consume as little memory as possible. But I realize this is 2008 and I could probably get away with using just normal ints, so I'll do that from now on. What was I thinking... Hehehe. :)

Thanks again for the help!

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this