greebo
-
Posts
16733 -
Joined
-
Days Won
51
Posts posted by greebo
-
-
Ok, the basic XMLRegistry system is working and committed now. The "show all light volumes button" is already using the new system.
edit: The code is now cleaned up and documented. Next step would be trying to port some of the other globals into the registry and see what problems arise or which interface functions are missing. But before that I will wait till you had a look at the code, in case I made some mistakes.
-
I'm not so keen on having two UI nodes though - is it possible to merge the nodes by adding all of the children of the new one onto the existing one? This way you could use unique XPaths rather than having to search by attribute.
No problem, the search for the attribute was just an example. The list of all children of a certain type can easily be obtained by using the right XPath query. E.g. if you search for the XPath "/darkradiant/ui/toolbar" you get a list of all toolbars that are children of the
node(s) under , regardless if there are one, two or hundred separate nodes below . In the above example this query would return a xml::NodeList with 2 Nodes. The actual structure of the in-memory XML tree is hidden from the rest of the world anyway. Unless there is a convenient method in libxml2 for actually merging the two
elements into each other, I would vote for leaving it with this. I agree that separate get and set functions would be the best way of doing this; I was a bit concerned that my original suggestion of just passing the xml::Document would give modules too much ability to screw up the registry.I wouldn't split out the name like that however - getXmlRegistryValue("/ui/state/showLightRadii") would be easier to use. I presume this was because the second text string was the name attribute to search for?
Yes, I thought it would be easier to use if the name was kept separate from the "class", where
is located. This depends on how the key/values are stored within the XML tree anyway - but thinking about this I agree that a single XPath will probably be easier to address the settings. -
Good news: I could add three child nodes from three different XML files to the
node. One of these was the node from doom3.game, and the other two contained tags, so I ended up with two tags under . The result looks like this: <darkradiant> <game type="doom3" ... patchtypes="doom3 def2doom3"> <filesystem> <shaders>...</shaders> </filesystem> <defaults></defaults> <entityInspector></entityInspector> </game> <ui> <toolbar name="standard"> <separator/> <toolbutton name="open" action="OpenMap" tooltip="Open a map file" icon="file_open.bmp"/> <toolbutton name="save" action="SaveMap" tooltip="Save the current map" icon="file_save.bmp"/> <separator/> </toolbar> </ui> <ui> <toolbar name="test"> <separator/> <toolbutton name="test" action="bla" tooltip="Open a map file" icon="file_open.bmp"/> </toolbar> </ui> </darkradiant>
This whole xml::document is stored in memory and can be assembled during DarkRadiant startup.
I could retrieve the toolbar named "test" via the following XPath: ui/toolbar[@name=test], so the concept works. Basically we can throw just any XML file into this global document and use it as registry - as long as the container structure is the same, it doesn't matter which XML file the information originally came from.
I would suggest writing functions that allow random XML files to be added with a single function call during program start, so that this unified XML structure is created.
The second part is writing a small API additional to findXPath to make value lookup and manipulation easy (the findXPath method is fine for lookups but its result still has to be interpreted). I would prefer to have two functions like getXmlRegistryValue("ui/state","show_light_radii") and setXmlRegistryValue("ui/state","show_light_radii","1"). All the safety checks could be delegated to these two functions, which would be much more convenient for the handling of a global variable.
For more complex queries like the ones in the EntityInspector the findXPath() can still be used.
Nothing of my suggestions is implemented yet, so what's your opinion on this?
-
As a first step I will try to merge two XMLNodes into one. Once I figured this out, I will write the getXmlRegistry() function (and perhaps a specialist function to directly access values) and implement it with the showAllLightRadii.
As soon as this works all the legacy CGameDescription values can be ported over to using the new one, but we can take our time with this one.
-
Ok, I will see what I can accomplish, but my time will be more limited the next week than it was in the last, so maybe this will take a bit.
-
When you were examining the render code for the lights, did you get any idea of how difficult it would be to create a function to overlay light falloff textures on their 2D volume renderings? I thought this might be a nice feature to help people line up lights without relying on the render view, but I don't know how difficult it would be.
Heh, I got the same idea yesterday
Not a high-priority feature, I'd say, but still nice to have.I think it's possible, as there are a lot of cascaded render() methods and it's just a matter of finding the right one, but I need to know the appropriate GL commands (which I need to learn anyway), before I could test anything in that direction.
-
I think this can be done rather easy with this function:
xmlNodePtr xmlNewChild (xmlNodePtr parent, xmlNsPtr ns, const xmlChar * name, const xmlChar * content)
although I haven't tested it yet.
Considering this works, how should we encapsulate this XML tree? At the moment all the game description XML is accesssed through the CGameDescription class, but this would be the wrong place for the ui.xml definitions, wouldn't it?
Should we create a new class (CRadiant, CRadiantRegistry, CRadiantDescription, whatever) and pack all of the existing info (GameDescription and EntityInspector definitions) into this new class? What do you suggest, Orbweaver?
-
Okay, I've added the solution as described in (2), so the "Show all light volumes" button is functional now (already on SVN).
I will have a look at the XML library and see if I can implement (3) as well :-)
-
The fix is now on SVN.
-
I just noticed that the tooltips are missing in newly created toolbars, fix is in progress and will be committed soon.
-
Thanks, I think I will first try to implement (2) and if I get that to work, I will have a go at merging the XML structures.
-
I'm currently working on adding the according ToggleToolButton to toggle the "show all light volumes" state.
What's the best way to define a global variable that can be accessed from both mainframe.cpp and plugins/entity/lights.cpp? I'm somehow struggling with this, I always get a linker error if I define the variable in a header file that's included by those two cpps.
Apart from that everything's working fine, it's just the global I need to query.
-
Ok, all these changes are committed now.
Any new tasks for me? Or should I pick one from the DarkRadiant ToDo-List in the Programming Forum?
-
OK, some preliminary comments in decreasing order of importance.
First of all thanks for reading through the code, I will do my best and will update the files as soon as possible.
[*]You seem to be allocating the toolbar creator on the heap with new but never deleting it. You should never do that in C++, it is a memory leak. If you only need the object for the duration of the function, allocate it on the stack as follows:toolbar::ToolbarCreator toolbarCreator(GameToolsPath_get());
This way it gets destructed automatically at the end of the scope, and doesn't cause a leak.
Yes, I suspected something like that. I will test if it is sufficient to stack-allocate the class or if this causes problems with the GtkToolbar instances (but it should not, as the GtkToolbars are pointers to GTK-allocated objects, aren't they?).
I don't know if this is a memory leak as well: I store all the created GtkToolbar* pointers in a std::map, do I have to clear that one in the destructor of the class or is this done by C++ routines automatically?
[*]UI code should be in the ui namespace, rather than an individual namespace like toolbarOk, I was unsure about that one, I will change this to ui, makes more sense anyway.
[*]Don't write functions to take const char* as arguments -- make them take const std::string& which can be implicitly created from const char* without having to code an explicit cast.Ok, so in general, I should not use const char*? I will change this, no problem.
[*]Your ToolbarCreator::ParseXml() function is taking an xml::Document by value rather than by reference, which means that the object will get copied when the function is called. This doesn't actually matter in this case because xml::Document only contains a pointer, but for larger objects this may be a problem. In fact, if I had written xml::Document properly it would actually "own" the entire XML document tree, which means that a pass-by-value would result in a complete traversal of the XML tree in order to copy it.I know about that one and I tried to change it once to xml::Document& but it somehow didn't work so I left it, knowing that it is copied into the call stack. I will look into it and iron this out.
[*]I don't think ui.xml should be in the doom3.game folder, as it is not game-specific. I would put this in the main DarkRadiant install folder.Ok, no problem, the path to the file is passed to the constructor, so this should be easy.
[*]Functions should start with lowercase letters, so that they can be distinguished easily from objects which have capital letters (i.e. ToolbarCreator::getToolbar() not GetToolBar())I obviously mixed this one up.
I will change the methods to lowercase beginnings.[*]You don't need to indent everything within a namespace -- most new code should be in a namespace and this would cause you to lose 4 characters for everything.Will change it too, no problem.
Good work in all though, this should set the stage for greater UI customisability in future. I wonder if it is possible to merge the UI and game XML trees so that you could call GlobalRadiant().getXPath() with either a "/ui/blah/bleh" or "/game/blah/bleh" from other code.Yes this should indeed be possible. At first I kept the files separate because GtkUIManager did not like the XML definitions to be encapsulated by another container (it expected the
tag to be the root tag). Now that I kicked out GtkUIManager this should be no problem, we just need to store everything within a top level container, in accordance to the XML definitions, like this: <darkradiant> <game> </game> <ui> </ui> </darkradiant>
But on the other hand, we take the risk to mix everything into one file (as you said above, the UI is not game-specific) that maybe hard to maintain or overview if it is getting larger.
And thanks again for providing the feedback!
-
The code cleanup is committed now, I performed a full compile and it finished without errors.
-
I just committed the ToolbarCreator class and some smaller changes. As for now, the standard radiant toolbar is outsourced into the ui.xml file.
@orbweaver: Could you test it on your machine if it compiles without errors? I also made a change to sconscript so I just want to make sure it compiles on Linux as well.
The cleanup of the old, unused code is still to come, I wanted to do this in a separate commit - it would be easier to revert if I break anything.
Oh, and please do step on my feet if I committed some horrible code or design sins, feedback is always appreciated
-
Heh, I finally found a way to integrate my new toggle buttons with the existing callbacks, so the toolbar is fully working now.
Next steps are testing, documenting the changes and stripping the unused code. This won't take too long, so hopefully I can commit something this evening or tomorrow.
-
Okay, it is straight-forward right to the point where these callbacks are involved. The one who constructed this maze of callbacks and callers must've been either a genius or on drugs (or both).
I found that the radiant code for toggleButtons uses deprecated GTK-functions, and I somehow did not manage to re-use them (GTK tells me that it is not possible to mix deprecated and non-deprecated functions).
So I've been wading through all those typedefs and templates and classes that all seem to call themselves for over an hour now and it's still not quite clear to me. My goodness, two weeks of C++ and now this...
For the moment I'm stuck at this point: the toolbar is displayed and all the buttons are functioning, but the toggle buttons don't work as expected (it is possible to toggle mulitple buttons at a time, but it shouldn't).
-
No, not from scratch :-). I will probably just add two functions to the existing xmlutil/Node.cpp, and the rest should be straight-forward.
-
Yes, this GtkUIManager implementation seems rather incomplete in regard to what GTK+ is usually capable of. And there is nearly no documentation or example code on the net (except for the specs on gnome.org and the Python thing), which transforms coding into a series of trial & error.
I'm writing my own XML parser now, it's easier anyway. :-) I hope that I can finish this till the weekend.
-
Ok, I was so close... all the code for loading the toolbar with GtkUIManager, assigning the actions and the right icons was done - and then I realised that GtkUIManager doesn't support ToggleButtons.
At least I could not find anything on this topic anywhere on the net. Yuck.
If it wasn't for the ToggleButtons, the solution using GtkUIManager would've been really smooth and elegant (just load the actions and connect them to the existing functions, then assign the icons), but there is no clean way to integrate the ToggleButton callbacks, except for a dirty workaround that would make the use of GtkUIManager pointless in the first place. (No wonder there is so little documentation on the use of GtkUIManager - it seems no one is using it).
So I'm afraid I will fall back to my first plan to write an own XML parser for the toolbars that can handle both ToolButtons and ToggleToolButtons.
-
I just created a brush, applied a random texture on it and played around with Ctrl-Left, Ctrl-Right, -Up and -Down and occasionally the horizontal and vertical stretches became zero, but this didn't do any harm for me. I tried to use Ctrl-Z (or Undo) till the Undo buffer was empty - but no crash.
Have I missed anything?Ok, I think I can see where the autosave problem lies. Obviously you have the "snapshot" checkbox (in Preferences>Autosave) enabled, otherwise there wouldn't be any up-to-date snapshots in your maps folder. If you have this option enabled, the xxx.autosave.map file is no longer used, as automatically saved maps go into the snapshots folder (saving the xxx.autosave.map would be kind of redundant).
Short story: if you want your map to be auto-saved, you can either use the xxx.autosave.map style or the snapshot/xxx.123.map style, not both.
-
If you call gtk_widget_show_all() on a top-level widget it will automatically show any subwidgets. This is how I make dialog components visible, hence the lack of individual gtk_widget_show() calls - you may be able to do a show_all() on the toolbar itself which should make all of the icons visible provided they are parented first.
Thanks, I found that one immediately after posting, as I thought there has to be a way around showing every single widget you create. At the moment I'm not progressing much, as my breaks are so short that I can compile just once or twice while eating till I have to leave home again.
-
I created a new thread as this deserves its own thread:
well as I said earlier about autosave; when I first created my map, it saved an mapname.autosave.map, but after that it was never updated again. Then I changed my mapname, and an autosave has never been created for it. However, in the maps\snapshot folder, I have many snapshots entitled 'rivertown.1.00.map - 2.11.map).Did you create this snapshot folder on your own or has this been automatically created by radiant? Can you give me a directory listing of this folder (Screenshot or "dir >list.txt" in the dosbox)?
As for providing a map, sure just tell me exactly what you want. I don't know how to reproduce the autosave problem... it just doesn't do it... as for texture, just hit ctrl-arrow till the texture smears. It should crash.Could you put a map with a simplistic box room plus the texture plus the material file into a pk4 and send it to me? Then tell me what steps I will have to do to reliably produce the crash. I can look into it, but I still am a bit of green regarding the texture code, so I can't give you any promises. But if the problem is properly documented, I'm confident that there will be a remedy available in the future. :-)
Combining Xml Trees
in DarkRadiant Feedback and Development
Posted
Yes, the button is currently on by default, I will change that in the next commit (this was a "leftover" from testing the globals).
I agree that the naming of the functions is to be changed to setProperty() or setValue(). I also thought about changing the Globalradiant()-interface to the XMLRegistry, so that the actual pointer to the xmlRegistry instance is returned (instead of translating the functions). This is easier and the plugins get access to the whole functionality of the class.
I will write a method that saves a specified node and all its children (e.g. "globals/state") to a file. On the next startup this can easily be loaded before the default values (duplicate nodes have their priority set in the order in which they were imported, imported first >> top priority). Unless we want the order to change, it's just the getProperty() method that has to be adapted.