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

Play Time #3543

Merged
merged 2 commits into from
Apr 11, 2022
Merged

Play Time #3543

merged 2 commits into from
Apr 11, 2022

Conversation

CSutter5
Copy link
Contributor

@CSutter5 CSutter5 commented Mar 7, 2022

Made it so CKAN can track how long you play the game each time you launch the game. Then it saves it to a file. You can view the total amount of playtime by going under File->Play Time

@CSutter5 CSutter5 marked this pull request as ready for review March 7, 2022 16:35
GUI/Main/Main.cs Outdated Show resolved Hide resolved
GUI/Main/Main.cs Outdated Show resolved Hide resolved
GUI/Main/MainTime.cs Outdated Show resolved Hide resolved
GUI/TimeLog.cs Outdated Show resolved Hide resolved
GUI/TimeLog.cs Outdated Show resolved Hide resolved
GUI/Main/Main.cs Outdated Show resolved Hide resolved
GUI/TimeLog.cs Outdated Show resolved Hide resolved
GUI/Main/Main.cs Outdated Show resolved Hide resolved
@HebaruSan HebaruSan added Enhancement GUI Issues affecting the interactive GUI Pull request labels Mar 7, 2022
GUI/Controls/PlayTime.cs Outdated Show resolved Hide resolved
GUI/TimeLog.cs Outdated Show resolved Hide resolved
@CSutter5
Copy link
Contributor Author

CSutter5 commented Mar 8, 2022

Ok, I think I have fixed all of the comments you made

@vinix38
Copy link
Contributor

vinix38 commented Mar 8, 2022

I found 3 strings that could be translated, so here they are in French:

<data name="GamePlayTime.Text" xml:space="preserve"><value>Temps de Jeu (H:M.s)</value></data>
<data name="PlayTimeTabPage.Text" xml:space="preserve"><value>Temps de Jeu</value></data>
<data name="TotalPlayTime" xml:space="preserve"><value>Votre Temps de Jeu Total</value></data>

Tell me if there is anything else to translate 🙂

@CSutter5
Copy link
Contributor Author

CSutter5 commented Mar 8, 2022

@vinix38 Thank your for the translation there is also GUI/Controls/PlayTime.resx

@vinix38
Copy link
Contributor

vinix38 commented Mar 8, 2022

Indeed, there you go:
<data name="nameHeader.Text" xml:space="preserve"><value>Nom de l'Instance</value></data>
<data name="timeHeader.Text" xml:space="preserve"><value>Votre temps de jeu (H:M.s)</value></data>
<data name="PlayTimeOkButtom.Text" xml:space="preserve"><value>Ok</value></data>

GUI/CKAN-GUI.csproj Outdated Show resolved Hide resolved
@HebaruSan

This comment was marked as resolved.

@CSutter5
Copy link
Contributor Author

CSutter5 commented Mar 9, 2022

@HebaruSan I wasn't able to replica the error but I added some stuff to stop it from crashing

@HebaruSan
Copy link
Member

HebaruSan commented Mar 9, 2022

@HebaruSan I wasn't able to replica the error but I added some stuff to stop it from crashing

That's a bit better, but still not usable because it displays a popup and then the form doesn't work. I will push a commit in a moment to fix this plus several other small things that I noticed that are easier to just change than describe and check...

@vinix38, I updated the fr-FR strings because I also changed the time display format. When I see people talk about how long they've played a game, I think it's 100% in terms of "hours"; nobody cares about days or minutes or seconds. So now if you've played 5 hours and 30 minutes, we'll display "5.5", and the column headers are "Hours Played" in English. I plugged that in to Google Translate for French, please change if it's not right.

@DasSkelett, do you think it makes sense to have a menu item to load a tab page for this? I think it's questionable, but it probably doesn't hurt either and it's already done.

@vinix38
Copy link
Contributor

vinix38 commented Mar 10, 2022

So now if you've played 5 hours and 30 minutes, we'll display "5.5", and the column headers are "Hours Played" in English. I plugged that in to Google Translate for French, please change if it's not right.

Just did so 👍‍

@HebaruSan

This comment was marked as resolved.

@CSutter5
Copy link
Contributor Author

Remaining things I want to address before merge

I can start working on it. I should be able to get stuff done next week b/c it's spring break for me.

@CSutter5
Copy link
Contributor Author

Does anything bad happen if I close CKAN while the game is still running?

When CKAN closes it now save the current time on the stopwatch

Is it possible to launch the game, switch to another instance with the game still running, and launch again?

You can now Launch an instance then switch to a new instance and it will save the play time to the correct instance

What happens if the launch fails (e.g., for a fake instance)? Does the timer stop?

If the launch fails it should be caught by the try catch and not start the stopwatch

Consider adding the instance play time to the status bar next to the instance name

Added

Consider showing total play time somewhere on the manage game instances popup

I believe this was already added, unless I'm thinking of something else

Should there be a setting to turn this functionality on and off? I.e., will any users be annoyed by it enough to want that?

I don't think there is a reason to turn it off, I think most users would like this.

Is there any way we could capture play time when launching the game from Steam or the command line?

This can be done, I believe we would have to use the Steam WebAPI, Users would also have to have a public profile

Should the play time be cleared when cloning an instance, to avoid double counting?

Added

Should the play time be a public property of GameInstance?

Added

@CSutter5 CSutter5 requested a review from HebaruSan March 20, 2022 00:30
GUI/Main/Main.cs Outdated Show resolved Hide resolved
@HebaruSan

This comment was marked as resolved.

@CSutter5
Copy link
Contributor Author

Ok, I think that is a fair argument to be made, will think of away to turn it on an off

@CSutter5 CSutter5 requested a review from HebaruSan April 2, 2022 17:06
@HebaruSan
Copy link
Member

Just pushed a somewhat big commit, the general idea of which is to let the user edit the instances' play times in the grid instead of using an "external time" concept/popup:

image

This way we don't have to explain what "external time" means, and the user can split up their previous Steam play time across different instances if they want to. Also moved the total time out to a separate label so the user doesn't try to edit that.

Consider showing total play time somewhere on the manage game instances popup

I believe this was already added, unless I'm thinking of something else

I meant on this form:

image

But on second thought, this doesn't seem important. A user who's really curious can check the dedicated Play Time screen.

Should there be a setting to turn this functionality on and off? I.e., will any users be annoyed by it enough to want that?

I don't think there is a reason to turn it off, I think most users would like this.
Ok, I think that is a fair argument to be made, will think of away to turn it on an off

The column in Manage Game Instances is now auto-hidden if all the values are 0, which means it'll become visible automatically if such a user starts launching from CKAN but otherwise stay out of the way. No need for an explicit setting now.


I'm pretty happy with how this works now. If you want to make any other changes, go ahead and do that, otherwise I'll squash this down to a small number of commits and merge.

Copy link
Member

@HebaruSan HebaruSan left a comment

Choose a reason for hiding this comment

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

Let's try it! 🎆 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement GUI Issues affecting the interactive GUI Pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants