search

godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.08k stars 69 forks source link

Allow plugins to register shortcuts in the editor menu #2024

Open dark-penguin opened 3 years ago

dark-penguin commented 3 years ago

Describe the project you are working on

An editor plugin (a better Git integration attempt).

Describe the problem or limitation you are having in your project

I have a shortcut that opens the plugin window (and probably other shortcuts in the future). Currently, I have to either hard-code it, or create a Settings dialog with all the interface to customize the shortcut.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Being able to register at least shortcuts and maybe also settings would make them easier to implement, and also easier for the user to find all shortcuts in one place. We already have a "Shortcuts" menu and a "Settings" in the editor; we should not be having separate "Shortcuts" and "Settings" menus for each plugin.

Also, if we get #831 that helps make addons "a part of the editor", then this would greatly improve usability and the experience that addons really feel like an optional part of the editor.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

We could have get_editor_interface().register_shortcut(myshortcut, "self.show") that adds a new shortcut to the list of editor shortcuts under a separate "My plugin" group.

The same goes for settings. We could have get_editor_interface().register_setting("Auto-add new files", TYPE_BOOL, self.autoadd) to register the plugin settings, which would then be added to the editor settings in a "My plugin" group.

If this enhancement will not be used often, can it be worked around with a few lines of script?

We can create "Settings" dialogs in each plugin, and find a way to let the user find them easily, while the user is not even intuitively aware that there are more settings than in the editor settings menu. Then we have to code a shortcut picker, which has no way to detect clashes with other registered shortcuts. This is more then "a few lines", but it's possible.

Is there a reason why this should be core and not an add-on in the asset library?

If such things were possible to implement with an addon, this wouldn't have been a problem in the first place. :)

me2beats commented 3 years ago

How could register_setting be different from EditorSettings set/add_property_info or ProjectSettings set/add_property_info ?

