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

Add a default keyboard shortcut to take a screenshot in a running project #76982

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented May 12, 2023

The default key binding for ui_take_screenshot is F12, but it can be changed in the project settings' input map (make sure to enable Show Built-in Actions). See godotengine/godot-proposals#6789 for detailed rationale.

Testing project: test_take_screenshot.zip

Preview

Screenshot of a project with a transparent background while paused, without any tearing despite having V-Sync disabled:

screenshot_2023-05-12T041210

@Calinou Calinou requested review from a team as code owners May 12, 2023 02:13
@Calinou Calinou added this to the 4.x milestone May 12, 2023
Comment on lines +1397 to +1568
// Using formatting around the printed path allows using Ctrl + Click in VS Code's integrated terminal to open the screenshot:
// <https://github.com/microsoft/vscode/issues/97941>
print_line_rich(vformat("Screenshot saved to: [u]%s[/u]", path));
Copy link
Member Author

@Calinou Calinou May 12, 2023

Choose a reason for hiding this comment

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

We should probably use this in other parts of the engine 🙂
See microsoft/vscode#97941.

Note that quoting paths in either single or double quotes will not work, so formatting must be used for "generic" paths. It can be any formatting, not necessarily underline (but I think underline makes the most sense in context – plus, VS Code will display a second underline on hover, so you still get visual feedback on hover). There are other types of paths with spaces supported, but these are intended to be used for programming stack traces.

@Calinou Calinou force-pushed the add-project-screenshot-key branch from 37a5031 to fe9ba64 Compare May 12, 2023 02:44
@Sauermann
Copy link
Contributor

Sauermann commented May 12, 2023

When using embedded windows, the PR will create multiple screenshots, one for each embedded window, that receives the InputEvent, possbibly overwriting each other when they happen within the same second.
Possible solution would be to set the InputEvent as handled after taking the screenshot.

Additional native windows will not be included in the screenshot.
Maybe this PR could be complemented by #75142.

@Calinou Calinou force-pushed the add-project-screenshot-key branch from fe9ba64 to 4a4b236 Compare May 21, 2023 22:02
@Calinou
Copy link
Member Author

Calinou commented Jun 26, 2023

Possible solution would be to set the InputEvent as handled after taking the screenshot.

Done 🙂

scene/main/window.cpp Outdated Show resolved Hide resolved
@Calinou Calinou force-pushed the add-project-screenshot-key branch from 4a4b236 to ec4ca13 Compare July 17, 2023 07:05
Copy link
Contributor

@Sauermann Sauermann left a comment

Choose a reason for hiding this comment

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

Regarding Input-handling, the changes are looking good.

@akien-mga
Copy link
Member

I'm conflicted about this for very selfish reasons, as I use F12 for my drop-down terminal yakuake on Linux. I've been using that terminal for years and F12 is its default keybinding. This change would conflict and since it's part of the project input map, it would be impossible for me to disable it in all the tests projects or games I use.

@Zireael07
Copy link
Contributor

oh, another Yakuake user here, yeah F12 would conflict :(((

@Calinou
Copy link
Member Author

Calinou commented Sep 11, 2023

I'm conflicted about this for very selfish reasons, as I use F12 for my drop-down terminal yakuake on Linux. I've been using that terminal for years and F12 is its default keybinding. This change would conflict and since it's part of the project input map, it would be impossible for me to disable it in all the tests projects or games I use.

F12 would also conflict with the default Steam screenshot key (resulting in 2 screenshots being taken every time), so maybe it's better to use something like Shift + F12 instead. What do you think?

@akien-mga
Copy link
Member

Shift + F12 sounds better to me.

That being said, the more I think about it the more I'm worried about the engine providing a builtin shortcut in all games which may conflict with user systems and favorite tools.

The current builtin input actions are those strictly necessary for expected UI interactions, and are unlikely to conflict with user hotkeys (as they're the same used by other GUI toolkits so they're not suitable for global hotkeys in the first place).

This PR would add something that departs from this and adds an always on hotkey. While the game developer can remove it on a per-project basis, most won't (that's the point of it) and thus most Godot games will support this hotkey, possibly clashing with the player's preference.

So I'm not sure whether we should actually implement this, even if I support the use case.

More opinions would be welcome :)

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.

Add a default keyboard shortcut to take a screenshot in a running project
5 participants