Understanding the Code

[STiFU 774]

Damn, the XdFilechooser makes problems again on win32 release build on my side (didn't test x64), when I don't launch it through the IDE. It crashes on the initial load when the definition is duplicated and it crashes, when importing a duplicated definition regularly via the GuiSelector and the pagecounts of the XData definitions don't match up. Since its hard to do this without a debugger and since it all works perfectly when launched through the IDE, I guess I'll just disable previewing for XdFileChooser for this release. It's rather rare that you run over a duplicated definition anyway.

 

I uploaded the files in case you still want to take a look. The problematic definitions are "testing/test" and "readables/source_material/tdm_universe_city_book". You would just have to uncomment one line in onSelectionChanged() if you want to test it.

bla.xd.txt

readableeditortestbla.xd.txt

unnamed2.xd.txt

[greebo 103]

I was able to attach the debugger and catch an access violation in the ~RenderableCharacterBatch() destructor. The call stack is mostly useless, but the crash seems to be coming from my nvidia driver, in the call to glDeleteBuffersARB.

 

It's well possible that this is an openGL-related issue, probably due to some mistake on my behalf. I'll be off to bed today, maybe there's a chance that I look into it tomorrow or during the weekend.

[greebo 103]

Hm, no. This is not the right tree I'm barking up.

 

The first google search results in this link: http://stackoverflow.com/questions/2347562/program-crashes-only-in-release-mode-outside-debugger

 

It suggests that there is some memory leak/overwrite/corruption going on somewhere. This fits in the picture we're getting, as the behaviour is rather inconsistent.

[STiFU 774]

Ah ok. Well I guess we should postpone the release then for now. If it's really a leak, commenting previewing out in a class probably is not really a fix.

 

I'll have to check my code and see what could possibly leak. Seeing that I used shared_ptr nearly everywhere, one would think a leak would be easy to find.

 

Edit: Uhm, yeah! Little embarrassing!! Forgot to clear the vectors in the destructor of the XData class... I think that fixed it. At the time I started coding I wasn't familiar with the STL yet, so I didn't realize std::vector is a dynamic structure that need to be freed. Sorry!

[greebo 103]

The std::vector class is clearing itself upon destruction, or were you allocating them on the heap, using new std::vector()?

 

edit: the crash is still there.

 

edit2: It's reproducible: create a readable, open dialog, open xdataselector, choose readables/source_material/tdm_universe_city_book, OK, select the third option unnamed2.xd, hangup.

 

This is my xdata folder: http://www.dramthethief.com/darkmod/files/xdata.7z

[STiFU 774]

Nope, I never used regular pointers. I only use the new pointer on the shared_ptr constructor...

 

Same goes for std::map I assume?

 

@edit2: Yeah, that seems to be that issue with the amount of pages again. The first two readables have 3 pages and the third has 4. Deleting the two page4 statements fixes it, although num_pages still says 4 and there's also a forth gui defined.

 

Edit: I attached DarkRadiant.exe to the debugger and there's a runtime_error exception on the DefTokeniser in the hasMoreTokens() method. The tokeniser object itself cannot be evaluated. Seems like that one doesn't exist anymore for some strange reason. I am not very familiar with this kind of debugging though. I am doing this on the release build, maybe that has something to do with it? The weird thing is that when I continue execution, everything imports fine and the preview works as well.

[greebo 103]

Same goes for std::map I assume?

Yes. All objects in an STL container will get their destructor called when the container itself is cleared or destroyed.

 

@edit2: Yeah, that seems to be that issue with the amount of pages again. The first two readables have 3 pages and the third has 4. Deleting the two page4 statements fixes it, although num_pages still says 4 and there's also a forth gui defined.

I take it, the amount of pages makes it crash in release builds only? That'd be strange.

 

Edit: I attached DarkRadiant.exe to the debugger and there's a runtime_error exception on the DefTokeniser in the hasMoreTokens() method. The tokeniser object itself cannot be evaluated. Seems like that one doesn't exist anymore for some strange reason. I am not very familiar with this kind of debugging though. I am doing this on the release build, maybe that has something to do with it? The weird thing is that when I continue execution, everything imports fine and the preview works as well.

Debugging a release build has some glitches, as many objects present in the code (like temporaries) can be optimised away in the actual release build, so it might appear that the instruction pointer is skipping lines. Also, you cannot always inspect the classes in the memory.

 

Make sure the crash is reproducible and it happens always in the same place, even if you do something different before the actual reproduction steps. If the crash is not occurring in the same location every time and seems to randomly depend on the action you do before reproducing the crash, there is definitely some heap corruption going on.

 

If all else fails, there's "cave-man debugging", which means that you speckle your code with std::cout output (disabling the StdCOutRedirector in the main radiant binary) and check where the crash is happening. This is how I debugged the first year in my DarkRadiant development, because I didn't have a working debugger back then. What a pain, but you learn a lot of things about the code and you know class names by heart afterwards.

[STiFU 774]

Ok, I think I got it. Stupid again, actually! Forgot to initialize _numPages on XData objects. Well, I intended too, but I used the wrong method. So what happens is that the last value was used (3 in this case) and the setContent resulted in the runtime_error then because the acquired pageIndex exceeded numPages.

 

Yup, that fixed it! FINALLY!! I'll compile the release builds now then after some further testing and upload to sourceforge, okay?

[greebo 103]

Yup, that fixed it! FINALLY!! I'll compile the release builds now then after some further testing and upload to sourceforge, okay?

Wait a sec, don't be so quick on the draw. I'd like to do some rudimentary checking on my own before we go ahead and upload it. On top of that, I also need a chance to update the darkradiant website, and I'm in the office currently where I can't do that.

 

Glad you found the issue though, finally that one is fixed.

[STiFU 774]

Ok, sure! I tend to get a little over-exited, sorry. Especially regarding Springs announcement. Would be a shame to waste the opportunity, when we're basically set! I'll give him a hint about our status. Maybe he decides to wait a little, if not, fine as well!

 

I'll get the packages ready for release and when you give the "ok", I can directly upload them, unless you want to do it yourself.

[Crispy 8]

I wouldn't be too concerned about a simultaneous announcement. There's definitely an argument that two separate announcements are better than one!

 

(This, by the way, is exactly the same reason why game developers like to tease about their games slowly - release little bits of information regularly, you ultimately wind up with more attention that way.)

[STiFU 774]

I just doublechecked our dependencies. There were actually some missing. I wasn't quite sure how to add the MODULE_ARCHIVE dependency. I appended "PK4" for now. Module names are also missing for idialogmanager.h, itextstream.h and irenderable.h. I assume those don't need to be added to the dependencies?

[greebo 103]

I'll go through them. You won't need the ArchivePK4 dependency, as this is implicitly handled by the VFS. You don't access PK4/ZIP files directly, do you?

[STiFU 774]

Nope, all VFS. If you go over it later anyway, you can just remove it.

[greebo 103]

Ok, done, not a big deal.

 

I briefly tested the 32 bit version, I think we're ok. Do you know exactly which files to create and where to upload them? We need both 32 bit and 64 bit installers as well as ZIPs. You can use the DarkRadiant_install folders to create the ZIPs.

[STiFU 774]

I read the article, if that's what you mean! I am currently looking up how to upload to sourceforge.

[greebo 103]

SFTP: frs.sourceforge.net

User: schtifu,darkradiant

Pw: Your sourceforge password

 

Remote folder: /home/frs/project/d/da/darkradiant/darkradiant

[STiFU 774]

Yup, got it already. Uploading right now! Although the information on sourceforge seems to be outdated. I am currently uploading to /home/pfs/project/d/da/darkradiant/darkradiant/1.2.0 . Clicking on frs changed my directory to that folder. Tried it multiple times. I guess they've renewed that system.

[greebo 103]

Tried it multiple times. I guess they've renewed that system.

Yes, the release system has been changed every of the last three times I prepared a release. Those guys seem to make a sport out of it. For me that's just tiresome to figure out where to put the fricking files everytime again.

[STiFU 774]

Done! All files are tested.

[greebo 103]

Well done, thanks. I updated the website in the meantime, I think we're set.

[STiFU 774]

The image of the readable editor on sourceforge doesn't show on my side.

[greebo 103]

Ah, forgot to upload the images, will be fixed in a minute.

[STiFU 774]

Also, maybe you should add somewhere, that the readable editor only works with TDM 1.01 .

[greebo 103]

Yeah, added that. Though people will notice soon enough.

 

Next up will be some planning towards 1.3.0, I'm going to push a few issues on the roadmap. The transparency rendering issue or the I18N project sound like suitable near-term goals.

 

What are you going to tackle next? Refactoring the XData class structure?