feat: implement dynamic key buffer system

[quarterstar]

This PR implements a new dynamic keybinding system that makes the controller class more flexible. Specifically, its primary purpose is to fill the needs of issue #41 and lay down the road for implementing custom keybinds in the future.

The PR implements an action-based system. However, this only addresses the shortcuts defined in the controller itself. For the other hard-coded keybinds in the tool files, further adjustment will be needed.

Currently, the shortcuts implemented are Space + Left Click for using the move tool and Right Click for using the erase tool. The implementation is not finished and I am requesting comments before I proceed further.

[Prayag2]

Could you please explain how this action based system works in detail?

[quarterstar]

@Prayag2, an action is essentially a task that is executed when one or a combination of keyboard shortcuts is pressed. For one task, there can be many shortcuts. The task class only encapsulates the execution logic and the keyboard shortcuts are decoupled from it. You can see this from the Facade class that is a container for both the data of the task (ActionData) and the task executor itself (classes that implement IActionTask interface).

The TaskContext is global state that is passed to all task executors. Conversely, ActionState is unique to every task and is stored in the Facade class. The reason it is not stored inside ActionData is because that is only meant to be used for data imported from JSON, which is described below.

This system is meant to allow Drawy to store keyboard shortcut data in a JSON format like this:

[
  {
    "action": "Use Tool", // The ID of the action type in the code
    "actionParameters": {
      "tool": "Move" // The name of the tool to be used
    },
    "keys": [
      {
        "id": 1234, // Qt input tokens (mouse or keyboard)
        "type": "Mouse",
        "count": 1 // Must be >= 1 (fallbacks to 1)
      }
    ],
    "holdRequired": false, // Whether to stop using the tool on key release
    "maxGapMs": 50 // The time threshold in which the keys need to be pressed, if they are not pressed in their defined order; the threshold time the other keys need to be pressed in (0 means they need to be pressed in-order, -1 means infinite threshold)
  }
]

You can see the default actions that are created in the getDefaultActions function. One change I need to make, however, is make the shortcuts stored in ActionData be able to store multiple separate key combinations. For example, we might want to have both Middle Click and Space + Left Click as acceptable default shortcuts for using the move tool temporarily. This is now implemented; refer to getDefaultActions, where you can see Middle Click and Space + Left Click being synonymous for a single action.

Actions are just not limited to tools. I have made the code abstract so that we can define actions for other things that we may need in the future.

The main issue with this implementation, as I mentioned, is that the shortcuts inside the tools themselves remain hard-coded. I have only decoupled the logic for the shortcuts in the controller class, and I need further advice for how we can make the system cover those as well, and if it needs to do so.

[Prayag2]

Please avoid goto statements whenever possible. They make code harder to read and can easily be avoided.

[Prayag2]

Use better variable names! For example, what is s and what's have? Use descriptive variable names, even if they are long. For example, eventSize instead of n, eventIndex instead of s. Even long variable names are fine if they're descriptive, e.g., requiredCountOfKeyTokens. Instead of using a uint64_t directly to represent a token, you can alias it to a more meaningful name like KeyToken.

[Prayag2]

You should also split the function into smaller parts, one for the ordered check, one for timed check, and other checks if you have them.

[Prayag2]

The class ApplicationContext was recently made a singleton, which means only one instance of it exists in the app. You don't have to pass its pointers anymore. Just access it by using ApplicationContext::instance().

[Prayag2]

Don't use the I prefix, it's not a convention I follow here.

[Prayag2]

Hi @quarterstar, there are a few things that need to be taken care of before you continue. Even though the project does not require a dynamic buffer system at the moment, you can proceed with adding it.

A few points that were not mentioned in my reviews:

• Add Doxygen style comments to every function, method, class and struct you introduce. Include a brief description along with details for all parameters and the return type. I will apply the same documentation style to the rest of the code base.
• The name Action is already used by another class. Creating a second class with the same name will create confusion. We should discuss how to combine the two since they have similar goals.
• Before you continue with the implementation, I need a clear explanation of the approach you plan to take. The keybinding system works differently and this should not be merged with the KeybindManager. A configuration system to load settings from a file can be added later in a separate PR.

Everything else looks good. Thank you for contributing.

[quarterstar]

Hi @Prayag2, yesterday I received an email which said my Discord account disabled, for seemingly no apparent reason. I'm not sure what caused it but I suspect that it was my VPN or unusual browser activity being flagged. I will look into reinstating my account, but until then I will keep you informed with these comments about my progress.

[Prayag2]

Hi @quarterstar,
This project is moving to https://invent.kde.org/Prayag/drawy. Please fork that repo instead of this one and make a new pull request there as these changes can't be pushed there. Thanks :)