Shortcut for current tool now switches to previous tool by bjorn · Pull Request #4280 · mapeditor/tiled (original) (raw)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements a toggle behavior for tool shortcuts, allowing users to switch back to the previously selected tool by pressing the same shortcut again. Additionally, it modernizes the codebase by replacing explicit type declarations with auto and simplifying the destructor.

Added mPreviousSelectedTool member to track the last selected tool
Implemented toggle behavior in shortcut handling to switch between current and previous tools
Modernized code style by using auto for type inference and defaulted destructor

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/tiled/toolmanager.h	Added mPreviousSelectedTool member variable to store the previously selected tool
src/tiled/toolmanager.cpp	Implemented toggle behavior in createShortcuts(), updated setSelectedTool() to track previous tool, added cleanup in unregisterTool(), and modernized code style with auto

Comments suppressed due to low confidence (1)

src/tiled/toolmanager.cpp:164

The early return in selectTool() prevents mPreviousSelectedTool from being updated when the same tool is selected again. This means that if selectTool(toolA) is called while toolA is already selected (e.g., through the toggle behavior), the previous tool won't be preserved correctly, breaking subsequent toggles. Consider removing this early return or updating the logic in setSelectedTool() to handle this case.

    if (mSelectedTool == tool)
        return true;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.