dolphin
dolphin copied to clipboard
dolphin-emu
Track Time Played (Core and QT)
This adds a total "Time Played" tab in the QT interface - the time is started at the beginning of emulation and ended when emulation shuts down. A general TimePlayed class is implemented which the desktop QT interface pulls from. This does not require a standard exit of emulation to count the time.
This needs to be made translatable. https://doc.qt.io/qt-6/i18n-source-translation.html#using-numbered-arguments
So design wise, but I'm concerned about the amount of horizontal space the time played takes in that screenshot. I would definitely prefer if it was in a shortened format like this - "6h 8m". It still conveys the information but it takes WAY less space.
I squashed the commits again though the language files seemed to be automatically modified
It looks as if you rebased this pull request on the latest version of master but then undid the changes from the last version of master.
EDIT: You undid more than just the latest commit, actually. See the change to State.cpp, for instance.
The git fetch upstream is important, and must be done before every reset if you stick to the recipe above.
@BhaaLseN Hold on, why is there a soft reset in the instructions you posted? I'm pretty sure that's the cause of this happening.
The
git fetch upstreamis important, and must be done before every reset if you stick to the recipe above.
I did that, I followed these steps that were posted before.
git fetch upstream git reset --soft upstream/master git commit -m "your commit message here" git push -f origin HEAD
@BhaaLseN Hold on, why is there a soft reset in the instructions you posted? I'm pretty sure that's the cause of this happening.
The soft reset leaves changes (from this branch; not from the diff between the old and new master) in the staging area, vs. a mixed reset that leaves them unstaged. It shouldn't cause this problem (at least not by itself). I didn't check history yet, but a git pull beforehand is likely to cause this.
It might cause this when the commits this branch is based on are suddenly gone; but this would mean that master was rewritten; or that this PR was based on another one (which was rebased in the mean time) - neither of which is the case here.
I'm afraid at this point, the only way to fix this is by unstaging the unintended reverts by hand, and resetting them locally. At least I can't think of a simple way to get back; other than git reset --hard a139affaf843231ce88b6cb75d34b19c72d37767 and trying again (except that I don't think I can force GitHub to give me that ref, so I can't try at the moment)
The soft reset leaves changes (from this branch; not from the diff between the old and new master) in the staging area, vs. a mixed reset that leaves them unstaged. It shouldn't cause this problem (at least not by itself).
When you soft reset, the state of all files is kept as they were. That is, not only are the changes from the PR preserved, but none of the files that have been modified in master are brought up to date with master. When you then commit your staged changes, you are committing your old versions of files that have been modified in master.
The soft reset leaves changes (from this branch; not from the diff between the old and new master) in the staging area, vs. a mixed reset that leaves them unstaged. It shouldn't cause this problem (at least not by itself).
When you soft reset, the state of all files is kept as they were. That is, not only are the changes from the PR preserved, but none of the files that have been modified in master are brought up to date with master. When you then commit your staged changes, you are committing your old versions of files that have been modified in master.
Interresting, I've used this a bunch before and never had any issues. The docs are are bit ambiguous though, now that I read the section again ("doesn't touch the index" but "leaves all changed files to be committed"). Guess I'll drop this out of my saved replies for safety reasons if it has the potential to mess things up.
I also don't see any handling of the emulator pausing here, so paused time counts as playtime. I suspect that is not desired.
I also don't see any handling of the emulator pausing here, so paused time counts as playtime. I suspect that is not desired.
It would not count time while in the middle of TASing a game and have the game paused, but maybe an average user expects a paused game to not count time; I'm conflicted on whether to count it or not.
Edit: I decided to not add to the time when emulation is paused.
@BhaaLseN wrote:
I like the idea in general; and we could explore a future extension of this to also work for Wii games and write their playtime to the NAND (so they show up in the Wii menu).
I thought this already worked on master, but it looks like it only properly works if you launch games from the system menu (otherwise you get a 23:59 blank entry). Apparently the system menu needs to do some stuff with play_rec.dat beforehand, and then games will update it. But we aren't currently creating it beforehand, so things break (and presumably we'd also need to update the message board before each start so that things behave properly if multiple games are launched without loading the system menu).
As far as I can tell, the feature added by this PR works correctly for Wii games, though.
I thought this already worked on master, but it looks like it only properly works if you launch games from the system menu
Yes, it should work that way already. But I meant that we can also do that now if you run it from the game list. If we want to, that is.
Github is still showing unresolved requested changes, it cannot be merged until that is sorted.
Also lint is mad at you.
Ah, sorry I kinda forgot about this.
Anyway, this is better but the threading model is still pretty fishy. In particular:
- Any given 30 second time slice is either counted as playtime or not playtime depending on whether the state is
Runningor not at a very particular timepoint. This will make the time of sessions where you repeatedly pause and unpause very inaccurate. I'm not sure if this is really a big deal -- I have no idea how accurate you want this to be or why you want this feature in the first place -- but just to note. - You're timing by sleep, which is not particularly accurate and will drift over time. Again, might not be a big deal, but it's definitely not accurate.
- And this is the big one... there's no exit condition other than state being
PowerDownat the end of a timeslice. This means that when exiting emulation, the main thread may be forced to wait up to 30 seconds for the timer thread to finish, during which you cannot start another emulation because the current one isn't done shutting down yet. (It also hangs the UI.)
Oh, and one more thing: This doesn't seem to handle game ID switches correctly. So if you eg. start the Wii menu and launch a game, all the time will be counted for the Wii menu and not the game.
Finally revived this PR to address AdmiralCurtiss' changes. This uses a conditional_variable wait instead of sleep so now when Dolphin goes to shut down, it will call notify.one() to close the thread without interrupting the main thread. A steady_clock is used for more accurate timing. The game_id is also refetched for each loop so if the game changes, it will add the time to the correct game. Continually pausing could still cause the time to be less accurate but I think this is preferable to counting paused time.
The commit history was a complete mess (and also didn't build and failed lint) so I rebased and squashed it (to ed1c7b4e7b92918f5cd41784252386e84a786354). If you want to do any further changes yourself please do a force pull locally before you commit anything.
Anyway,