Understanding the Code

[STiFU]

Oh my God. It works perfectly well for me. I get the message "failed to import[...]" just like intended. Try to browse guis first and see if it happens too then. Maybe it's somewhat related to my issue.

 

The question about my issues is, how do you generally approach a bug that doesn't occur when launched through your IDE?

[greebo]

Oh my God. It works perfectly well for me. I get the message "failed to import[...]" just like intended.

Hm, don't get that exception anymore, it's gone now, hopefully for good.

 

The question about my issues is, how do you generally approach a bug that doesn't occur when launched through your IDE?

I've had the opposite case, where things were crashing when run in Visual Studio, but not when run from explorer. Generally, the difference between these cases is that Visual Studio employs what is called the "debug heap", which performs additional error checking with respect to memory allocation.

 

Doesn't help in your case though, I guess there's something else going wrong, but it could still be memory-related.

[greebo]

There's still something off with the xdata importer. I have the file unnamed.xd in my darkmod/xdata folder, containing a duplicate definition (I guess it has been copied over from the test_readables.xd when I hit "Save" in the dialog.

 

unnamed.xd.txt

 

There is a std::runtime_error thrown, at least that's what it looks like. Here's the stacktrace:

 

dm.gui.dll!XData::OneSidedXData::setPageContent(XData::ContentType cc=Title, unsigned __int64 pageIndex=0, XData::Side side=Right, const std::basic_string,std::allocator > & content="Power Structure

") Line 340 + 0x4d bytes C++

> dm.gui.dll!XData::XDataLoader::storeContent(const std::basic_string,std::allocator > & statement="page1_title", parser::DefTokeniser * tok=0x000000000012c700, const std::basic_string,std::allocator > & defName="readables/source_material/tdm_universe_city_book", const std::basic_string,std::allocator > & content="") Line 287 C++

dm.gui.dll!XData::XDataLoader::parseXDataDef(parser::DefTokeniser & tok={...}, const std::basic_string,std::allocator > & definitionName="readables/source_material/tdm_universe_city_book") Line 163 + 0x4c bytes C++

dm.gui.dll!XData::XDataLoader::importDef(const std::basic_string,std::allocator > & definitionName="readables/source_material/tdm_universe_city_book", std::map<:basic_string>,std::allocator >,boost::shared_ptr<:xdata>,std::less<:basic_string>,std::allocator > >,std::allocator<:pair>,std::allocator > const ,boost::shared_ptr<:xdata> > > > & target=[1](("darkmod/xdata/test_readables.xd",{px=0x0000000016bbf970 pn={...} })), const std::basic_string,std::allocator > & filename="") Line 50 + 0x2b bytes C++

dm.gui.dll!ui::XdFileChooserDialog::import(const std::basic_string,std::allocator > & defName="readables/source_material/tdm_universe_city_book", boost::shared_ptr<:xdata> & newXData={...}, std::basic_string,std::allocator > & filename="", boost::shared_ptr<:xdataloader> & loader={...}, ui::ReadableEditorDialog * editorDialog=0x000000000012de50) Line 22 + 0x4b bytes C++

dm.gui.dll!ui::ReadableEditorDialog::initControlsFromEntity() Line 542 + 0xa3 bytes C++

dm.gui.dll!ui::ReadableEditorDialog::_postShow() Line 127 + 0xa bytes C++

[STiFU]

Arrr that damn importer is a real pain. Should never have started with two separate XData subclasses. I'll definitely refactor the whole crap eventually, maybe even before the release if I find the time. Aside from the XData classes the ReadableEditorDialog would have to be adjusted quite a bit too though. String-based content requests, dynamic textview creation et cetera. I'll also have to make sure we maintain a comfortable gui. Phew!! Good thing the support for the import-directive already works like XData is supposed to work, so I'll only need to replace those content-vectors by a single map storing all statements. But this time, before implementing the whole thing, I'll have a look at the TDM XData parsing code...

 

There are two awkward things about the bug you posted. Firstly tdm_universe_city_book was stored under the map derived filename while imported files should be stored in their original file, secondly I thought the implementation of the importers was exception-free. The Side-parameter passed to setPageContent() is also "Right". This generally is no problem, as that parameter is discarded on OneSidedXData, but it could be an indicator of what's going wrong here. I'll see tomorrow... Thanks for the report!

[greebo]

Ok, the linux build scripts are updated now too. This should pave the way for the upcoming release, save any bug fixes that are left to do.

 

Good luck for the exam tomorrow, btw.

[STiFU]

Thanks. I hope it works out ok. It's a pretty nasty one!!

 

save any bug fixes that are left to do.

*BAM*! That was the language barrier we just hit... Not quite sure what to do with this. Should I save bugfixes for later until it's released or should I save the last missing ones to the SVN? The latter sounds more logical to me, though!! I mean, we still need to fix some things, but I don't want to possibly let all the makefile work you've done go to waste, so I rather ask before...

[greebo]

I meant something along the lines of: "we're set for the release, except for the few bug fixes we still have to do." So feel free to commit your changes, bugfixes should definitely go in before release. It's not like I want to rush anything anyway, so there's plenty of time for you.

[STiFU]

Understood!!

 

I can't reproduce your problem though. What exactly did you do? Both readables open fine using the XData browser or the entry, as well as the initial load. (x86 debug build currently)

[STiFU]

I just discovered another issue as well. xport() with the command MergeOverwriteExisting overwrote too much in the string, destroying the following definition. I already fixed it. Seems like fstream::get() returns more characters than the derived string has. Replaced it all by string/char and it works like intended now. Weird!

[STiFU]

And yet again an issue with the importer. What does this statement do?

import "readables/source_material/tdm_universe_city_book"

Import all statements found inside the sourcefile? A syntax like this currently is not supported. I could add it though I guess. I assume the local statements are preferred over imported ones. At the moment after the import statement an '{' is asserted and a trailing "} from [...]".

 

Nevermind, I'll just have a look at the TDM sourcecode.

[STiFU]

Hmn, I am thinking to leave the importer as it is for now. If I add the missing features regarding the import-directive, I would (other than I initially thought) have to restructure it all quite a bit and it will become really dirty.

 

I will rewrite the whole thing anyway eventually and it'd just be wasted effort to implement it now because of that. And I am thinking, as long as the users can import and export their own readables, everything should be fine.

[greebo]

I'm fine with that. Things will come to the point where the maintainer (== you in this case) will wipe everything off the table and come up with the "proper" design. The first stabs at things (especially at problems you don't know the exact requirements beforehand, like this one) are bound to end up with one or more design issues.

 

