-
Notifications
You must be signed in to change notification settings - Fork 19
refactor: Establish Graphable Concept #2037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Intriguingly, the connection types are only rarely needed. It may be worth making this a separate concept
The context is now bound to the concept
Again I guess
Again. It seems like something weird is going on here
trisyoungs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I fully admit that I need to read-up more on concepts, I get the thrust of the idea and this makes for some nice, clean, template-free code. I think that, in time and once we're used to using them, there are several other places where they can be immediately useful.
|
|
||
| // A graph node model for looking at the generators on a configuration | ||
| class GeneratorGraphModel : public GraphModel<GeneratorGraphNode> | ||
| class GeneratorGraphModel : public GraphModel<GeneratorGraphNode, CoreData *> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's worth pointing out that this additional Context will not be required in the end product since all data will be local to a given graph. The current dependency on CoreData as a "data resource" for the code is part of what stopped me in my tracks when doing the generalised custom model PR #1959.
| std::string nodeName(const GeneratorGraphNode &value); | ||
| std::string nodeTypeName(const GeneratorGraphNode &value); | ||
| std::string nodeTypeIcon(const GeneratorGraphNode &value); | ||
| void setNodeName(GeneratorGraphNode &value, std::string); | ||
| QVariant nodeGetValue(const GeneratorGraphNode &value); | ||
| bool nodeConnect(GeneratorGraphNode &source, int sourceIndex, GeneratorGraphNode &destionation, int destinationIndex); | ||
| bool nodeConnectable(const GeneratorGraphNode &source, int sourceIndex, const GeneratorGraphNode &destination, | ||
| int destinationIndex); | ||
| bool nodeDisconnect(GeneratorGraphNode &source, int sourceIndex, GeneratorGraphNode &destination, int destinationIndex); | ||
| QHash<int, QByteArray> &nodeRoleNames(Phantom<GeneratorGraphNode> proxy, QHash<int, QByteArray> &roles); | ||
| bool nodeDelete(GeneratorGraphNode &item, CoreData *coreData); | ||
| QVariant nodeData(const GeneratorGraphNode &item, int role); | ||
| bool nodeSetData(GeneratorGraphNode &item, const QVariant &value, int role); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, so these sets of "floating" functions taking the specific types to be included in the graph model are what initially confused me, but I get it now. Should / can they be namespaced or must they be defined at the most global level?
This PR merges into GraphViewModules and not develop
I'm mostly posting this PR to ensure that the rest of the team will be comfortable with the use of concepts in the module graphs. One of the biggest changes that comes out of all of this is that several templated functions have been turned into regular, overloaded functions. It also ensures at compile time that a type is capable of being displayed as a graph. This check was previously performed at link time with complaints about missing implementations of template functions.
The most interesting files to look at are:
nodeRoleNamesto work without being a template function, despite not being passed an instance of the type