Skip to content

Add option to display onscreen time even while running content (refactoring of PR #7475)#10526

Closed
Ryunam wants to merge 1 commit into
libretro:masterfrom
Ryunam:onscreen-time
Closed

Add option to display onscreen time even while running content (refactoring of PR #7475)#10526
Ryunam wants to merge 1 commit into
libretro:masterfrom
Ryunam:onscreen-time

Conversation

@Ryunam

@Ryunam Ryunam commented Apr 27, 2020

Copy link
Copy Markdown
Contributor

Description

This commit consists of a refactoring of PR #7475, originally made by @grimpunch to address feature request #6890. This was a request I had opened for an option to have a persistent display of the current time onscreen, even while running content.

Compared to the initial implementation, this accomplishes the following:

  • the duplication of the entries that were mentioned by @RobLoach has been avoided altogether;
  • the option has been integrated into the Show FPS / Show Memory widget, so that it follows the same font style / size and reacts to the same scaling options;
  • the option also works independently from the Show FPS / Show Memory widgets. Users can choose to mix and match between displaying FPS, memory and the current time (or possibly all of these at the same time);
  • I have also provided some translations for the newly-added menu entry and corresponding sublabel.

Naturally all credits go to @grimpunch for the original code that is now in retroarch.c. I merely refactored the original work, updated it to fit the current RetroArch codebase and fixed a few remaining issues that were described in the other thread.


A few pics of how it looks (this is with the custom font and size that I chose for my notifications, which appears a bit tiny in these screenshots. By default the text will use the same font and size of the FPS indicator):

With only Onscreen Time enabled:

Annotation 2020-04-27 223810

With FPS, Memory and Onscreen Time enabled:

Annotation 2020-04-27 223940

Related Issues

#6890

Related Pull Requests

Serves as a replacement for #7475

@lgtm-com

lgtm-com Bot commented Apr 27, 2020

Copy link
Copy Markdown

This pull request introduces 1 alert when merging 05a427f into 6642868 - view on LGTM.com

new alerts:

  • 1 for Use of potentially dangerous function

@inactive123

Copy link
Copy Markdown
Contributor

I'm posting this message on @jdgleaver's behalf since for whatever reason he wasn't able to post this message here -


This PR is a nice idea!

It would be good to implement this as an independent widget, but I understand that doing so would be a tall order. Could probably be done in a separate PR, anyway.

The one issue I see is the code here - I would be reluctant to call strftime(osd_time, sizeof(osd_time), "%H:%M:%S", localtime(&time_)); on every frame (it's slow, and involves a mutex lock and filesystem access).

For menu stuff we normally call this:

void menu_display_timedate(gfx_display_ctx_datetime_t *datetime);

...which caches the result and only updates when required - but this would mean you'd have to use the current menu time display format, so if you want a fixed "%H:%M:%S" then it's probably not appropriate...

I guess you could add a new static global osd time string, and update it here once per second if onscreen_time_show is true...?

@Ryunam

Ryunam commented May 22, 2020

Copy link
Copy Markdown
Contributor Author

Hi! I would like to work on this a bit more, rebase it and apply @jdgleaver’s suggestions that we also discussed on Discord. For the time being I will close this so as to avoid excessive cluttering. I’ll submit a new PR when all the requested changes have been implemented.

@rbreaves

rbreaves commented Feb 5, 2025

Copy link
Copy Markdown

Feel like this needs revisiting - been something like 4-5 years since this was last looked at and still nothing comparable has been merged. I do appreciate the work that has already been done on this - gives someone the opportunity to update it and get it merged.

I do think the default for the time formatting though might ought to exclude seconds imo and/or allow the user to set the time formatting. I think one real & large use case for this is average users that just want to know what time it is while gaming - not just speed runners.

@Ryunam Ryunam deleted the onscreen-time branch May 9, 2026 22:18
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.

3 participants