Open GoogleCodeExporter opened 8 years ago
Goes down next commit I do.
Thanks.
Original comment by forsaken...@gmail.com
on 24 Sep 2011 at 10:23
After taking a quick look at uses of Mem_Zero, I guess that it may cause more
problems.
Even if it doesn't cause crashes (haven't seen a String or some other more
complex type yet in one of the overwritten structs, but haven't checked all of
them), It most probably causes memory-leaks, because the structs contained
pointers.. if they are nulled, the data they point to may leak.
(But maybe it's just data on the stack or there are still other pointers, this
needs more investigation.)
Cheers,
- Daniel
Original comment by metalcae...@gmail.com
on 24 Sep 2011 at 10:45
Any Mem_Zeros on structs containing strings were a mistake brought over from
when we switched from C-style strings to mine; if you can find any, let me know.
Yes, std::string and std::vector are proven to work, but they are annoying to
work with at times and the API is not entirely clear in all cases - I created
custom classes which appear more user-friendly and adhere to our code
standards. They are simplistic templates, and have been tested in nearly every
use scenario to work just fine.
Any mem_zeros on structs, or at least ones that I've consciously written,
should have had all of their memory freed beforehand (keep in mind we use a
tagged memory system, and just free the entire tag, so we don't have to deal
with re-initialization and all that crap).
-P
Original comment by Jonno.5000
on 24 Sep 2011 at 11:13
Original comment by Jonno.5000
on 24 Sep 2011 at 11:26
Your custom String, List and Exception Classes don't even build on Linux/GCC
because of circular dependencies (Strings throw Exceptions and an Exception
gets a String as argument..).
I'm currently playing with a fixed r520. The Vector class didn't compile
either, but that was easy to fix, I may post a patch for that soon.
However, it seems like save games don't work (the game crashes on load) -
probably because SIntermissionState is just written as a blob into the savegame
and restored as such and std::string probably does not like this. I haven't
looked too close at your own String class, but I'd be surprised if it works -
after all it stores a pointer to heap allocated data (_array) containing the
string itself and this data will not be serialized when just dumping the
SIntermissionState struct as a blob (even if the data is saved somewhere else
the address won't be the same after loading the savegame, I guess).
Cheers,
- Daniel
Original comment by metalcae...@gmail.com
on 24 Sep 2011 at 11:31
Last time I checked, gcc built it just fine. Though, that was before the
"String" class was introduced... You're welcome if you want to do something
about that, as I'm pretty much done with linux.
Also, try making a small program where you write a class' contents into an ad
hoc file, and one of the class' contents is in fact a C string. See what
happens when you load it back. If the results are what you're expecting (char*
being written address instead of the contents) go ahead and I/we'll look into
it.
Original comment by forsaken...@gmail.com
on 25 Sep 2011 at 2:05
I've done this, using simplified versions of your Read()/Write() functions.
Note the segfault when trying to read the serialized file again :) (If it
doesn't segfault for whatever reason, it will at least not contain "What's
gonna happen?" but "muhahaha".)
You can also look at the file (bla.dat) in a hex editor: You won't find the
string in it.
Also: sizeof(Foo) doesn't change (of course not, it's a compile-time constant),
no matter how long the string pointed to by str is, so Write(buffer,
sizeof(TType)); etc can't possibly write f.str.
So to write structs that contain pointers to files, you need proper
serialization..
In fact, one reason for me to look at ccq2 was that proper serialization (and
thus more portable savegames) are a lot easier with object oriented code..
Original comment by metalcae...@gmail.com
on 25 Sep 2011 at 7:15
Attachments:
Exceptions throwing String was replaced in my copy, and now only accepts const
char* (this was done a long time ago, I just haven't commited it yet).
Again, as for std::string saving, it is completely unintended and I did not
write that consciously - basically what happened was I was focused more on
entities saving, and somewhere near the middle of the process I had changed
most of the fixed-length strings to std::string and hadn't really thought about
the consequences. I am completely aware of serialization, and before you go
yelling at me and going Y U NO SERIALIZE STD STRINGS DON'T FWRITE, look at 99%
of the rest of the game where I DO save everything properly.
It was an oversight, and nothing more.
-P
Original comment by Jonno.5000
on 25 Sep 2011 at 11:35
Also, if you want to be added to the project members list so you can commit
properly, just let me know - I have no problem with that. I just don't have
time lately with school to work on this as much as I could before, but I would
like to get it to a "working" state once again :)
-P
Original comment by Jonno.5000
on 25 Sep 2011 at 11:43
Original comment by Jonno.5000
on 26 Sep 2011 at 12:11
Am 26.09.2011 01:35, schrieb cleancodequake2@googlecode.com:
Ok, I may give the latest revision another try, but I still think using
std::string and std::vector would be a better idea than NIH-ish custom
classes. (I've had pretty bad experiences with custom String and List
classes in the past..)
Relax, my answer was referring to Forsaken's reply:
"Also, try making a small program where you write a class' contents into
an ad hoc file, and one of the class' contents is in fact a C string.
See what happens when you load it back. If the results are what you're
expecting (char* being written address instead of the contents) go ahead
and I/we'll look into it."
I just did what he suggested and explained, why it can't work.
Original comment by metalcae...@gmail.com
on 26 Sep 2011 at 9:17
WTF, the quoted text from comment 8 was removed..
my first paragraph was after your first paragraph (the one about strings) and
my second paragraph about your second paragraph (the one about serialization).
Cheers,
- Daniel
Original comment by metalcae...@gmail.com
on 26 Sep 2011 at 9:21
Yeah I know, I just want to make sure you know that I'm not incompetent :p
Wasn't meant to be aggressive.
I know a lot of people have bad experiences with custom classes, but C++ gives
us the tools to do this, and I felt it was a lot simpler to use our own than
try to adapt and mix styles with our style and STL's style, which are
completely different. Most large libraries tend to use new versions to make
positive that they are cross platform - although that's not necessarily the
case here, it does show that there are reasons to do so.
I have a few rookie programmers who I often have look at the code to see if
there is anything that "sticks out" to them, and back when we used STL they
wondered why they looked so out of place, so I figured that it'd be better if
they mixed better with the style I tried to go with.
Would you like to be a project member?
-P
Original comment by Jonno.5000
on 26 Sep 2011 at 9:52
Original issue reported on code.google.com by
metalcae...@gmail.com
on 24 Sep 2011 at 8:08Attachments: