Added Expand/Collapse API to GroupLayer#4009
Added Expand/Collapse API to GroupLayer#4009PalaBeaveR wants to merge 3 commits intomapeditor:masterfrom
Conversation
Group layers can now be expanded/collapsed from scripts
|
I think it would be better to allow the scripting API to interact directly with the UI. There is no need for this to go through an undo command. I also don't think we need to store that state on the There already is Maybe it could just be some additional functions like |
|
Alright I've removed the state from One thing I noticed is that for now changing group layer expanded state does not reflect in Also I got segfaults when passing the wrong layer to the |
bjorn
left a comment
bjorn
left a comment
There was a problem hiding this comment.
@PalaBeaveR Sorry it seems this one got buried for way too long. Are you still interested in finishing it? It looks like you're almost there and I've leaving some notes now.
Somebody was asking about taking this over at #3997, so alternatively you can also let them know they can proceed.
One thing I noticed is that for now changing group layer expanded state does not reflect in
ObjectsView. This is what the code does right now, but we need to figure out how to go about it, because i assume it would be better to not sync them between trees, but need some input on that.
This is fine. Since we're interacting directly with the views, it works the same way as if the user has clicked around in the UI, and expansion of group layers or object groups is not synchronized between views.
Also I got segfaults when passing the wrong layer to the
setExpandedmethod and want to know whether this should be handled or not.
That should definitely be prevented, even misuse of the scripting API should not crash Tiled if it can be prevented. At the very least you need to check the incoming pointers to see if they're not nullptr to avoid a crash, and if you can still crash it then we should have a look at how we can prevent that.
| QList<Layer*>::iterator end() { return mLayers.end(); } | ||
| QList<Layer*>::const_iterator begin() const { return mLayers.begin(); } | ||
| QList<Layer*>::const_iterator end() const { return mLayers.end(); } | ||
|
|
There was a problem hiding this comment.
Please avoid changing this file at all. I realize these are remnants of earlier changes.
| auto documentManager = DocumentManager::instance(); | ||
| auto mapEditor = static_cast<MapEditor*>(documentManager->editor(Document::MapDocumentType)); | ||
| mapEditor->layerDock()->setExpanded(groupLayer(), expanded); | ||
| } |
There was a problem hiding this comment.
Please revert your changes to editablegrouplayer.cpp/h and editableobjectgroup.cpp.h entirely. Since we decided to go with a view-based API we don't need these functions here at all.
| void setMapDocument(MapDocument *mapDocument); | ||
|
|
||
| Q_INVOKABLE bool isExpanded(EditableGroupLayer *layer) const { return isExpanded(layer->groupLayer()); }; | ||
| Q_INVOKABLE void setExpanded(EditableGroupLayer *layer, bool expanded) { setExpanded(layer->groupLayer(), expanded); }; |
There was a problem hiding this comment.
These functions need to check at the very least that layer is not a nullptr. If so, raise a script error (see other script functions for how to do this) and return. I think their implementation should be moved into the cpp file.
| void setMapDocument(MapDocument *mapDoc); | ||
|
|
||
| Q_INVOKABLE bool isExpanded(EditableObjectGroup *layer) const { return isExpanded(layer->objectGroup()); }; | ||
| Q_INVOKABLE void setExpanded(EditableObjectGroup *layer, bool expanded) { setExpanded(layer->objectGroup(), expanded); }; |
There was a problem hiding this comment.
Same here regarding nullptr checks.
|
I'll close this one now that @Oval17 has taken over at #4379. Thanks for getting this started, @PalaBeaveR! |
2 participants
Related issue: #3997
Added a new scripting field in GroupLayer called expanded that controls the layer's expand state.
From what I'd been able to gather, in order to expand/collapse layers I would need access to LayerView(QTreeView) which controls layer expansion with setExpanded. It wasn't possible since those properties were privated so I added getters to access LayerDock, LayerView, QAbstractProxyModel.
Right now the expanded state is stored in GroupLayer and is not synced, however since Qt already stores that state itself, the scripting interface can be made to use that instead.
I also don't know if the same API is appropriate for ObjectGroup(as requested by #3997), but i have that code made in the same way as GroupLayer if it is needed.
I'm not sure if the new definitions in changelayer.h/undocommands.h are needed since i was just copying code from other places in the repo so it would be nice to have that clarified.