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.

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

Request for nonexistent InputMap action: ui_cancel #17210

Closed
Ranoller opened this issue Mar 3, 2018 · 12 comments
Closed

Request for nonexistent InputMap action: ui_cancel #17210

Ranoller opened this issue Mar 3, 2018 · 12 comments

Comments

@Ranoller
Copy link
Contributor

Ranoller commented Mar 3, 2018

Godot version:

Godot 3 custom

OS/device including version:

Win7

Issue description:

I delete in runtime all the custom actions and replace with my own, with InputMap.get_actions() and InputMap.erase_action()

All is working but godot shows all time a key or a button is pressed: Request for nonexistent InputMap action: ui_cancel.

I don´t want the default actions in runtime, there are noise in my input map logic. Is mandatory to have this actions in the runing game?

@bfloch
Copy link
Contributor

bfloch commented Mar 3, 2018

The ui actions are being use for stuff like GUI focus. ui_cancel in particular seems to be closing modal dialogs.
You should probably keep them around.

@groud
Copy link
Member

groud commented Mar 3, 2018

What? You can create and remove input mappings from the project settings > input tab. No need to use scripts to do so.

@akien-mga
Copy link
Member

There are various core Nodes which rely on the ui_ actions for their behaviour, so if you remove those, you break them. There's a reason why the presets can't be removed in the InputMap :)

@akien-mga
Copy link
Member

$ rg '"ui_' -g'!thirdparty'
editor/animation_editor.cpp
1868:                   if (key->is_pressed() && (key->is_action("ui_up") || key->is_action("ui_page_up"))) {
1870:                           if (key->is_action("ui_up"))
1872:                           if (v_scroll->is_visible_in_tree() && key->is_action("ui_page_up"))
1887:                   if (key->is_pressed() && (key->is_action("ui_down") || key->is_action("ui_page_down"))) {
1889:                           if (key->is_action("ui_down"))
1891:                           else if (v_scroll->is_visible_in_tree() && key->is_action("ui_page_down"))

core/input_map.cpp
221:    add_action("ui_accept");
224:    action_add_event("ui_accept", key);
228:    action_add_event("ui_accept", key);
232:    action_add_event("ui_accept", key);
234:    add_action("ui_select");
237:    action_add_event("ui_select", key);
239:    add_action("ui_cancel");
242:    action_add_event("ui_cancel", key);
244:    add_action("ui_focus_next");
247:    action_add_event("ui_focus_next", key);
249:    add_action("ui_focus_prev");
253:    action_add_event("ui_focus_prev", key);
255:    add_action("ui_left");
258:    action_add_event("ui_left", key);
260:    add_action("ui_right");
263:    action_add_event("ui_right", key);
265:    add_action("ui_up");
268:    action_add_event("ui_up", key);
270:    add_action("ui_down");
273:    action_add_event("ui_down", key);
275:    add_action("ui_page_up");
278:    action_add_event("ui_page_up", key);
280:    add_action("ui_page_down");
283:    action_add_event("ui_page_down", key);

scene/gui/base_button.cpp
189:            if (p_event->is_action("ui_accept")) {

scene/main/viewport.cpp
2020:           if (p_event->is_pressed() && p_event->is_action("ui_cancel") && !gui.modal_stack.empty()) {
2044:                   if (p_event->is_action("ui_focus_next")) {
2049:                   if (p_event->is_action("ui_focus_prev")) {
2054:                   if (!mods && p_event->is_action("ui_up")) {
2059:                   if (!mods && p_event->is_action("ui_left")) {
2064:                   if (!mods && p_event->is_action("ui_right")) {
2069:                   if (!mods && p_event->is_action("ui_down")) {

scene/gui/slider.cpp
96:             if (p_event->is_action("ui_left") && p_event->is_pressed()) {
102:            } else if (p_event->is_action("ui_right") && p_event->is_pressed()) {
108:            } else if (p_event->is_action("ui_up") && p_event->is_pressed()) {
115:            } else if (p_event->is_action("ui_down") && p_event->is_pressed()) {

scene/gui/item_list.cpp
562:            if (p_event->is_action("ui_up")) {
597:            } else if (p_event->is_action("ui_down")) {
631:            } else if (p_event->is_action("ui_page_up")) {
646:            } else if (p_event->is_action("ui_page_down")) {
662:            } else if (p_event->is_action("ui_left")) {
674:            } else if (p_event->is_action("ui_right")) {
686:            } else if (p_event->is_action("ui_cancel")) {
688:            } else if (p_event->is_action("ui_select")) {
699:            } else if (p_event->is_action("ui_accept")) {

@Ranoller
Copy link
Contributor Author

Ranoller commented Mar 3, 2018

I see... If their are mandatory, inputmap.erase_action should simply ignore this actions, it's nosensse that you can delete part of the engine in runtime.

@groud
Copy link
Member

groud commented Mar 3, 2018

The point is, why would you erase the default action ? Just ignore them if you don't need them.

@Ranoller
Copy link
Contributor Author

Ranoller commented Mar 3, 2018

Yes, this is the way that i will take, but iterating in get_action_list will be a bit more meshy testing that an action to erase is not one of the default action, and my input singleton is to meshy now!... To your question. I create and delete actions to reiniciate controls or depending of what kind of input the game will have (is a script reusable between projects) it's more confortable to me have one big script (3000 lines?) that in every project create an action list.... I know that i can do a template project... But this way i feel more confortable seeing that in code (same with group and connections, mix code with editor is prone to errors, this is my experience)

@Ranoller
Copy link
Contributor Author

Ranoller commented Mar 3, 2018

The other reason it's that actions names can be changed is enums without losing their funcionality in the game... Probably is the biggest script in my project and works very well (have funcionalities like classic toggle between keyboard and controller that allow to you to change ui inmediatly in screen like every game ir steam have)

@groud
Copy link
Member

groud commented Mar 3, 2018

I have no clue what you are trying to achieve byt removing / adding action. Usually you just change the mapping (by adding and removing event with action_erase_event or action_add_event), not the actions themselves.

@Ranoller
Copy link
Contributor Author

Ranoller commented Mar 3, 2018

I abstract actions to bools and floats and abstract action names to change this names in every project to maintain some coherency between action name and action results... I don't know if my aproach is good or bad, but works and works well, i will add a codition to ignore default actions in every iterating function and go on... But is strange to me that you can delete actions that are part to the core engine

@FeatherAntennae
Copy link

If the default actions are used by core components, they should probably be identified as such.

Giving them a normal but disabled delete icon and showing a tooltip on hover explaining why it's disabled would not only tell the user why it's important, but also show that it's not just a bug or something forgotten by the devs. People would be less tempted to go delete them manually afterward. Plus the UI would be a bit more consistent as the "add event" plus icon wouldn't change row from default actions to custom actions.

I think people mostly want to delete these action instead of ignoring them because it's hard to ignore them currently. It is a bit annoying to use the current UI to edit or set your own actions because they are all the way under a big list of premade actions you might never touch. Plus all actions are expanded by default, you can't search for actions and you can't move them up or down in the list.

A lot of people may never even change the defaults because they're pretty standard stuff. I don't see why someone would want to change "TAB" to anything else for "ui_focus_next". Your own inputs have way more chances of being the ones you want to edit.

There's a bunch of other things that could be improved in this UI. If it's interesting, I could open a new issue and make a list of things with some mocks to gather some input (pun intended) on a small overhaul to the InputMap editor window and then try to implement the changes myself.

@akien-mga
Copy link
Member

Superseded by godotengine/godot-proposals#303.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants