OrbWeaver 662 Posted July 4, 2007 Report Share Posted July 4, 2007 I suggest dividing the external interface to the Model Selector into two functions -- the current one which is used for choosing a NEW model, and displays all of the extra options, and a new one which is used for choosing an EXISTING model, which displays no extra options (and could theoretically not bother expanding skins either). Obviously the two functions would need to return different types, I suggest keeping the ModelCreationSettings (or whatever it is called) for the new model function, and just returning a string model name for the existing model function. Quote DarkRadiant homepage ⋄ DarkRadiant user guide ⋄ OrbWeaver's Dark Ambients ⋄ Blender export scripts Link to post Share on other sites
mohij 0 Posted July 4, 2007 Author Report Share Posted July 4, 2007 I think that's overtuned.When it's only about hiding the options entirely a niceModelSelectorResult chooseModel(bool showOptions=true);should do the job cleaner. Quote Link to post Share on other sites
OrbWeaver 662 Posted July 4, 2007 Report Share Posted July 4, 2007 Yes, that would do just as well, provided that it was understood that the options fields in the returned object were meaningless (or just set them to false by default). Quote DarkRadiant homepage ⋄ DarkRadiant user guide ⋄ OrbWeaver's Dark Ambients ⋄ Blender export scripts Link to post Share on other sites
mohij 0 Posted July 4, 2007 Author Report Share Posted July 4, 2007 But you have to set showOptions to false first, and if you set that one to false, you should already understand that the results in the returned structure are meaningless. I think I will now add the "add clipBrush" option to the right-click-menu. Quote Link to post Share on other sites
greebo 84 Posted July 4, 2007 Report Share Posted July 4, 2007 I would vote for hiding it if the option does not make sense in this context. edit: Whoops, just saw the new posts on the new page. Quote Link to post Share on other sites
mohij 0 Posted July 4, 2007 Author Report Share Posted July 4, 2007 How can I iterate the SelectionList? The SelectionList is a template (is there more than one?), and the iterator is templated to, so I don't know how to use the iterator. Same problem with a->name. Where is the actual instance of the SelectionList? for(iterator a = SelectionList.begin(), a != SelectionList.end(), a++) { // TODO: to be replaced by inheritance-based class detection if (!string_equal_nocase(a->name, "func_static")) { nonModel = true; break; } } Quote Link to post Share on other sites
greebo 84 Posted July 4, 2007 Report Share Posted July 4, 2007 The SelectionList is a private member of the SelectionSystem, so you shouldn't be directly iterating over it. The way to traverse the current selection is to callGlobalSelectionSystem().foreachSelected(Visitor& visitor);What do you intend to do? Maybe there is already existing functionality, that I know of? Quote Link to post Share on other sites
mohij 0 Posted July 4, 2007 Author Report Share Posted July 4, 2007 I want the menu button "create MonsterClip" to work when one or more models are selected. So I want to check the SelectionList for non models and switch the button sensible/insensible. When called I'd like to create the monsterClip for every item in the SelectionList. Quote Link to post Share on other sites
greebo 84 Posted July 4, 2007 Report Share Posted July 4, 2007 I see. Best thing is to write a Visitor then:typedef std::vector<scene::Path> InstanceList; class ModelFinder : public SelectionSystem::Visitor { InstanceList& _targetList; public: ModelFinder(InstanceList& targetList) : _targetList(targetList) {} void visit(scene::Instance& instance) const { Entity* entity = Node_getEntity(instance.path().top()); if (entity != NULL && isModel(entity)) { _targetList.push_back(instance.path()); } } }; // somewhere else in the code { InstanceList modelList; ModelFinder visitor(modelList); GlobalSelectionSystem().foreachSelected(visitor); if (modelList.size() > 0) { // One or more models found } }I don't know if the above code works, I haven't compiled it. Could be that I forgot to add a "mutable" keyword in the visitor class due to that stupid "const" declaration in the abstract base class. Quote Link to post Share on other sites
mohij 0 Posted July 4, 2007 Author Report Share Posted July 4, 2007 With that visitor class the "create monsterClip" button would work even with other stuff selected, but only operate on the models. Is that intended? Quote Link to post Share on other sites
greebo 84 Posted July 4, 2007 Report Share Posted July 4, 2007 Ah, not really intended. You're right, this has to be considered of course. A boolean flag set to true in the visitor class or anything else what comes to your mind will do the trick, I guess. Quote Link to post Share on other sites
mohij 0 Posted July 4, 2007 Author Report Share Posted July 4, 2007 Yeah, that's what I tried for the last half hour.... grrrrrthat stupid const function.Now I ended up keeping a bool* and an InstanceList* in the class and changing the outside objects.... Quote Link to post Share on other sites
greebo 84 Posted July 4, 2007 Report Share Posted July 4, 2007 You can declare your bool as mutable:class ModuleFinder : public SelectionSystem::Visitor { mutable _nonModelFound; public: // bleh };This mutable keyword can be used to make members writable even in const member methods. It's of course bad style to use it (in well-designed code it shouldn't be used at all, imo), but I guess it's ok in this case. If you're feeling ambitious, you can of course change the SelectionSystem::Visitor abstract base class and remove the const keyword. I don't see why visitors need to be const while traversing the selection. Quote Link to post Share on other sites
mohij 0 Posted July 4, 2007 Author Report Share Posted July 4, 2007 I hoped you would say that *happy* Okay, another question: Models are listed under the "func_static" entity. What key do I need to check for that? Quote Link to post Share on other sites
greebo 84 Posted July 4, 2007 Report Share Posted July 4, 2007 You can recognise models like this:Instance is an EntityEntity has spawnarg "classname" == "func_static"Entity has non-empty spawnarg "name"Entity has non-empty spawnarg "model" != "name"I think the best thing would be to extend the Entity interface (defined in ientity.h) to include a virtual bool isModel() const = 0;method to check whether this entity is a model. If you don't want to dig that deep, I can add this method for you, as I'm very familiar with the various Entity classes. Quote Link to post Share on other sites
mohij 0 Posted July 4, 2007 Author Report Share Posted July 4, 2007 I'd be gratefull if you do that, since I really don't entirely know the Scenegraph yet. Especially after you said how to recognize models.An Instance can be an entity? I thought an Instance was a path with rootnode, entity and primitives (nodes).Non-empty spawnarg name? in ientity.h: STRING_CONSTANT(Name, "Entity");I obviously get some things wrong here... Quote Link to post Share on other sites
greebo 84 Posted July 4, 2007 Report Share Posted July 4, 2007 Sorry, if I confused you about the Instances. It's easy to get lost in the scenegraph, speaking from my own experience. I only recently wrapped my head about the inner workings and I'm still not entirely enlightened about a few things. The scenegraph consists of scene::Nodes, which build up a tree. In theory, each node can have one or more child nodes. The topmost node is called the scenegraph root. There are several scene::Node classes in DarkRadiant, each with a different set of properties.RootNode (represents the scenegraph root)BrushNodePatchNodeDoom3GroupNode (the most complex of these all)EclassModelNodeGenericEntityNodeLightNodeMD5ModelNodePicoModelNodeThe Doom3GroupNode, EclassModelNode, GenericEntityNode and LightNode are Entities. They can have spawnargs (key/value pairs). To recognise a node as Entity, call Node_getEntity() or Node_isEntity(). This will try to cast the INodePtr onto an Entity*. An Instance is not a path, but an instance has a path. The path is used to lookup the instance in the scenegraph, it's some sort of unique ID. Every node needs a path before it can be instantiated. (I'm just realizing, that I don't explain that very well here, so hopefully I'm not confusing you even more. Sorry about that. ) Forget the bit about STRING_CONSTANT, this is mostly legacy stuff and I'm almost sure we can remove that. It was used to implement a home-grown type-cast system similar to C++'s dynamic_cast. I will implement that function, I'll post back here when it's done. Quote Link to post Share on other sites
greebo 84 Posted July 4, 2007 Report Share Posted July 4, 2007 Ok, the latest SVN revision features the isModel() method. Call it on the Entity:Entity* entity = Node_getEntity(scene::INodePtr); if (entity->isModel()) { // blah }You'll need to recompile almost everything as many other files are depending on the ientity.h header, that's why it took me so long Btw: there is also a wiki page on the DarkRadiant scenegraph, perhaps this helps to clarify things. Quote Link to post Share on other sites
mohij 0 Posted July 4, 2007 Author Report Share Posted July 4, 2007 I use the wiki actively all the time (otherwise I would by far haven't come as far as I have). According to your explanation the scenegraph is no real node structure with child/parent nodes, but a derivation structure with a base class and derived subclasses. Is that correct? You implemented that quite fast 8-|I hope the sync won't require many manual merges... Quote Link to post Share on other sites
greebo 84 Posted July 4, 2007 Report Share Posted July 4, 2007 No, the scenegraph is indeed a tree-like structure of nodes. It's the nodes themselves that are deriving from various base classes. Quote Link to post Share on other sites
mohij 0 Posted July 4, 2007 Author Report Share Posted July 4, 2007 radiant/ui/ortho/OrthoContextMenu.cpp: In member function 'void ui::OrthoContextMenu::checkMonsterClip()': radiant/ui/ortho/OrthoContextMenu.cpp:154: error: no matching function for call to 'SelectionSystem::foreachSelected(ui::ModelFinder (&)())' include/iselection.h:143: note: candidates are: virtual void SelectionSystem::foreachSelected(const SelectionSystem::Visitor&) const radiant/ui/ortho/OrthoContextMenu.cpp:157: error: request for member 'modelList' in 'ui::visitor', which is of non-class type 'ui::ModelFinder ()()' radiant/ui/ortho/OrthoContextMenu.cpp:157: error: request for member 'onlyModels' in 'ui::visitor', which is of non-class type 'ui::ModelFinder ()()'I get this error message that says that my visitor is no class.typedef std::vector<scene::Path> InstanceList; class ModelFinder : public SelectionSystem::Visitor { InstanceList targetList; bool onlyModels; public: ModelFinder() : onlyModels(true) {} void visit(scene::Instance& instance) { Entity* entity = Node_getEntity(instance.path().top()); if (entity->isModel()) { targetList.push_back(instance.path()); } else { onlyModels = false; }}};But this is obviously a class. For completeness here the lines that cause the error:ModelFinder visitor(); GlobalSelectionSystem().foreachSelected(visitor);I have really no idea what the reason is, as this is really just a normal class. Any ideas? Quote Link to post Share on other sites
OrbWeaver 662 Posted July 4, 2007 Report Share Posted July 4, 2007 It needs to be ModelFinder visitor; not ModelFinder visitor(); The second version is actually a forward declaration for a function finder which accepts no arguments and returns a ModelFinder. Quote DarkRadiant homepage ⋄ DarkRadiant user guide ⋄ OrbWeaver's Dark Ambients ⋄ Blender export scripts Link to post Share on other sites
mohij 0 Posted July 5, 2007 Author Report Share Posted July 5, 2007 After changing this Visitor::visitor from const to nonconst, a LOT of dependencies need to be changed too. At some point (after changing a lot of functions from const to non const) gcc spits this outradiant/brush/BrushVisit.h: In function 'const Functor& Scene_forEachSelectedBrush(const Functor&) [with Functor = BrushForEachFace]': radiant/brush/BrushVisit.h:162: instantiated from 'const Functor& Scene_ForEachSelectedBrush_ForEachFace(scene::Graph&, const Functor&) [with Functor = FaceSetShader]' radiant/brushmanip.cpp:385: instantiated from here radiant/brush/BrushVisit.h:25: error: no matching function for call to 'SelectionSystem::foreachSelected(BrushSelectedVisitor<BrushForEachFace>)' include/iselection.h:143: note: candidates are: virtual void SelectionSystem::foreachSelected(SelectionSystem::Visitor&) constBut these functions seem to comply.I did a lot trial-and-error on the functions in those files but at some point gcc always spits out a similar message. I start to wonder if it's a good idea to make the Visitor::visit nonconst...If someone wants to take a look, I uploaded a diff: monsterClipDiff.diffSome hint what's wrong would be helpful. Quote Link to post Share on other sites
greebo 84 Posted July 5, 2007 Report Share Posted July 5, 2007 At this point I'd rather suggest making the boolean mutable, perhaps I will look at making the visitor non-const later this day. Quote Link to post Share on other sites
mohij 0 Posted July 5, 2007 Author Report Share Posted July 5, 2007 So shall I revert all my changes in respect to the nonconsting? Quote Link to post Share on other sites
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.