openplanet-nl / issues

Issue tracker for Openplanet.
10 stars 0 forks source link

Suggestion: "Actions" and built-in keybind settings #298

Open nbeerten opened 1 year ago

nbeerten commented 1 year ago

Keybinds are currently done by a plugin itself, with different locations in the settings for each plugin, and no detection of conflicting keybinds (as far as I'm aware). It would be nice to have a single built-in keybind settings page with all keybinds of all plugins. My suggestion of a way to do that would be by introducing Actions: A way for plugins to label a function as an Action, with name and such. The keybind settings page would show a list of all Actions registered by all plugins, and users can bind keys to each Action.

Each Action would be attached to a function, like a handler, and could run code when called. My suggestion was a syntax similar to this:

[Action name="Refresh Leaderboard"]
void function HandleRefreshLBAction() {}

but a few people mentioned they need to be able to register keybinds at runtime, so some sort of registering with callback kinda system may be better.

codecat commented 1 year ago

As for registering at runtime, perhaps we can simply offer both methods; the one in your example, and a callback system:

Meta::RegisterAction("Refresh Leaderboard", ...);
skybaks commented 1 year ago

As someone who has a plugin that currently supports 10+ different keybinds i would appreciate a way for my plugin to migrate the user-bound keys in my plugin settings to the new binds in the openplanet settings.

I also would like for users to be able to unbind or disable Actions from this new UI without needing to go back to my plugin settings and checking a on/off box there.

skybaks commented 1 year ago

image This is my plugins current implementation for hotkey settings. In the generic solution I think it would also make sense to have a column for the owning "Plugin" and maybe also a tooltip where a short description of the behavior could be described.

codecat commented 1 year ago

Thanks for the additional info!

I would like for this system to have 2 purposes:

I propose the following API, roughly:

// An action with:
// - Name
// - (Default) shortcut
// - Description
// - Return value whether to block keyboard input
[Action name="Toggle AirBlock Mode" shortcut="A" description="Toggles the current mode for block placement in the air"]
UI::InputBlocking ActionToggleAirblockMode() {
  if (inEditor) {
    // ...
    return UI::InputBlocking::Block;
  }
  return UI::InputBlocking::DoNothing;
}

// An action with:
// - Name
// - Void return value
[Action name="Toggle Block Helpers"]
void ActionToggleBlockHelpers() { /* ... */ }

void Main() {
  // Registering actions at runtime
  auto action = Meta::RegisterAction("My runtime action", MyRuntimeAction);

  // Setting shortcuts at runtime (maybe a separate SetDefaultShortcut?)
  action.SetShortcut("B");

  // Triggering shortcuts at runtime
  auto hideDashboard = Meta::GetAction("Dashboard", "Hide Dashboard");
  if (hideDashboard !is null) {
    hideDashboard.Trigger();
  }
}

// Just a regular function
void MyRuntimeAction() { /* ... */ }

Triggering actions from other plugins could reduce some overhead and complexity in plugin dependency logic, and also allow for more dynamically triggering stuff at runtime. For example, I imagine a "scheduler" plugin that triggers actions at specific times of day. Additionally, it could be cool to have a "shortcuts" plugin (shipped with Openplanet) do all the input handling instead of Openplanet doing this by itself.

The only immediate concern I have with this proposal is input blocking: Note that ActionToggleBlockHelpers does not have a return value but ActionToggleAirblockMode does. I fear that this difference could potentially be confusing, and also make the implementation a little bit more complex.

Curious to hear everyone's thoughts. 😊

nbeerten commented 1 year ago

@codecat I do wonder how useful a return value that blocks or does nothing is. My current understanding is that UI::InputBlocking::DoNothing makes the input pass through to the game (that's correct right?)... Which would be weird and unexpected if you explicitly set a keybind for something, because you'd expect only the action bound to the keybind to run, no matter what it returns.

skybaks commented 1 year ago

One comment about assigning the key in the attribute. I would like for users to be able to rebind these actions to any key so this should be more of a default

nbeerten commented 1 year ago

@skybaks @codecat mentioned that already in the code comments, but yeah... That's the whole idea of this suggestion, to be able to set keybinds to them and let users assign keybinds in one central place

skybaks commented 1 year ago

Regarding double binding to the same key, different plugins might only use the action trigger in certain game contexts. For instance, a race plugin might only respond to the action when the player is racing, but an editor plugin might also be bound to the same key. Not sure if there is a generic way to describe game context but maybe that could be another string passed in when the action is setup

codecat commented 1 year ago

Which would be weird and unexpected if you explicitly set a keybind for something, because you'd expect only the action bound to the keybind to run, no matter what it returns.

Which is exactly why the return value is helpful: you can return whether the action was handled (or "consumed") by the plugin or not. Perhaps we can just use a differently named enum instead of re-using the one in UI. It could be Meta::ActionResult::Handled or Meta::ActionResult::Unhandled

skybaks commented 1 year ago

"Handled" makes me think that it will stop processing more actions if the first one returns that. Would that be the behavior or are all actions guaranteed to run?

codecat commented 1 year ago

If we can't guarantee a specific order for conflicting actions, I don't think a developer should be able to assume their action is going to block other actions.