Reimplement disabled Delete key features

include/editor.h

@@ -96,7 +96,6 @@ public:
     void renderDivingConnections();
     void addConnection(MapConnection* connection);
     void removeConnection(MapConnection* connection);
-    void removeSelectedConnection();
     void addNewWildMonGroup(QWidget *window);
     void deleteWildMonGroup();
     void configureEncounterJSON(QWidget *);
@@ -187,6 +186,7 @@ public:
 
     bool selectingEvent = false;
 
+    void deleteSelectedEvents();
     void shouldReselectEvents();
     void scaleMapView(int);
     static void openInTextEditor(const QString &path, int lineNum = 0);

include/mainwindow.h

@@ -224,9 +224,6 @@ private slots:
     void on_actionMove_triggered();
     void on_actionMap_Shift_triggered();
 
-    void onDeleteKeyPressed();
-    void on_toolButton_deleteObject_clicked();
-
     void addNewEvent(Event::Type type);
     void tryAddEventTab(QWidget * tab);
     void displayEventTabs();

include/ui/connectionpixmapitem.h

@@ -5,6 +5,7 @@
 #include <QGraphicsPixmapItem>
 #include <QPainter>
 #include <QPointer>
+#include <QKeyEvent>
 
 class ConnectionPixmapItem : public QObject, public QGraphicsPixmapItem {
     Q_OBJECT
@@ -36,14 +37,17 @@ private:
     static const int mHeight = 16;
 
 protected:
-    QVariant itemChange(GraphicsItemChange change, const QVariant &value) override;
+    virtual QVariant itemChange(GraphicsItemChange change, const QVariant &value) override;
-    void mousePressEvent(QGraphicsSceneMouseEvent*) override;
+    virtual void mousePressEvent(QGraphicsSceneMouseEvent*) override;
-    void mouseReleaseEvent(QGraphicsSceneMouseEvent*) override;
+    virtual void mouseReleaseEvent(QGraphicsSceneMouseEvent*) override;
-    void mouseDoubleClickEvent(QGraphicsSceneMouseEvent*) override;
+    virtual void mouseDoubleClickEvent(QGraphicsSceneMouseEvent*) override;
+    virtual void keyPressEvent(QKeyEvent*) override;
+    virtual void focusInEvent(QFocusEvent*) override;
 
 signals:
     void connectionItemDoubleClicked(MapConnection*);
     void selectionChanged(bool selected);
+    void deleteRequested(MapConnection*);
 };
 
 #endif // CONNECTIONPIXMAPITEM_H

include/ui/connectionslistitem.h

@@ -34,11 +34,12 @@ private:
     unsigned actionId = 0;
 
 protected:
-    void mousePressEvent(QMouseEvent*) override;
+    virtual void mousePressEvent(QMouseEvent*) override;
+    virtual void focusInEvent(QFocusEvent*) override;
+    virtual void keyPressEvent(QKeyEvent*) override;
 
 signals:
     void selected();
-    void removed(MapConnection*);
     void openMapClicked(MapConnection*);
 
 private slots:

include/ui/graphicsview.h

@@ -34,6 +34,7 @@ signals:
 
 class Editor;
 
+// TODO: This should just be MapView. It makes map-based assumptions, and no other class inherits GraphicsView.
 class GraphicsView : public QGraphicsView
 {
 public:
@@ -44,10 +45,10 @@ public:
 //    GraphicsView_Object object;
     Editor *editor;
 protected:
-    void mousePressEvent(QMouseEvent *event);
+    virtual void mousePressEvent(QMouseEvent *event) override;
-    void mouseMoveEvent(QMouseEvent *event);
+    virtual void mouseMoveEvent(QMouseEvent *event) override;
-    void mouseReleaseEvent(QMouseEvent *event);
+    virtual void mouseReleaseEvent(QMouseEvent *event) override;
-    void moveEvent(QMoveEvent *event);
+    virtual void moveEvent(QMoveEvent *event) override;
 };
 
 //Q_DECLARE_METATYPE(GraphicsView)

include/ui/mapview.h

@@ -73,7 +73,8 @@ public:
 private:
     QMap<int, Overlay*> overlayMap;
 protected:
-    void drawForeground(QPainter *painter, const QRectF &rect);
+    virtual void drawForeground(QPainter *painter, const QRectF &rect) override;
+    virtual void keyPressEvent(QKeyEvent*) override;
 };
 
 #endif // GRAPHICSVIEW_H

src/editor.cpp

