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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Input is_anything_pressed method #35012

Merged
merged 1 commit into from Jan 20, 2022
Merged

Add Input is_anything_pressed method #35012

merged 1 commit into from Jan 20, 2022

Conversation

Xrayez
Copy link
Contributor

@Xrayez Xrayez commented Jan 11, 2020

Your typical "Press any key" use case using the Input singleton, can also be used to conveniently detect and switch various idle states, such as character switching to bored state if you wish so. 馃檪

func _physics_process(_delta):
	if Input.is_anything_pressed():
		$message.text = "Thanks!"

There's another way for doing so using _input:

func _input(event):
	if event.is_pressed():
		$message.text = "Thanks!"

But it might not be possible to achieve in the future according to:

godot/core/os/input_event.h

Lines 191 to 194 in 02cd144

// To be removed someday, since they do not make sense for all events
virtual bool is_pressed() const;
virtual bool is_echo() const;
// ...-.

In any case, I'm here to tell that it might be a good idea to leave InputEvent.is_pressed() as is.

Test project

press-any-key.zip

if (E->get().pressed) {
return true;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The code is good, but I have a small suggestion! Instead of checking if each key is pressed which can harm performance, why not check to see if any key was pressed using only 1 if statement! This can be easily done by creating an integer counter before the for loop and initialized to 0, then instead of doing if (E->get().pressed) we do counter += (int)E->get().pressed .. after the for loop, if the counter is over 0, then a key was pressed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just profiled both current and your suggestion, I couldn't detect much performance difference (at most 1 usec improvement). That's around going through 5-8 actions. With the counter, you do have to go through all actions. But when you detect if the action is pressed, you can just exit the loop with return immediately. Most of the time the user would actually be pressing something, so we're interested in short-cutting I think. If the user is away, the performance doesn't matter anyway. So, readability would be preferred in this case imo.

I'll try to see if there's any more significant difference with more actions, but I doubt the user would like to define many of them. In fact, it's not going through all InputMap actions, but only those which were pressed during gameplay. It means that the more actions you press, the more the action_state would look like an InputMap.

@Chaosus Chaosus added this to the 4.0 milestone Jun 4, 2020
@Xrayez
Copy link
Contributor Author

Xrayez commented Mar 10, 2021

This PR is one year old, yes or no? It would be bad to close it just like that as this PR received some good chunk of 馃憤s.

@Calinou
Copy link
Member

Calinou commented Mar 10, 2021

I've been thinking about practical use cases for this, but I'm having trouble finding any.

can also be used to conveniently detect and switch various idle states, such as character switching to bored state if you wish so. 馃檪

This can generally be achieved by checking whether any of the movement or action keys have been pressed in the last N seconds.

As for "Press any key to continue" dialogs, this can be achieved using if event is InputEventKey and event.is_pressed(), although you'll also want to handle controller input and perhaps exclude specific keys (depending on your use case). There's no "one size fits all" solution here.

@Xrayez
Copy link
Contributor Author

Xrayez commented Mar 10, 2021

To clarify your post, there are use cases for this (it's quoted), there are just existing alternatives which may or may not make this PR less useful in comparison.

if event is InputEventKey and event.is_pressed()

It's the type of thing this PR aims to overcome. A lot of beginners simply won't be able to do this sort of filtering in one go.

doc/classes/Input.xml Outdated Show resolved Hide resolved
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Approved in PR meeting 馃憤

@Xrayez Xrayez requested a review from a team as a code owner January 20, 2022 16:23
@akien-mga akien-mga merged commit 73d0013 into godotengine:master Jan 20, 2022
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

5 participants