 |
|
|
|
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Hi,
I've got a C++ question, and if somebody here can answer it then I'd be
happy :) It's probably something super simple, but all my Google
searches have led to nothing. I'm just now getting into using classes
and have had success until this point.
I have some code which I use to read a binary file into an array. It
works flawlessly when I implement it in a non-OO manner using an ugly
mess of functions and global variables in my main source file. The
problem comes when I try to do the same thing using classes. Here is my
loading member function:
void Field::load(int worldX, int worldY, string fileName){
int tempFx, tempFy;
int val;
FILE *fieldFile;
string fieldFileName;
fieldFileName=assignName(fileName,worldX, worldY);
if(fieldFile = fopen(fieldFileName.c_str(), "rb")){
fread(&tempFx, 2, 1, fieldFile);
fread(&tempFy, 2, 1, fieldFile);
for(int y=0;y<fieldHeight;y++){
for(int x=0;x<fieldWidth;x++){
fread(&val, 2, 1, fieldFile);
map[x][y]=val;
}
}
fclose(fieldFile);
}
else
for(int y=0;y<fieldHeight;y++){
for(int x=0;x<fieldWidth;x++){
map[x][y]=0;
}
}
}
It opens the files just fine, but when it reads values from them into an
array it returns nonsense. For instance Field.map[0][0] is supposed to
be 255, but what I get instead is 41746432.
I have tried using ifstreams instead, but I still get the same large
values even after making sure the file was read from the beginning. I
have also tried making "map" a new int (and deleting it in the
destructor), but I get the same large values.
It doesn't make sense to me why this code won't work inside a class, but
will work just fine elsewhere. Any help would be appreciated. I'm really
stumped :(
Sam
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
stbenge wrote:
> will work just fine elsewhere. Any help would be appreciated. I'm really
> stumped :(
Step one of being un-stumped is to figure out which of your assumptions in
the code is not holding true.
Print out fieldFileName.
Print out fieldFile.
Print out tempFx, tempFy, and val each time through the loop.
Print out the return values from fread.
print out sizeof(val).
The question you seek to answer is whether val is being read incorrectly, or
whether it's the assignment that is failing.
I also wonder why you read tempFx and tempFy and then don't use them for
anything. I'm assuming this is some header?
Showing the declaration of the rest of the class (including map) is probably
a good idea.
HTH.
--
Darren New, San Diego CA, USA (PST)
There's no CD like OCD, there's no CD I knoooow!
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Darren New wrote:
> stbenge wrote:
>> will work just fine elsewhere. Any help would be appreciated. I'm
>> really stumped :(
>
> Step one of being un-stumped is to figure out which of your assumptions
> in the code is not holding true.
>
> Print out fieldFileName.
> Print out fieldFile.
> Print out tempFx, tempFy, and val each time through the loop.
> Print out the return values from fread.
> print out sizeof(val).
>
> The question you seek to answer is whether val is being read
> incorrectly, or whether it's the assignment that is failing.
Thanks! I made my project a console application and printed the values
as you suggested. I found out that by not explicitly assigning "val" (or
tempFx, tempFy) an initial value, it was picking up nonsense and
carrying it through as it read through the stream. I do not know why it
would act this way inside a class member but not in other scenarios, but
I suspect it has to do with how C++ allocates memory.
> I also wonder why you read tempFx and tempFy and then don't use them for
> anything. I'm assuming this is some header?
Yes. The first two values in the binary file(s) describe the width and
height of a playing field for a game. I dumped the values into useless
variables so I could step through them without actually needing them,
and wind up at the correct point in the stream to start reading values
into the array. I may need to use these values another time, so I left
that functionality in the binary format.
> Showing the declaration of the rest of the class (including map) is
> probably a good idea.
I'm not sure I need to, now :)
Sam
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
stbenge wrote:
> as you suggested. I found out that by not explicitly assigning "val" (or
> tempFx, tempFy) an initial value, it was picking up nonsense and
> carrying it through as it read through the stream.
That doesn't make a whole lot of sense. fread() should be overwriting the
value. You might want to check the return value from fread to make sure
you're getting what you think you're getting. Plus, using sizeof(int) is
probably a better idea than "2". If it's really 2 and not sizeof(int) in
your data file, then you ought to be reading 2 unsigned chars and doing the
math to turn them into an integer lest you get endian problems. Remember
that &xyz doesn't necessarily point to the low byte of the integer.
I.e., it would be better to do something like
unsigned char buf[2];
if (2 != fread(buf, 2, 1, fieldFile)) {
printf("Whoops!");
exit(1);
}
val = buf[0] + 256 * buf[1];
/* That only works for unsigned integers, btw */
If you have four-byte integers, I wouldn't bet the way you've written it
works correctly. Especially not for signed values.
> I do not know why it
> would act this way inside a class member but not in other scenarios, but
> I suspect it has to do with how C++ allocates memory.
Well, data of global lifetime is specified as being initialized to 0 unless
otherwise initialized. It could just be whatever random crap was in your
RAM, if both classes and variables were stack allocated.
> I may need to use these values another time, so I left
> that functionality in the binary format.
Ok. The better thing to do here would be to check that tempFx and tempFy
match fieldWidth and fieldHeight, if it's your intention that they always
match. Then the reader knows what you're talking about, plus you get a
chance to blow up if you later feed it the wrong data file.
--
Darren New, San Diego CA, USA (PST)
There's no CD like OCD, there's no CD I knoooow!
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Darren New wrote:
> if (2 != fread(buf, 2, 1, fieldFile)) {
Which should, of course, be (1 != fread(...
--
Darren New, San Diego CA, USA (PST)
There's no CD like OCD, there's no CD I knoooow!
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Darren New wrote:
> stbenge wrote:
>> as you suggested. I found out that by not explicitly assigning "val"
>> (or tempFx, tempFy) an initial value, it was picking up nonsense and
>> carrying it through as it read through the stream.
>
> That doesn't make a whole lot of sense. fread() should be overwriting
> the value.
Maybe I'm just working with a different kind of C++. I'm using a Mingw
compiler. Can the standards be that different? fread() only returns the
correct values when I clear the &xyz read variable to 0 first. I only
need to do this once, though.
> You might want to check the return value from fread to make
> sure you're getting what you think you're getting.
Ah, I made sure to do this as well. I made the first four map entries 0,
1, 255 and 254. I printed this out to the console. This let me see what
to do about the problem. When I fixed it, I saw those numbers. My
program loaded and displayed the field correctly. Everything is perfect
now, I can change levels at a whim with the arrow keys. Should I still
be worried? Maybe I should create one or two extra Fields and display
them, just to be sure?
> Plus, using
> sizeof(int) is probably a better idea than "2". If it's really 2 and
> not sizeof(int) in your data file, then you ought to be reading 2
> unsigned chars and doing the math to turn them into an integer lest you
> get endian problems. Remember that &xyz doesn't necessarily point to the
> low byte of the integer.
>
> I.e., it would be better to do something like
> unsigned char buf[2];
> if (2 != fread(buf, 2, 1, fieldFile)) {
> printf("Whoops!");
> exit(1);
> }
> val = buf[0] + 256 * buf[1];
> /* That only works for unsigned integers, btw */
I tried this, but with no success. I ended up with a screen full of the
same tile. I think it is because I did not create the binary file in
this manner. I'll look this all over again later, because I would like
to do things in a safer manner.
> If you have four-byte integers, I wouldn't bet the way you've written it
> works correctly. Especially not for signed values.
They are four-byte integers, and this may be part of the previous
problem. This seems like the kind of issue which might bite me in the
@ss later if I don't follow standard procedure.
>> I do not know why it would act this way inside a class member but not
>> in other scenarios, but I suspect it has to do with how C++ allocates
>> memory.
>
> Well, data of global lifetime is specified as being initialized to 0
> unless otherwise initialized.
>
> It could just be whatever random crap was
> in your RAM, if both classes and variables were stack allocated.
This sounds like a reasonable explanation. I could attempt to prove that
theory by making some code changes and see if that alters the nonsense
value I was getting...
>> I may need to use these values another time, so I left that
>> functionality in the binary format.
>
> Ok. The better thing to do here would be to check that tempFx and tempFy
> match fieldWidth and fieldHeight, if it's your intention that they
> always match. Then the reader knows what you're talking about, plus you
> get a chance to blow up if you later feed it the wrong data file.
This sounds like good coding practice. I don't ever plan to feed it the
wrong files, but if I accidentally do, then at least I can crash the
program gracefully ;)
Sam
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
stbenge wrote:
> Maybe I'm just working with a different kind of C++. I'm using a Mingw
> compiler. Can the standards be that different? fread() only returns the
> correct values when I clear the &xyz read variable to 0 first. I only
> need to do this once, though.
The problem comes from ....
> be worried? Maybe I should create one or two extra Fields and display
> them, just to be sure?
... you reading 2-byte values into ...
> They are four-byte integers, and this may be part of the previous
> problem. This seems like the kind of issue which might bite me in the
> @ss later if I don't follow standard procedure.
... 4-byte integers. You can't expect that to work properly.
You need to either make it
val = buf[0] + 256 * buf[1]
or
val = buf[1] + 256 * buf[0]
depending on the endian-ness of the values you've written. Try your code
with a val of 64000 and see what you get. Or with a val of -10.
In other words, it works when you clear things to 0 first because the calls
to fread() are only initializing part of the integers. You're reading 2
bytes into a memory space allocated 4 bytes at a time, so whatever garbage
was in the high bytes is getting left there.
> This sounds like good coding practice. I don't ever plan to feed it the
> wrong files, but if I accidentally do, then at least I can crash the
> program gracefully ;)
Exactly. You should always check your return values. Get in that habit, and
you'll be in good shape. Also get in the habit of not assuming particular
memory layouts and not assuming particular sizes of integers. It's good to
get in the habit of not stepping outside the standard even if it happens to
work on the machine you're on, because 3 years from now when you move to a
different OS/CPU/whatever, you'll be surprised. (Indeed, even people on the
68000 got surprised when their code was run on a 68020 and all of a sudden
all 32 bits of an address register were significant, so all the folks
storing flag bits in the top byte were screwed.) Pretend you don't actually
know what the machine is doing underneath the definition of the language and
you'll write better code. If you spend too long relying on particular
implementation techniques, you'll never remember what's actually C and what
happens to be MingW.
The folks with conficker installed would almost certainly have preferred
that IE crash gracefully. :-)
--
Darren New, San Diego CA, USA (PST)
There's no CD like OCD, there's no CD I knoooow!
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Darren New wrote:
> The problem comes from you reading 2-byte values into 4-byte integers.
> You can't expect that to work properly.
>
> You need to either make it
> val = buf[0] + 256 * buf[1]
> or
> val = buf[1] + 256 * buf[0]
I made the changes you suggested and it works perfectly. There is no
need to set "val" to an initial value now that I am writing to the
correct data type. I can feel better about my code.
Now I can start working on a simple character class to test field
scrolling and collision detection. I'm really digging OOP at this point.
My global variables are few and I can load several fields at once. I
might even have a chance at finishing a game.
Your input has been very helpful! I don't know why I didn't think to
print values to the console for debugging before, it's so useful :) Now
I will consider my data types and memory usage more carefully. Thanks
Darren!
> The folks with conficker installed would almost certainly have preferred
> that IE crash gracefully. :-)
People should know by now not to use IE =P
Sam
P.S. I'm jealous. You live near some of the best mineral hunting grounds
in the U.S. ;)
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
stbenge wrote:
> Your input has been very helpful! I don't know why I didn't think to
> print values to the console for debugging before, it's so useful :) Now
> I will consider my data types and memory usage more carefully. Thanks
> Darren!
No problem. Glad to help.
It's not the printing of values per se that helps. It's the "figure out what
step isn't doing what you think it's doing." Whether you use an interactive
debugger, print statements, or hex core dumps, figuring out where the
process first goes wrong is the most important step. That's part of what
makes unsafe languages (i.e., those that don't check array bounds for
example) harder to debug - it's possible something goes wrong and breaks
code completely unrelated to the broken code. With a safe language, the code
does exactly what you wrote, regardless of what came before, even if what
you wrote isn't what you wanted.
> P.S. I'm jealous. You live near some of the best mineral hunting grounds
> in the U.S. ;)
I did not know that.
--
Darren New, San Diego CA, USA (PST)
There's no CD like OCD, there's no CD I knoooow!
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
|
|
