gogcom / galaxy-integrations-python-api

NOTE: Please report here only issues related to the python API. Issues and general feedback regarding the Galaxy Client 2.0 shall be sent via Galaxy Client menu
MIT License
1.25k stars 226 forks source link

Better manual time tracking support / access to cloud playtime data #167

Open urwrstkn8mare opened 3 years ago

urwrstkn8mare commented 3 years ago

Problem

The problem is that if implementing manual time tracking for games on launchers that don't support it (in my case, using galaxyutils), time data collected on one device will overwrite time data from another device. For example, my integration tells GOG Galaxy using the API that the user has played a game for 14 minutes on device 1 after manually tracking how long they've played while launching via GOG Galaxy. Then the same user on device 2 plays the same game for 2 minutes. This playtime has also been tracked by my integration that's running on that device 2's GOG Galaxy as well. Since device 2's time was reported later, it will overwrite wherever GOG Galaxy stores playtime in the cloud and the playtime for the game on both devices will read: 2 minutes. However, it should actually read 16 minutes.

This is not an issue in for one device but becomes a very big issue as soon as you use >1 devices.

Solution

An optimal solution would be to provide another method of reporting playtime that does not make the integration tell you the user's total playtime (that method is useful for launchers that do natively collect playtime data). This method involves reporting time per session. The integration, after a user finishes playing a game, immediately reports to GOG Galaxy the playtime for only that session. The added benefit is that the API can get the last played time from the timestamp of when playtime for the last session was reported. Instead of overwriting the playtime in the cloud, the API will simply add the playtime of the latest session to the total playtime and change the last played value to the timestamp of when the integration reported the latest session's playtime. This would solve the problem.

Here is a very rough example of how this could be used in code: (the following functions would be in the integration class)

# Launch Minecraft no matter the game_id (not actually how it would be done)
async def launch_game(self, game_id):
    cmd = "minecraft.exe" # not the actual command/path to launch Minecraft
    self.game_process = await subprocess.Popen(cmd) # Assign the returned process to a property of the integration class
    self.start_game_time = datetime.now() # Store time when game started

def tick(self):
    if self.game_process is not None: # A process has actually been assigned to the property.
        if self.game_process is not None: # Process has exited.
            end_game_time = datetime.now() # Get time when game finished
            time_played = end_game_time - self.start_game_time # Get total time played
            self.report_played_session(time=time_played) #Report the time played to GOG Galaxy (it can infer the last played time)

Another bonus is that GOG Galaxy will be almost instantaneously notified by the integration when there's an update to playtime negating the need to request it repeatedly from the integration.

Alternatives

You could also allow the integration to fetch the playtime stored on the cloud so they can manually add the time to the cloud playtime and report that to GOG Galaxy.

mbanczerowski commented 3 years ago

@urwrstkn8mare thanks for the report!

The another comprehensive solution would be to provide game executable name to be tracked by Galaxy itself like it does for GOG games (and thus cloud syncing), but I'm not sure if it is easily achievable and can be added any time soon. I'll discuss the issue importance within my team.

urwrstkn8mare commented 3 years ago

Yes, that is a good idea, but the for some integrations providing game executable may not be that feasible as there may be peculiarities that integrations can better handle (eg. args or more complex).

I think an easier way to implement time tracking easily without any of the problems described above, may be to get GOG Galaxy to track time based on game status. I think the best way is through another rough example:

def __init__(self, reader, writer, token):
    super().__init__(
        Platform.Test,
        "0.1",
        reader,
        writer,
        token,
        auto_time_tracking=True # Option to enable auto time tracking based on game status.
                                # Default: False
    )

async def launch_game(self, game_id):
    cmd = "minecraft.exe" # not the actual command/path to launch Minecraft
    self.game_process = await subprocess.Popen(cmd) # Assign the returned process to a property of the integration class
    self.update_local_game_status(LocalGame('minecraft', LocalGameState.Installed|LocalGameState.Running)) # Update game status to running

def tick(self):
    # Probably should check if the game is actually installed but this is just an example :)
    if self.game_process is not None: # A process has actually been assigned to the property.
        if self.game_process is not None: # Process has exited.
            self.update_local_game_status(LocalGame('minecraft', LocalGameState.Installed)) # Update game status to not running

This would negate the need for almost any change to the API facing the integration developer except for an optional parameter in the __init__ function. I obviously can't say for sure but I don't think this should be that hard to implement in GOG Galaxy. This will fix the issue above while also removing the need to implement repetitive manual time tracking in any integration which launcher doesn't natively support time tracking - making developing integrations a little easier :). I hope that is a little more achievable but I'm not sure.

mbanczerowski commented 3 years ago

Yes, that is a good idea, but the for some integrations providing game executable may not be that feasible as there may be peculiarities that integrations can better handle (eg. args or more complex).

Can you give an example?


Your proposal is really nice, I have some dubs on implementation you're proposed.

Imagine plugin have 2 kind of games: one supporting game time, the other - not. Then Galaxy would need to call get_owned_games anyway and apply its own time tracking only if plugin marks it so either by returning GameTime(None) or other special value, like GameTime(time_played=TRACK_BY_GALAXY). In the last case, we don't need auto_time_tracking in constructor at all.

