Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

New Input Map Editor and Editor Settings Shortcut Editor #44181

Merged
merged 2 commits into from
Feb 19, 2021
Merged

New Input Map Editor and Editor Settings Shortcut Editor #44181

merged 2 commits into from
Feb 19, 2021

Conversation

EricEzaM
Copy link
Contributor

@EricEzaM EricEzaM commented Dec 8, 2020 •
edited
Loading

This PR adds UI to expose #43662 to the user interface, and it is partially dependent on #43663 getting merged, and #43663 should be merged first.

The first 2 commits of this PR are from #43663, as this UI relies on some of those changes. If you are reviewing the code of this PR, please ignore the first 2 commits.

Features

New Action Editor

  1. Actions can now be filtered. (Closes Search and filter input map actions in project settings godot-proposals#203, Closes Cannot search for an action in the project setting input_map #26595)
  2. Built-in actions can be hidden by toggling the checkbox (Closes Add ability to remove default input map godot-proposals#303)
  3. Built-in actions have the icon next to them to show that they are part of the built-in actions
  4. Actions and events can be reordered by drag and drop (Closes Input map action reordering #16833)

image
Closes #44155

New Event Configuration Editor
Closes #44461
Keys can be listened for as before, and then manually edited as well.
event_config_listen

Keys and buttons can also be manually selected from a list.
image

New Editor Shortcuts Editor with support for editing built-in actions

You can now edit built in actions from the shortcut editor.
Closes the below once #43663 is merged.
Closes #17927
Closes #12164
Closes #42139
Closes godotengine/godot-proposals#1532

editor_actions_editor

This PR is affected by bugs:
#44178
#44180

@EricEzaM EricEzaM changed the title Pr/inp5 new input editor New Input Map Editor and Editor Settings Shortcut Editor Dec 8, 2020
@EricEzaM EricEzaM force-pushed the PR/INP5-new-input-editor branch 2 times, most recently from ca02b90 to 2bd3ff2 Compare December 8, 2020 07:14
@Chaosus Chaosus added this to the 4.0 milestone Dec 16, 2020
@EricEzaM EricEzaM mentioned this pull request Dec 18, 2020
Closed
@EricEzaM
Copy link
Contributor Author

Rebased

groud
groud approved these changes Dec 20, 2020
View reviewed changes
Copy link
Member

@groud groud left a comment

Choose a reason for hiding this comment

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

Looks good overall to me. The UX seems better, and the code looks clean to me.
My only concern stay about what is the "Store command" checkbox about. I think the tooltip is not really clear about what is this useful for. I believe it requires more information.

@EricEzaM EricEzaM closed this Dec 20, 2020
@EricEzaM EricEzaM reopened this Dec 20, 2020
@EricEzaM
Copy link
Contributor Author

Oops, bad close.

Yeah groud I will just update the tool tip to use the documentation wording. Actually I am wondering if I could just use DocData directly so that 2 place don't have to be updated if it changes?

@BluesM18A1
Copy link

Nice! The input map as it currently stands in Godot has been one of my biggest gripes with the engine, but this takes care of a lot of the clutter. I'll be happy to see these in upcoming releases.

@akien-mga
Copy link
Member

Needs a rebase, and then I think we can finally merge it and have this huge chunk of work completed.

@EricEzaM
Copy link
Contributor Author

@akien-mga Since the first 2 commits of this PR are just from #43663 I am waiting on that to be merged first

@FeatherAntennae
Copy link

I'm not sure about the choice of icon for the built-in.
This looks like a download icon and could lead to confusion (is it part of an add-on, etc.)
I suggest code brackets or gears to indicate a built-in functionality.

Otherwise, this is a pretty neat improvement!

@EricEzaM EricEzaM requested review from a team as code owners February 18, 2021 23:28
@EricEzaM
Copy link
Contributor Author

Rebased. All seems well.

Icon changed to pin, which might suit better.

image

@EricEzaM EricEzaM force-pushed the PR/INP5-new-input-editor branch 2 times, most recently from c7e08f2 to 7a867cc Compare February 19, 2021 07:39
@akien-mga
Copy link
Member

You included some unrelated changes in your latest rebase:
Screenshot_20210219_100518

@EricEzaM
New ActionMapEditor to replace InputMapEditor. Used in ProjectSettings.
3db45ff 
Renamed to ActionMapEditor as it is more generic and can be used for more than just the InputMapEditor if required.
This also includes a new Event Configuration dialog (previously "Press A key...") which can be used to create and edit InputEvents for any use - like the Project Settings input map, or the Editor Settings shortcuts.
@EricEzaM
Added Built-in Action editor to Editor Settings dialog.
8d9256e 
Built-in actions can now be edited for the Editor too.
Also added usage of the new Event confifiguration dialog to for better UX.
@EricEzaM
Copy link
Contributor Author

Oopsie, fixed.

akien-mga
akien-mga reviewed Feb 19, 2021
View reviewed changes
scene/main/window.cpp
Comment on lines -884 to -886
if (Engine::get_singleton()->is_editor_hint() && (Object::cast_to<InputEventJoypadButton>(p_ev.ptr()) || Object::cast_to<InputEventJoypadMotion>(*p_ev))) {
return; //avoid joy input on editor
}
Copy link
Member

Choose a reason for hiding this comment

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

There's potential for some issues with this, as gamepad input is sent to all applications by the OS, so when you're testing your game with a gamepad, you might also end up changing stuff by mistake in the editor since default ui_* actions handle gamepad too.

But I guess let's merge and test to see what happens?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I guess at the end of the day the current "select a button/axis from a menu" is there anyway, so reverting this shouldn't be an issue. It is very nice to have but if it causes too many issues in the editor then it should be reverted.

Nothing else should even be bound to joypad stuff in the editor though.

@akien-mga akien-mga merged commit 61e26d4 into godotengine:master Feb 19, 2021
@akien-mga
Copy link
Member

Thanks!

@EricEzaM
Copy link
Contributor Author

Looking forward to this being used and tested. Hopefully not too many bugs :)

@dalexeev dalexeev mentioned this pull request Feb 21, 2021
Closed
@Paulb23 Paulb23 mentioned this pull request Mar 7, 2021
Closed
@EricEzaM EricEzaM mentioned this pull request Apr 8, 2021
Open
@pouleyKetchoupp pouleyKetchoupp mentioned this pull request May 6, 2021
Closed
This was referenced Jun 21, 2021
Closed
Merged
@Calinou Calinou mentioned this pull request Sep 1, 2021
Closed
@EricEzaM EricEzaM deleted the PR/INP5-new-input-editor branch October 5, 2021 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@groud groud groud approved these changes

@akien-mga akien-mga akien-mga approved these changes

Assignees
No one assigned
Labels
bug feature proposal topic:editor topic:input usability
Projects
None yet
Milestone
4.0
6 participants
@EricEzaM @BluesM18A1 @akien-mga @FeatherAntennae @groud @Chaosus