As for registering shortcuts, I find this useful. Afaik, there is no way to register shortcuts in the editor yet :(

dark-penguin commented 3 years ago

It's already there?! :-X I suspected the shortcuts could be there, but couldn't find anything; but then I did not expect that settings would be there, if the shortcuts are not.

So, we already have the settings - the shortcuts could be implemented in a similar way!

me2beats commented 3 years ago
  1. Should shortcut argument be an InputEventKey? 
    register_shortcut(shortcut: InputEventKey, obj:Object, method:String)`
  2. May method get parameters? 
    register_shortcut(shortcut: InputEventKey, obj:Object, method:String, args:Array = [])`

  3. Also, it seems some other API methods are needed like has_shortcut, remove_shortcut etc

    So It could be EditorShortcuts class that would have these methods like register_shortcut, has_shortcut, remove_shortcut etc

    EditorShortcuts get_editor_interface().get_editor_shortcuts()
dark-penguin commented 3 years ago

Currently, I have to do it like this:

    # CTRL + K
    var key = InputEventKey.new()
    key.scancode = KEY_K
    key.control = true

    var sh = ShortCut.new()
    sh.shortcut = key
    self.shortcut = sh

I don't really understand why do we need a Shortcut class - it could have just been InputEvent. But if this is how you do it, then it should probably be a Shortcut.

Now that I think about it, there is a simple way to register shortcuts for anything that can currently have shortcuts (Buttons and PopupMenu items). Shortcut.set_shortcut could also register the shortcut in the settings, but only if it has a description specified:

    var sh = ShortCut.new()
    sh.shortcut = key
    sh.description = "Commit" # If this is not set, the shortcut is not registered in the editor
    self.shortcut = sh

This is actually what I originally wanted, so the "I have a problem that needs a solution" part ends here.

Now, it would be pretty cool to be able to register a shortcut that calls any method using the existing signal system. However:

AlexDarigan commented 3 years ago

I would really appreciate a way to add a shortcut option to something like the InputMap so I can allow users of my plugin tools to set their own shortcut key combinations. Currently I've set one on a button but it requires users to mess with the internals of the tools I give them which can lead to its own obvious problems.

me2beats commented 3 years ago

Btw actions can be added to InputMap from tool scripts and plugins already. But

dark-penguin commented 3 years ago

Hm, indeed, I forgot that we have InputMap for games. But it is unsuitable for tool scripts for the above reasons. So... would it make more sense to modify the behavior of InputMap for addons, or is that too unintuitive, and tools are better off just registering shortcuts?..

me2beats commented 3 years ago

I have revised my views (see my first comment). Here are my thoughts at the moment:

  1. I'd like the user to be able to flexibly, conveniently and intuitively customize the shortcuts that are used in my plugins.

(same as implemented in Shortcuts and InputMap) Be able to

I also find it useful to have an API for working with PluginShortcuts from code:

Not sure if there is a more reliable and convenient plugin ID than the plugin name or plugin path.

Also here I have doubts which type of shortcut argument would be preferable: shortcut: Shortcut orshortcut: InputEvent (in this case, perhaps PluginInputMap would be a better name than PluginShortcuts).

  1. I'm not sure what these settings should be in at the Editor level (in EditorSettings), because this will mean that these are settings for all projects.

I think it is wiser to have these settings at the project level (and therefore in ProjectSettings — maybe along with InputMap or even in PluginList window), even though the plugin user might not be comfortable setting up shortcuts every time a new project is created (because I think this is more of a job for [local] Project Templates than for kinda Global Plugins).

  1. Since the implementation of such functionality is not an easy and most likely slow task, maybe we could use InputMap as a work-round, but in this case, the displaying of actions added from plugins in InputMap window, their editing by the user and their saving should be added (see my second comment).
dark-penguin commented 3 years ago

If we get #831 , "editor addons" will be "potentially for all projects". But there are also "normal addons", which are a part of your project - like what we have now. Since they are a part of your project, their shortcuts should be stored in the project, but for global addons, it's certainly better to store them in the editor settings. I guess that will come with their implementation.

Not sure if there is a more reliable and convenient plugin ID than the plugin name or plugin path.

I think the plugin name is good enough - I would even suggest that you don't have to pass it; instead, it's automatically derived from your plugin. You should probably not be able to assign a shortcut for a different plugin, and this way, the user can always be sure it's a setting from this plugin, even if they fork it under a different name.

I like this implementation - I think it's even better than what I had in mind. Additionally, since we also have "shortcuts" for Buttons and PopupMenus that work differently, maybe giving them a capability to easily auto-register those shortcuts would be an easy-to-implement nice-to-have?..

AlexDarigan commented 3 years ago

Shortcuts are a TYPE_OBJECT so you can actually register them directly in the project settings image

it can get messy because there's no way to really fold them up once defined so they take up a lot of space. Regardless for now you can set them here and then have the relevant keys access them somehow.

me2beats commented 3 years ago

@CodeDarigan This is an option but it gives UI with many settings instead of (more convenient) "Press Any Key" popup.

You need to type scancode number which is inconvenient.

Another sad thing is that plugin developers will create different ProjectSettings categories and this would be messy too. A solution would be Godot to provide one place for plugins Shortcuts/InputEvents/Actions.

Xananax commented 1 year ago

My current solution:

  1. create a Shortcut resource called default_shortcut.tres in the plugin's directory. Set it up through the inspector.
  2. add this to the plugin.gd source:
func _input(event: InputEvent) -> void:
    if shortcut and shortcut.matches_event(event):
        print("wowie") # Do whatever you want here

var shortcut := _get_or_set_shortcut()

const PLUGIN_ID := "my_plugin"
const PLUGIN_PATH := "plugins/"+PLUGIN_ID

static func _get_or_set_shortcut() -> Shortcut:
    var key := PLUGIN_PATH + "/shortcut"
    if not ProjectSettings.has_setting(key):
        var default_shortcut := preload("./default_shortcut.tres")
        var property_info := {
            "name": key,
            "type": TYPE_OBJECT,
            "hint": PROPERTY_HINT_RESOURCE_TYPE,
            "hint_string": "Shortcut"
        }
        ProjectSettings.set_setting(key, default_shortcut)
        ProjectSettings.add_property_info(property_info)
    var loaded_shortcut: Shortcut = ProjectSettings.get_setting(key)
    return loaded_shortcut

note: make sure to tick the "advanced settings" switch of the project settings dialog.

This solution, while not perfect, allows consumers of the plugin to change the key easily by directly editing the tres file in the editor. If they did not want to touch the source at all, they have the opportunity to specify a different shortcut resource file.

It can also be adapted to use multiple shortcuts easily if one wanted to.

Edit: Spoken too early, in Beta 11 (and 9), doing this makes the project crash with the message:

ERROR: No loader found for resource: res://../default_shortcut.tres.
ERROR: Error parsing /.../project.godot at line 22: Can't load resource at path: 'res://addons/my_addon/default_shortcut.tres'. File might be corrupted.

If I edit project.godot manually and remove the keys, then the project loads. The file isn't corrupt. Do anyone know if this is reported anywhere? Or should I file a bug report? An initial search brought up no results.

In the meantime, here's a workaround to the workaround:

static func _get_or_set_shortcut() -> Shortcut:
    var key := PLUGIN_PATH + "/shortcut"
    if not ProjectSettings.has_setting(key):
        var default_shortcut := preload("./default_shortcut.tres")
        var path := default_shortcut.resource_path
        var property_info := {
            "name": key,
            "type": TYPE_STRING,
            "hint": PROPERTY_HINT_FILE,
            "hint_string": "*.tres" 
        }
        # store path instead of resource itself
        ProjectSettings.set_setting(key, path)
        ProjectSettings.add_property_info(property_info)
    var shortcut_path: String = ProjectSettings.get_setting(key)
    if shortcut_path == "":
        return null
    var loaded_shortcut: Shortcut = load(shortcut_path)
    return loaded_shortcut
Herbherth commented 2 weeks ago

If I edit project.godot manually and remove the keys, then the project loads. The file isn't corrupt. Do anyone know if this is reported anywhere? Or should I file a bug report? An initial search brought up no results.

I've been playing around with your solution, and I don't know if what you found was indeed a bug or you just forgot to deactivate the plugin. Anyway, here is my workaround based on your workaround:

Tested with Godot 4, 4.2.2, and 4.3.beta2 and worked in all of them:

# You can create as many shortcuts as you wish
var shortcut: Shortcut
# Path the user will have to do on Project -> Project Settings -> ...
const PLUGIN_PATH := "plugins/my_plugin/shortcut" 
# Remember to create the Shortcut Resource and name it correctly bellow
var shortcut_res: Shortcut = preload("res://addons/myplugin/default_shortcut.tres")

func _enter_tree():
    # On startup or activation, set shortcut
    shortcut = set_shortcut(PLUGIN_PATH, shortcut_res)

func _exit_tree():
    # On exit or deactivation, remove shortcut and ProjectSetting path
    shortcut = null
    ProjectSettings.clear(PLUGIN_PATH)
    ProjectSettings.save()

# Handles shortcuts
func _shortcut_input(event: InputEvent) -> void:
    # Will only be called once when pressed
    if not event.is_pressed() or event.is_echo(): return
    if shortcut.matches_event(event):
        print("wowie") # Do whatever you want here

func set_shortcut(project_setting_path: String, resource: Shortcut) -> Shortcut:
    ProjectSettings.set_setting(PLUGIN_PATH, resource)
    return resource

It also works with EditorSettings instead of ProjectSetting, but you will need to get the editor: var editor_setting := EditorInterface.get_editor_settings() and to remove the setting, you use editor_setting.erase(PLUGIN_PATH) instead of clear and save (no save needed here, but it is for projectsettings)

In the end, this is the final result: image image

It is way nicer than the user go to your code or search your files to change a shortcut, but it would still be better if we could have access to Editor Settings -> Shortcuts tab or add something similar on our tab to be cleaner

Edit: to be fair, if you try to open two instances of the project, since the code is not yet removed from project.godot, the resource_loader will still bug trying to infer what type of resource it is. Have @Xananax opened a bug reported about it?

It doesn't bug if you use Editor Setting instead of Project Setting though