But this solution is still a hack to existing API design, as get_owned_games is normally run frequently. What if plugin responds differently each time? I'm not saying it is impossible but it has to be think over for different edge-cases. And those have to be handled in Galaxy as a higher level element.

This would negate the need for almost any change to the API

Don't worry for changes in this repo, more important is overhead on Galaxy client.


Using GOG games time tracker has one more big advantage on session-based API interface: you don't have to implement your own tracker. And this is not only start-stop game thing, Galaxy time tracker is smart enough to know if you're idle in the game.

EDIT: So still we have 2 approaches to be considered:

Both has +/- es we need to take a closer look which one is better and easier to do.

urwrstkn8mare commented 3 years ago

Can you give an example?

Now that I think about it, for the Minecraft or Riot plugins, both have can provide GOG Galaxy with executables as long as it can run the executable with the args. However, I'm not sure, but I think some integration may need to launch a game through a launcher (e.g. startclient://launchgame/{game_id}) but the launcher doesn't have any time tracking capabilities. The launcher, however, does report game status (not game time) or the integration is capable of looking at running tasks and see if the game is running.

So still we have 2 approaches to be considered

Because Galaxy's time tracker is smarter than a start-stop game time tracker, I think the best case would be to implement both approaches but not necessarily at the same time. I think whichever one is easier to do should be implemented first and then the other. However, using game status to track may cover more integrations than executables but I can't say for sure because I'm not sure how other integrations work.

Then Galaxy would need to call get_owned_games anyway and apply its own time tracking only if plugin marks it so either by returning GameTime(None) or other special value, like GameTime(time_played=TRACK_BY_GALAXY).

Sorry if I'm wrong, but isn't GameTime returned from get_game_time? If that was what you were referring to, then you can call it for each game as usual. But after the first time in a session of GOG Galaxy, if the integration has reported tracking game time by status, then we can safely not call it again for the specific game.


Edit: Kind of unrelated, but allowing the integration to store and receive a very small JSON object in the cloud would be a pretty cool additional feature that could be very useful. It would first become a bandage for the issue I described in this thread and could also allow keeping the user signed in across devices if applicable similar to how the official Epic Games integration does it.

mbanczerowski commented 3 years ago

Can you give an example?

Now that I think about it, for the Minecraft or Riot plugins, both have can provide GOG Galaxy with executables as long as it can run the executable with the args. However, I'm not sure, but I think some integration may need to launch a game through a launcher (e.g. startclient://launchgame/{game_id}) but the launcher doesn't have any time tracking capabilities. The launcher, however, does report game status (not game time) or the integration is capable of looking at running tasks and see if the game is running.

I see, valid point.

Because Galaxy's time tracker is smarter than a start-stop game time tracker, I think the best case would be to implement both approaches but not necessarily at the same time (...)

The less is better, but in that case you're probably right

Sorry if I'm wrong, but isn't GameTime returned from get_game_time?

Yes, I was referring to get_game_time, my mistake.

But after the first time in a session of GOG Galaxy, if the integration has reported tracking game time by status, then we can safely not call it again for the specific game.

True. Still, there is some kind of order-wise logic layer in Galaxy required. And once-per-session does not solve a plugin state problem as user data should be consistent between sessions, even with 2 running plugin instances. For example after plugin update, if it switches from "plugin-track" solution to "track-by-Galaxy" Galaxy backends should be prepared to store game time sessions (additive game times) in addition to current solution (overridden game times) that will be applied to case of switching from "track-by-Galaxy" to "plugin-track". Maybe those are edge-cases but better to create simple, but robust solution to avoid future problems.

Edit: Kind of unrelated, but allowing the integration to store and receive a very small JSON object in the cloud would be a pretty cool additional feature that could be very useful. It would first become a bandage for the issue I described in this thread and could also allow keeping the user signed in across devices if applicable similar to how the official Epic Games integration does it.

That is another nice feature, we just need to know why it should be done before other cool features. Isn't it workaround for other problems? What it gives to plugin authors? How many users will benefit? Storing foreign platforms credentials is a security risk that need much more protection than storing them in local database, not speaking about additional resources for backend traffic. Feel free to create separate issue for this.

urwrstkn8mare commented 3 years ago

That is another nice feature, we just need to know why it should be done before other cool features. Isn't it workaround for other problems? What it gives to plugin authors? How many users will benefit? Storing foreign platforms credentials is a security risk that need much more protection than storing them in local database

Other than just a temporary workaround, that was the main idea: storing foreign platforms credentials. I realise there need to be security and traffic capacity considerations. Sorry about that I think that feature shouldn't be at all a priority.

Maybe those are edge-cases but better to create simple, but robust solution to avoid future problems.

Totally agree.

Thanks @mbanczerowski, for your input in this issue I really appreciate it.

AndrewDWhite commented 2 years ago

A potential untested workaround solution to allow for a plugin to determine the playtime between two computers would be to use the total time and timestamp stored in the galaxy database. Assuming the local galaxy database is up to date on both machines when it is time to update or retrieve this value, a plugin developer can pull this information from the galaxy-2.0.db manually and decide how to update it based on the total and past times in the LastPlayedDates and GameTimes database.