Hi,

I´m currently developing a little menu system for a console game. As a starting point I divided the Menu in three areas: the actual data structure, a navigation class which can navigate through the "tree" and holds the state (currently active SubMenu) and a class which draws the whole thing. In its simplest form I want it to behave/look like the following:

1) Every "Node" represents a MenuItem

1.1) Each MenuItem can either be a SubMenuItem or an ActionItem

1.2) Every SubMenu can contain 0 to N childs

2) If I navigate to a SubMenuItem and it  has childs I want to display them. If I navigate to an ActionItem it should trigger a function.

As a class hirarchy to actually build the menu I thought something like this would work:

class MenuItem {
public:
  virtual void action() = 0;
  virtual string& getItemName() = 0;
}

class ActionItem : public MenuItem {
public:
  void action() override {
  	//Trigger stuff
  }
  string& getItemName() {
  	return itemName;
  }
}

class SubMenuItem : public MenuItem {
private:
  vector<MenuItem*> childItems;
public:
  void action() override {
  	//What to do?
  }
   string& getItemName() {
  	return itemName;
  }
  void add(MenuItem* m) {...}
  void rm(MenuItem* m) {...}
  MenuItem* find(const string& name) {...}
}

To create the menu I could then do this:

SubMenuItem root, options;
ActionItem exit, start, fullScreenMode, windowMode
  
options.add(&fullScreenMode);
options.add(&windowMode);

root.add(&start);
root.add(&options);
root.add(&exit);

// Should look like this
+--start
|
+--options--+--fullScreenMode
|           +--windowMode
+--exit

But now I created myself a problem:

How should I actually get the type of my MenuItem? Lets say I want to find the options SubMenu and add some ActionItems at runtime. In order to do this I can call find on the root object but then I only have a MenuItem pointer which can call action() or getItemName(). To get around this problem I could add some methods to my MenuItem interface, but I think that would be the wrong way (as you can see with the action method -> SubMenuItem has no real use for it). Another solution could be the use of dynamic_cast but wouldn´t it be a sign of a bad design?

MenuItem* options = root.find("options");
options. ???
  
//Fix?
class MenuItem {
public:
  virtual void action() = 0;
  virtual string& getItemName() = 0;
  virtual bool isSubmenu() = 0;
  virtual vector<MenuItem*> getChild() = 0;
}

MenuItem* options = root.find("options");
if(options->isSubmenu())
  options->getChild().push_back(new ActionItem());

//Or maybe this?
SubMenuItem* options = dynamic_cast<SubMenuItem*>(root.find("options"));
if(options)
  options->add(new ActionItem())

Am I on the right track or is this complete bs? 

Should I use templates (CRTP) for this kind of problem or is maybe the visitor pattern a good match for this problem?

I hope you understand my problem can can give me some hints!

Since you haven't gotten any replies yet, I'll try to get some conversation going.

I think there may sometimes be a bit of a disconnect between so-called best practices and what one tends to actually find in the field. Things like downcasts, globals/singletons, and so on are often viewed with skepticism from a theoretical perspective, yet you'll often find them used in real-life projects, popular engines/frameworks/libraries, and so on. (For example, don't hold me to this, but I think Unity's 'traditional' entity-component system uses downcasts for component access.)

Note that I'm not making an argument here - just an observation. (I do think some idioms may be worth avoiding in C++ specifically - anything that brings static initialization and/or destruction order into play is probably worth keeping an eye on.)

If you're interested in getting some more insight on downcasting, you might try searching (not just on GDNet, but via a general search engine) for e.g. 'downcast bad practice', and see what people have to say about it. For example, I found this thread:

https://stackoverflow.com/questions/30704567/why-would-down-casting-be-a-bad-practice-in-c-and-not-in-another-language

Which has some discussion you might find relevant. (There's some discussion of the performance of dynamic_cast in C++, but I don't know how much of a concern that would be for the use case in your example.)

For what it's worth, instead of doing something like this:

SubMenuItem* options = dynamic_cast<SubMenuItem*>(root.find("options"));

You can create function templates to which you submit the desired type and which do the casting for you, so that you don't have to perform the cast separately each time. That might look something like this (just typed into post, so there may be errors):

SubMenuItem* options = root.find<SubMenuItem>("options");