@@ -811,6 +811,9 @@ void Editor::displayConnection(MapConnection *connection) {
     connect(listItem, &ConnectionsListItem::openMapClicked, this, &Editor::openConnectedMap);
     connect(pixmapItem, &ConnectionPixmapItem::connectionItemDoubleClicked, this, &Editor::openConnectedMap);
 
+    // Pressing the delete key on a selected connection's pixmap deletes it
+    connect(pixmapItem, &ConnectionPixmapItem::deleteRequested, this, &Editor::removeConnection);
+
     // Sync the selection highlight between the list UI and the pixmap
     connect(pixmapItem, &ConnectionPixmapItem::selectionChanged, [=](bool selected) {
         listItem->setSelected(selected);
@@ -869,11 +872,6 @@ void Editor::removeConnection(MapConnection *connection) {
     this->map->editHistory.push(new MapConnectionRemove(this->map, connection));
 }
 
-void Editor::removeSelectedConnection() {
-    if (selected_connection_item)
-        removeConnection(selected_connection_item->connection);
-}
-
 void Editor::removeConnectionPixmap(MapConnection *connection) {
     if (!connection)
         return;
@@ -1257,13 +1255,12 @@ void Editor::unsetMap() {
 }
 
 bool Editor::setMap(QString map_name) {
-    if (map_name.isEmpty()) {
+    if (!project || map_name.isEmpty()) {
         return false;
     }
 
     unsetMap();
 
-    if (project) {
     Map *loadedMap = project->loadMap(map_name);
     if (!loadedMap) {
         return false;
@@ -1286,13 +1283,14 @@ bool Editor::setMap(QString map_name) {
     connect(map, &Map::connectionAdded, this, &Editor::displayConnection);
     connect(map, &Map::connectionRemoved, this, &Editor::removeConnectionPixmap);
     updateSelectedEvents();
-    }
 
     return true;
 }
 
 bool Editor::setLayout(QString layoutId) {
-    if (layoutId.isEmpty()) return false;
+    if (!project || layoutId.isEmpty()) {
+        return false;
+    }
 
     this->layout = this->project->loadLayout(layoutId);
 
@@ -2224,6 +2222,50 @@ bool Editor::eventLimitReached(Event::Type event_type) {
     return false;
 }
 
+void Editor::deleteSelectedEvents() {
+    if (!this->selected_events || this->selected_events->length() == 0 || !this->map || this->editMode != EditMode::Events)
+        return;
+
+    DraggablePixmapItem *nextSelectedEvent = nullptr;
+    QList<Event *> selectedEvents;
+    int numDeleted = 0;
+    for (DraggablePixmapItem *item : *this->selected_events) {
+        Event::Group event_group = item->event->getEventGroup();
+        if (event_group != Event::Group::Heal) {
+            numDeleted++;
+            item->event->setPixmapItem(item);
+            selectedEvents.append(item->event);
+        }
+        else { // don't allow deletion of heal locations
+            logWarn(QString("Cannot delete event of type '%1'").arg(Event::eventTypeToString(item->event->getEventType())));
+        }
+    }
+    if (numDeleted) {
+        // Get the index for the event that should be selected after this event has been deleted.
+        // Select event at next smallest index when deleting a single event.
+        // If deleting multiple events, just let editor work out next selected.
+        if (numDeleted == 1) {
+            Event::Group event_group = selectedEvents[0]->getEventGroup();
+            int index = this->map->events.value(event_group).indexOf(selectedEvents[0]);
+            if (index != this->map->events.value(event_group).size() - 1)
+                index++;
+            else
+                index--;
+            Event *event = nullptr;
+            if (index >= 0)
+                event = this->map->events.value(event_group).at(index);
+            for (QGraphicsItem *child : this->events_group->childItems()) {
+                DraggablePixmapItem *event_item = static_cast<DraggablePixmapItem *>(child);
+                if (event_item->event == event) {
+                    nextSelectedEvent = event_item;
+                    break;
+                }
+            }
+        }
+        this->map->editHistory.push(new EventDelete(this, this->map, selectedEvents, nextSelectedEvent ? nextSelectedEvent->event : nullptr));
+    }
+}
+
 void Editor::openMapScripts() const {
     openInTextEditor(map->getScriptsFilePath());
 }

src/mainwindow.cpp

@@ -153,12 +153,6 @@ void MainWindow::initExtraShortcuts() {
     shortcutDuplicate_Events->setObjectName("shortcutDuplicate_Events");
     shortcutDuplicate_Events->setWhatsThis("Duplicate Selected Event(s)");
 
-    // TODO: Reimplement this using keyPressEvent on the relevant widgets. Otherwise it steals the key event from anything else trying to use delete.
-    /*auto *shortcutDelete_Object = new Shortcut(
-            {QKeySequence("Del"), QKeySequence("Backspace")}, this, SLOT(onDeleteKeyPressed()));
-    shortcutDelete_Object->setObjectName("shortcutDelete_Object");
-    shortcutDelete_Object->setWhatsThis("Delete Selected Item(s)");*/
-
     auto *shortcutToggle_Border = new Shortcut(QKeySequence(), ui->checkBox_ToggleBorder, SLOT(toggle()));
     shortcutToggle_Border->setObjectName("shortcutToggle_Border");
     shortcutToggle_Border->setWhatsThis("Toggle Border");
@@ -320,6 +314,7 @@ void MainWindow::initEditor() {
     connect(this->editor, &Editor::wildMonTableEdited, [this] { this->markMapEdited(); });
     connect(this->editor, &Editor::mapRulerStatusChanged, this, &MainWindow::onMapRulerStatusChanged);
     connect(this->editor, &Editor::tilesetUpdated, this, &Scripting::cb_TilesetUpdated);
+    connect(ui->toolButton_deleteObject, &QAbstractButton::clicked, this->editor, &Editor::deleteSelectedEvents);
 
     this->loadUserSettings();
 
@@ -857,13 +852,7 @@ bool MainWindow::userSetMap(QString map_name) {
 }
 
 bool MainWindow::setMap(QString map_name) {
-    // if map name is empty, clear & disable map ui
+    if (map_name.isEmpty() || map_name == DYNAMIC_MAP_NAME) {
-    if (map_name.isEmpty()) {
-        unsetMap();
-        return false;
-    }
-
-    if (map_name == DYNAMIC_MAP_NAME) {
         logInfo(QString("Cannot set map to '%1'").arg(DYNAMIC_MAP_NAME));
         return false;
     }
@@ -2585,60 +2574,6 @@ void MainWindow::on_horizontalSlider_CollisionTransparency_valueChanged(int valu
     this->editor->collision_item->draw(true);
 }
 
-void MainWindow::onDeleteKeyPressed() {
-    auto tab = ui->mainTabBar->currentIndex();
-    if (tab == MainTab::Events) {
-        on_toolButton_deleteObject_clicked();
-    } else if (tab == MainTab::Connections) {
-        if (editor) editor->removeSelectedConnection();
-    }
-}
-
-void MainWindow::on_toolButton_deleteObject_clicked() {
-    if (editor && editor->selected_events) {
-        if (editor->selected_events->length()) {
-            DraggablePixmapItem *nextSelectedEvent = nullptr;
-            QList<Event *> selectedEvents;
-            int numDeleted = 0;
-            for (DraggablePixmapItem *item : *editor->selected_events) {
-                Event::Group event_group = item->event->getEventGroup();
-                if (event_group != Event::Group::Heal) {
-                    numDeleted++;
-                    item->event->setPixmapItem(item);
-                    selectedEvents.append(item->event);
-                }
-                else { // don't allow deletion of heal locations
-                    logWarn(QString("Cannot delete event of type '%1'").arg(Event::eventTypeToString(item->event->getEventType())));
-                }
-            }
-            if (numDeleted) {
-                // Get the index for the event that should be selected after this event has been deleted.
-                // Select event at next smallest index when deleting a single event.
-                // If deleting multiple events, just let editor work out next selected.
-                if (numDeleted == 1) {
-                    Event::Group event_group = selectedEvents[0]->getEventGroup();
-                    int index = editor->map->events.value(event_group).indexOf(selectedEvents[0]);
-                    if (index != editor->map->events.value(event_group).size() - 1)
-                        index++;
-                    else
-                        index--;
-                    Event *event = nullptr;
-                    if (index >= 0)
-                        event = editor->map->events.value(event_group).at(index);
-                    for (QGraphicsItem *child : editor->events_group->childItems()) {
-                        DraggablePixmapItem *event_item = static_cast<DraggablePixmapItem *>(child);
-                        if (event_item->event == event) {
-                            nextSelectedEvent = event_item;
-                            break;
-                        }
-                    }
-                }
-                editor->map->editHistory.push(new EventDelete(editor, editor->map, selectedEvents, nextSelectedEvent ? nextSelectedEvent->event : nullptr));
-            }
-        }
-    }
-}
-
 void MainWindow::on_toolButton_Paint_clicked()
 {
     if (ui->mainTabBar->currentIndex() == MainTab::Map)

src/ui/connectionpixmapitem.cpp

@@ -9,6 +9,7 @@ ConnectionPixmapItem::ConnectionPixmapItem(MapConnection* connection, int x, int
       connection(connection)
 {
     this->setEditable(true);
+    setFlag(ItemIsFocusable, true);
     this->basePixmap = pixmap();
     this->setOrigin(x, y);
 }
@@ -110,17 +111,20 @@ bool ConnectionPixmapItem::getEditable() {
 }
 
 void ConnectionPixmapItem::setSelected(bool selected) {
+    if (selected && !hasFocus()) {
+        setFocus(Qt::OtherFocusReason);
+    }
+
     if (this->selected == selected)
         return;
     this->selected = selected;
+
     this->render();
     emit selectionChanged(selected);
 }
 
 void ConnectionPixmapItem::mousePressEvent(QGraphicsSceneMouseEvent *) {
-    if (!this->getEditable())
+    setFocus(Qt::MouseFocusReason);
-        return;
-    this->setSelected(true);
 }
 
 void ConnectionPixmapItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) {
@@ -131,3 +135,18 @@ void ConnectionPixmapItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) {
 void ConnectionPixmapItem::mouseDoubleClickEvent(QGraphicsSceneMouseEvent *) {
     emit connectionItemDoubleClicked(this->connection);
 }
+
+void ConnectionPixmapItem::keyPressEvent(QKeyEvent* event) {
+    if (event->key() == Qt::Key_Delete || event->key() == Qt::Key_Backspace) {
+        emit deleteRequested(this->connection);
+    } else {
+        QGraphicsPixmapItem::keyPressEvent(event);
+    }
+}
+
+void ConnectionPixmapItem::focusInEvent(QFocusEvent* event) {
+    if (!this->getEditable())
+        return;
+    this->setSelected(true);
+    QGraphicsPixmapItem::focusInEvent(event);
+}

src/ui/connectionslistitem.cpp

@@ -10,6 +10,7 @@ ConnectionsListItem::ConnectionsListItem(QWidget *parent, MapConnection * connec
     ui(new Ui::ConnectionsListItem)
 {
     ui->setupUi(this);
+    setFocusPolicy(Qt::StrongFocus);
 
     const QSignalBlocker blocker1(ui->comboBox_Direction);
     const QSignalBlocker blocker2(ui->comboBox_Map);
@@ -101,3 +102,16 @@ void ConnectionsListItem::on_button_Delete_clicked() {
 void ConnectionsListItem::on_button_OpenMap_clicked() {
     emit openMapClicked(this->connection);
 }
+
+void ConnectionsListItem::focusInEvent(QFocusEvent* event) {
+    this->setSelected(true);
+    QFrame::focusInEvent(event);
+}
+
+void ConnectionsListItem::keyPressEvent(QKeyEvent* event) {
+    if (event->key() == Qt::Key_Delete || event->key() == Qt::Key_Backspace) {
+        on_button_Delete_clicked();
+    } else {
+        QFrame::keyPressEvent(event);
+    }
+}

src/ui/graphicsview.cpp

@@ -24,6 +24,14 @@ void GraphicsView::moveEvent(QMoveEvent *event) {
         label_MapRulerStatus->move(mapToGlobal(QPoint(6, 6)));
 }
 
+void MapView::keyPressEvent(QKeyEvent *event) {
+    if (editor && (event->key() == Qt::Key_Delete || event->key() == Qt::Key_Backspace)) {
+        editor->deleteSelectedEvents();
+    } else {
+        QGraphicsView::keyPressEvent(event);
+    }
+}
+
 void MapView::drawForeground(QPainter *painter, const QRectF&) {
     for (auto i = this->overlayMap.constBegin(); i != this->overlayMap.constEnd(); i++) {
         i.value()->renderItems(painter);

src/ui/maplistmodels.cpp

@@ -404,8 +404,6 @@ bool MapGroupModel::setData(const QModelIndex &index, const QVariant &value, int
     }
 }
 
-// TODO: Deleting MAPSEC support? Currently it has no limits on drag/drop etc, so editing is disabled (so delete key from the map list is ignored)
-//       and it has no delete action in the context menu.
 
 
 MapAreaModel::MapAreaModel(Project *project, QObject *parent) : MapListModel(parent) {