From what I've skimmed, I think the design is missing a central manager class (like XDataManager) managing the loading, caching, reloading and exporting of XDatas. Or maybe I missed it?

[STiFU]

Yeah, the XDataLoader was initially called XDataManager, but then I decided to make the export() method a member of the xdata-class because I felt that is pretty convenient, which left the XDataManager just with the import-methods. Hence the renaming. But an XDataManager will be part of ReadableEditor 1.1 for sure!!

[STiFU]

Ok, once again I am trying to get the pre-releasebuilds done but just like before, the editor crashes on certain operations. Those are selecting an XData definition in the Selector and opening the XdFileChooser or selecting something in the XdFileChooser. Can't tell for sure, because when opening it, something gets selected automatically.

 

I could locate the problem a little further. The crash occurs only on the copy I use for creating the installer, which is completely isolated from my working copy of the SVN. It works perfectly when I start DR in my SVN folder through the explorer. So what could be the cause of this problem? A friend told me I should check the working folder in the project settings, which is empty, like on every other project. So that can't be it. The output-folder is alright as well. In the copy_install_files script I just changed some paths, so that should be alright too. I am pretty clueless what to do here.

 

I also tried copying the whole install folder to another location, but it's still the same issue...

 

Edit: Woops. Found it!! Compatibility mode breaks it. I was used to enabling it because desktop composition wouldn't turn off without it... But it works now.

[STiFU]

Question about the VFS: Does it notice changes in the filestructure, e.g. a new xd file has been created while DR is running?

[STiFU]

I just packed the editing pane and the guiview into a paned. Any idea what needs to be done to get gtk_paned_set_position() to work? It always sets the position to 0. Hard to find decent information about this on the web...

[greebo]

I couldn't post any replies yesterday.

 

Question about the VFS: Does it notice changes in the filestructure, e.g. a new xd file has been created while DR is running?

The VFS is using boost filesystem iterators to traverse "actual" directories (not in a PK4) and this is not cached as far as I know, so this should be a "yes".

[STiFU]