At this point perhaps someone else will jump in and suggest a more sophisticated approach that would avoid downcasts altogether (e.g. visitation, as you mentioned, or something of that sort). Meanwhile though, I'll suggest a super-simple approach, which is simply to store references (in the general sense - would probably be a pointer in this case) to menu nodes that you want to be able to work with later. In this case, knowing that you want to be able to add items to the options menu, you'd just store a properly typed pointer to the options menu node for that purpose. Obviously this is a little less convenient, and it may or may not be a suitable solution, depending on the circumstances, but it's a simple solution that might be worth considering.

Lastly, I see some things in your code that I'm a little curious about, such as returning name strings by non-constant reference, but getting into that would probably be off-topic here. Once you have an actual implementation put together though, that and a couple other things from your example code might be worth taking another look at.

Many thanks for you response!

23 hours ago, Zakwayda said:

Lastly, I see some things in your code that I'm a little curious about, such as returning name strings by non-constant reference, but getting into that would probably be off-topic here. Once you have an actual implementation put together though, that and a couple other things from your example code might be worth taking another look at.

I typed the code into the code tags from gamdev...this code wasnt´t meant to be run into a real world application (only for demonstrating my problem)

After some research I get the impression there is no real "solution". Either you use rtti (typeid, dynamic_cast ...) or you spin your own "manual" rtti system (or you save all concrete types into the submenu as you mentioned). The visitor pattern doesnt work for my problem either....sure I get the right type in my visitor implementations but how can I return the object(with the right type)?

I also looked into two ECS implementations (GetComponent<T>()) and they always use some sort of "marker"(id, enum, string) to identify their component in a container and cast them with static_cast. 

For the record, there was a mistake in my earlier post. This:

SubMenuItem* options = dynamic_cast<SubMenuItem*>(root.find("options

Was supposed to be:

SubMenuItem* options = dynamic_cast<SubMenuItem*>(root.find("options"));

But by the time I noticed it, the edit window had expired 

Quote

I typed the code into the code tags from gamdev...this code wasnt´t meant to be run into a real world application (only for demonstrating my problem)

Sure, understood. The reason I mentioned there being some possible issues is that with such "off the top of one's head" code, one has to guess what issues are due to the code being off the cuff, and what issues might manifest in 'real code' as well. I often see similar issues (lack of const, suspicious use of references, ownership ambiguities, and so on) in 'real' code posted here, so I figured it was at least worth mentioning.

48 minutes ago, Daniel Ricci said:

Did I drink too much coffee or did you copy paste the same two line?

Uh...that's peculiar. It seems unlikely that I would have made the exact same mistake twice in a row. I'll try it again here (if the results are the same, that would suggest maybe it's a forum error):

SubMenuItem* options = dynamic_cast<SubMenuItem*>(root.find("options"));

[Edit: It came out right that time. I don't know what happened in that last post ]

On 7/2/2019 at 11:05 PM, Geosesarma said:

How should I actually get the type of my MenuItem? Lets say I want to find the options SubMenu and add some ActionItems at runtime

Assuming you want your new actions somewhere at a known place in the existing menu structure, why not specify the full path from the root to the point of insertion?

Then obviously each intermediate node must be a submenu, which you can blindly assume to be true, or check anyway at runtime as paranoia check.

4 hours ago, Daniel Ricci said:

Did I drink too much coffee or did you copy paste the same two line?

Ok, so I pasted the same line into another post above, and initially it came out correctly. But now it's wrong again! (That is, the characters after 'options' have once again disappeared.)

Maybe it's a forum bug or idiosyncrasy.

Looks like there's a forum bug. Looking at the post and then refreshing caused it to change. Refreshing several times can cause both variations (correct + incorrect) to appear.

5 hours ago, Alberth said:

Then obviously each intermediate node must be a submenu, which you can blindly assume to be true, or check anyway at runtime as paranoia check.

You are absolutely right every intermediate node hast to be a submenu. But lets assume a Submenu S1 contains 2 Items: 1 ActionItem A2 and 1 Submenu S2. Both have the same parent (S1). But in this case S2 doesnt has any items....therefore I dont can trust the "is it an itermediate node check" or am I completly wrong?

+-- S1 -- + -- S2
          + -- A2