8000 Qt: Implement reset play time for disc sets by Pesa · Pull Request #3442 · stenzek/duckstation · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Qt: Implement reset play time for disc sets #3442

New issue

Have a question about this pr 8000 oject? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 14, 2025
Merged

Qt: Implement reset play time for disc sets #3442

merged 2 commits into from
Jun 14, 2025

Conversation

Pesa
Copy link
Contributor
@Pesa Pesa commented Jun 11, 2025 •
edited
Loading

A few things I'm not sure about:

  • Should I use entry->disc_set_name or entry->title? is there a difference if the entry is a discset? Ended up using entry->path because disc_set_name is empty for the actual DiscSet list entry.
  • I don't fully understand the locking (and the use of a recursive mutex doesn't make it any easier). It seems that the whole thing is already executed with the lock held, starting from MainWindow::onGameListEntryContextMenuRequested, so maybe I don't need to lock again in clearGameListEntryPlayTime()? Assuming that the triggered menu action is executed synchronously inside menu.exec()...
  • I kept changes to a minimum but, if desired, repeated invocations of GameList::ClearPlayedTimeForSerial could be optimized by passing a list of serials, so that we iterate s_entries only once when resetting multiple entries.

stenzek
stenzek reviewed Jun 11, 2025
View reviewed changes
stenzek
stenzek reviewed Jun 11, 2025
View reviewed changes
@stenzek
Copy link
Owner
stenzek commented Jun 11, 2025

I don't fully understand the locking (and the use of a recursive mutex doesn't make it any easier). It seems that the whole thing is already executed with the lock held, starting from MainWindow::onGameListEntryContextMenuRequested, so maybe I don't need to lock again in clearGameListEntryPlayTime()? Assuming that the triggered menu action is executed synchronously inside menu.exec()...

It's a bit janky, yeah. exec() is blocking, so it won't return until the menu is closed. This keeps entry alive, since it is captured by reference in the callbacks. The recursive mutex is needed in case any of those actions mutate game list state.

The downside is that scanning will effectively get halted while a context menu is open. We'd have to create copies of the entry's fields for the actions/callbacks to avoid that, and is actually what I've done in my other duckstation-derived host.. should probably backport it.

@stenzek
Copy link
Owner
stenzek commented Jun 11, 2025

Should I use entry->disc_set_name or entry->title? is there a difference if the entry is a discset?

Probably want to use disc_set_name. I don't support custom titles for disc sets currently, but if it was to be added, then it could differ.

@Pesa
Copy link
Contributor Author
Pesa commented Jun 12, 2025

Addressed review comments but haven't tested the new commit yet.

@Pesa Pesa marked this pull request as draft June 13, 2025 04:00
@Pesa Pesa marked this pull request as ready for review June 13, 2025 19:32
@Pesa
Copy link
Contributor Author
Pesa commented Jun 13, 2025

It's tested and working now.

@stenzek stenzek merged commit 70225f8 into stenzek:master Jun 14, 2025
11 checks passed
@Pesa Pesa deleted the reset-discset branch June 14, 2025 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stenzek stenzek stenzek left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Pesa @stenzek
0