Ok, thanks. The reason I am asking about the paned is that I'd like to have the guiView acquire minimum space on the ReadableEditorDialog initially. The guiview, as well as the controls have shrinking disabled, so moving the gutter to the farthest right position would result in the perfect initial layout and rescaling maintains this layout, which is just what I wanted the preview to behave like. gtk_paned_set_position() however doesn't affect the position of the gutter. Maybe I have to set some flag to enable this??

 

If I add the guiView with expand disabled we get that perfect initial layout, but then the rescaling doesn't work as we want anymore, because the gutter is fixed in it's position and not moving relative to the scale of the window. The guiView can then only be scaled by the user by dragging the gutter. I also tried adjusting the expand-property on child2 after adding the guiView with expand disabled, so that we get the cool layout, but can still rescale properly. Apparently I didn't succeed. Tried accessing the property directly by casting the child to GtkPaned and also tried to use gtk_container_child_set_property(), but I didn't know how to deal with those GValue pointers. The reference-documentation regarding GtkPaned and this method is very thin, so it was quite a bit of failed experimenting on my side, which I eventually gave up on. It was just too frustrating for the moment...

 

I also installed glade to see how that deals with GtkPaned. That programm is missing a C exporter though, so it was of virtually no use to me.

[greebo]

Welcome to GTK, is all I can say. Most things are easy, but some other stuff is really tiresome to achieve. I wouldn't waste too much time on tweaking the GUI wrestling with the GTK API.

[STiFU]

Fidcal and Jdude gave me some feedback about the editor. It tested ok, despite some minor issues, I already fixed. One being that the dialogs weren't toplevel anymore after alt+tab, which I fixed by adding proper parents to all dialogs.

 

If you're happy with all the changes I recently made, I could wrap it all up and get it ready for release. Those changes are especially:

• Making the editor window maximizable (Requested by Fidcal). I'd prefer if there was no minimize-button, but I couldn't figure out how to hide that one, while maintaining visibility of the maximize-button.
  
• Added a "Save & Close"-Button.
  
• Added the XData storage path to the ReadableEditorDialog title.
  

 

I'll also write a wiki-entry then...

 

Further questions:

• Are we going to call it 1.1.1 now or 1.2? Bugtracker says there is still another feature missing for 1.2 .
  
• Should I mark the Readable Editor as resolved once the release is done? I mean, basically it's all still WIP.
  

[greebo]

Are we going to call it 1.1.1 now or 1.2? Bugtracker says there is still another feature missing for 1.2 .

It will be 1.2.0 as the readable editor is a large feature which justifies the minor version bump.

 

I moved the I18N issue to 1.3.0 - it won't make it into this release, the readable editor is enough for this version.

 

Should I mark the Readable Editor as resolved once the release is done? I mean, basically it's all still WIP.

Fine with me if all your immediate ToDos are resolved and a first wave of internal user feedback is considered. Further bug reports will be reported as separate tracker entries anyway.

[STiFU]

I am currently putting all the pieces together how I am going to refactor the xdata classes. I still have one remaining question. Is the "pageN_" prefix hardcoded? To clarify my question: Say, I created a gui with two text window defs called "foo" and "bar". If I wanted to define multiple pages on that gui, would I just define "page1_foo", "page2_foo", etc. ? And would an XData definition that only defines those keys without the page prefix also be compatible with the gui?

[greebo]

The pageN_ prefix is hardcoded, yes. See scripts/tdm_readables.script, method darkmod_readable_base::setupPageData. It's always "page" + pageNumber + "_" which is requested by the script code. Btw, if the mapper defines a "page1_body" on the entity itself it will be overriding the xdata definition, that's by design to allow an xdata-free map-only setup but I don't know whether anybody is using that.

[STiFU]

OK, nice. My XData data structure will then basically remain as it is, but instead of those StringLists for the page contents, I'll use a map<string, vector<string>>, with the vector holding the mapped value of a key value of different pages. Or even better, I'll use a map<size_t, string> for the vector. This way, I won't have to deal with acquiring memory myself. The old importer was very unclean and uncomfortable in this aspect.

 

By the way, I just realized that with the current implementation, we can only import 9 pages maximum. I'll change that now. This is way too limiting for a release, although I have a hardcoded maximum of 20 pages anyway.

[STiFU]

Now that I think of it map<size_t, string> probably isn't such a good idea for the page-contents, because I need a datastructure that allows easy shifting of contents, e.g. when I delete or insert a page. A map is not so suitable here, because the key-value is const. I guess I'll stick to vectors then and raise vector sizes upon request...