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

Change: Automatic screenshot numbering #9781

Merged
merged 4 commits into from Nov 8, 2022

Conversation

larryfenn
Copy link
Contributor

Invoking the screenshot console command with a filename ending in %d will cause it to be numbered automatically.

Motivation / Problem

As part of running my OpenTTD server I use console scripts to capture a screenshot of the game periodically. I use these screenshots to make time-lapse videos of the game.

However, the default behavior of the console command screenshot with a user-provided filename will always save just to that filename, causing it to be overwritten if it exists. The only way to get a 'reel' of screenshots automatically is to omit the filename argument, in which case the screenshots will be saved as e.g. screenshot.png, screenshot#1.png, screenshot#2.png, etc.

Description

The proposed change causes the automatic numbering to happen to any user provided filename that ends in %d.

Limitations

  • The %d token must be at the end of the user provided filename.
  • The automatically numbered files start with no number, and then count up from 1. This matches the existing behavior for generated filenames, where the first file generated is screenshot.png, followed by screenshot#1.png.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR touches english.txt or translations? Check the guidelines
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

Invoking the `screenshot` console command with a filename ending in %d
will cause it to be numbered automatically.
src/screenshot.cpp Outdated Show resolved Hide resolved
Invoking the `screenshot` console command with a filename ending in %d
will cause it to be numbered automatically.
@TrueBrain
Copy link
Member

Hmm. I am a bit torn. On one hand, I fully understand why you did the "has to end with" part. On the other hand, for %d that feels really odd.

Maybe we should do something like: when it ends with #, or something. Purely because anyone who understands %d would not expect it to only work at the end.

That all said, for sure this PR misses a change in documentation for the console command, mentioning this. Otherwise only those who have seen this PR know that this is now possible :) Mind updating the docs for this console command?

@larryfenn
Copy link
Contributor Author

I've added a documentation string to the console command - are there other places in the code where the screenshot command is documented?

@LC-Zorg
Copy link

LC-Zorg commented May 9, 2022

What do you think about using an actual date in front of each "screenshot" or "Donnington Transport, 2037-08-20"? I mean something like this:
real date + hour + screenshot / company name, game date that is for example: 2022-05-09 23.28.37 screenshot or 2022-05-09 23.28.37 Donnington Transport, 1950-03-28. In my opinion it would be useful also in case of all other screenshots made by players. Thanks to this, it would be easier to keep the chronology in the album and thus find the photos you are looking for. The history of the game would also be created in this way. For server and automatic screenshots, this can also make finding events easier.

With screenshot numbers like #1, #2, #3 ... #287 there may be a problem that if you remove the middle positions, the newest screenshots will most likely have the first free name, which will destroy the chronology. This problem existed on Sony phones and, until recently, on Windows as well. This is obviously a different use, but the experience was really terrible. However, if you want to keep this form, it is worth making sure that each subsequent screenshot never uses a number lower than the last one.

Copy link
Member

@2TallTyler 2TallTyler left a comment

Choose a reason for hiding this comment

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

@LC-Zorg's suggestions could be another enhancement PR but shouldn't preclude this one from being merged. Anyone writing a script to automate taking screenshots should be more than capable of sorting their screenshots.

Commits need squashing but that can happen when merging. 🙂

@LordAro LordAro merged commit d738cd2 into OpenTTD:master Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants