enigma-dev/RadialGM

Protocol Model

RobertBColton opened this issue · 1 comments

It's important that we get this worked out sooner rather than later, and I'd prefer the work to be done starting at the head of the grpc branch.

How things work now

Every editor is constructed with a proto model and a single mapper that can only read and write from the resource associated with that editor:

T *EditorFactory(ProtoModel *model, QWidget *parent) {

This allows editor construction to be generic and a map of the model for each resource (with its tree node as the key) to be held weakly by the main window.

The problem with the current approach

The main window currently has a crash from the destruction of the QHash happening before the QObject::destroyed signal is received by the mdi subwindows:

subWindow->connect(subWindow, &QObject::destroyed, [=]() {

Some people feel this is a mistake with the Qt documentation which explains as though the ::destroyed signal is processed before the destructor, when it's actually the opposite. So the QHash is gone when we get the signal and we can't remove it from the map then.

Not only is this a lot of pointer madness, it doesn't even allow a potential Room editor to access Object or Sprite data. Clearly we can just overload the constructor to the room editor, but then there would go our generic editor construction.

What we should do instead

Let's combine all of the individual proto models into a table-like model and hold a unique_ptr to one instance of it in the main window. Not only is this a lot less nonsensical, and safer, it allows editors to refer to other resources without having to overload their constructors.

Specifically, the editor factory should be changed to:

T *EditorFactory(ProtoModel *model, QModelIndex &modelIndex, QWidget *parent) { 

All editor constructors should be changed to:

BaseEditor::BaseEditor(ProtoModel* model, QModelIndex &modelIndex, QWidget* parent)
: QWidget(parent), model(model), modelIndex(modelIndex), mapper(new ImmediateDataWidgetMapper(this)) {

The ProtoModel class should have a few public helper methods with the following prototypes:

QModelIndex indexOfName(QString resourceName);
QModelIndex indexOfId(buffers::resources::TreeNode::TypeCase type, int id);

Editors have only the requirement that they should be allowed to construct additional ImmediateMapper instances if they so choose to use one for editing resources of another kind. They should still have a default one automatically for the resource of the kind they were created for. They should also be able to use the helper methods on the global protocol model passed to them in their constructor in order to find resources of another kind using the string reference from their proto.

Closing out as resolved by fundies model refactors. Though we did not end up going with the giant 3D recursive model I had in mind. It had the same pitfalls as fundies traditional MVC way in that additional proxy models would still be required to display the data differently in different views.