-
Notifications
You must be signed in to change notification settings - Fork 220
New setting options for screenshots: Periodically take auto-screenshots + option for timestamps in file names #3406
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
Conversation
…feature for adding the current timestamp to file names
|
This slowly needs a submenu just for screenshots :D |
|
Open for discussion: Is that screenshot timestamp setting even needed or should it just become the default and replace the old naming? |
I would be fine with changing it to the new timestamped naming scheme by default. |
|
I've seen different examples of people using the screenshot feature ingames, things like a dynamic battle bg based on map screenshots from dynrpg, or the maniacs screenshot command. I never tooka look at those tho, to see if they allow reusing the screenshot default filename |
Ghabry
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be approved after adressing the nitpicks
src/window_settings.cpp
Outdated
| auto fmt_sample_name = [](bool is_auto_screenshot) { | ||
| auto name = Output::GetScreenshotName(is_auto_screenshot); | ||
| if (Player::player_config.screenshot_timestamp.Get()) { | ||
| //return name + "[_1].png"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment can be removed
src/game_config.h
Outdated
| BoolConfigParam lang_select_in_title{ "Show language menu on title screen", "Display language menu item on the title screen", "Player", "LanguageInTitle", true }; | ||
| BoolConfigParam log_enabled{ "Logging", "Write diagnostic messages into a logfile", "Player", "Logging", true }; | ||
| RangeConfigParam<int> screenshot_scale { "Screenshot scaling factor", "Scale screenshots by the given factor", "Player", "ScreenshotScale", 1, 1, 24}; | ||
| BoolConfigParam screenshot_timestamp{ "Screenshot timestamp", "Add the current date and time to the file name", "Player", "ScreenshotTimestamp", false }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets change this to default true as is obviously the better default
| auto diff = std::chrono::duration_cast<std::chrono::seconds>(Game_Clock::now() - last_auto_screenshot); | ||
| if (diff.count() >= Player::player_config.automatic_screenshots_interval.Get()) { | ||
| last_auto_screenshot = Game_Clock::now(); | ||
| Output::TakeScreenshot(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(just a suggestion, not something you have to add here)
Slightly more useful (for me) would be when this is based on game frames and not by time.
My current workflow to create these comparison videos in the blog post is:
Create a recording via --record-input rec and the record the session in the old and the new player executed via --replay-input rec.
Then I manually sync both recordings in kdenlive.
The recording stuff could be automated via this screenshot feature. (but I guess this will kill the performance when taking 60 screenshots per second, so is maybe a bad idea xD)
b75ff8a to
a6cf6ee
Compare
Periodically take automatic screenshots
Timestamps in filenames
This was also requested recently & is pretty easy to implement, so I added this option as well:
If there is a naming conflict (multiple presses of the screenshot key in a single second), file names will still be indexed (starting with "_1.png", rather than "_0.png")