High frequent file search refreshing game state every second

[jneumaier]

My drives show read access on plenty of uplay_install.state files all the time forever with GOG Galaxy started.

Status for all games is updated in an endless loop with a sleep time of 1 seconds between executions.

galaxy-integration-uplay/src/local_game_status.py

Line 183 in 41cd311

time.sleep(1)

In each execution the game installation and running state is updated. While the running state is retrieved with rather cheap mechanisms, the game installation status update requires disk access for each game.

I think it would be better to update the game installation status less frequently, at most one time every minute.

Maybe the GameStatus.Installed could be cached in each game object and only be updated if a boolean flag is passed to this method:

galaxy-integration-uplay/src/local_game_status.py

Lines 157 to 169 in 41cd311

	def _get_game_status(self, game, line_list):
	status = None
	if game.type == GameType.Steam:
	status = get_steam_game_status(game.path)
	else:
	if not game.path:
	game.path = get_local_game_path(game.special_registry_path, game.launch_id)
	status = get_game_installed_status(game.path, game.exe, game.special_registry_path)
	if status == GameStatus.Installed:
	if self._is_game_running(game, line_list):
	status = GameStatus.Running
	return status

Not sure about the implementation but this update mechanism interval does not seem reasonable to me. I can prepare a PR if you agree to my finding.

[FriendsOfGalaxy]

I agree that this is not optimal. PR welcomed.

But instead of managing timeout I suggest to stat this file often (like every second or a few seconds), but read it only if it has changed.

Like here:
https://github.com/FriendsOfGalaxy/galaxy-integration-epic/blob/master/src/local.py#L40

or in batch like here:
https://github.com/FriendsOfGalaxy/galaxy-integration-origin/blob/master/src/local_games.py#L281

This way we make it lightweight for drive and still keep a nice feature that is quick detection just after you installed a uplay game (that is desirable in case you install it though Galaxy).

[FriendsOfGalaxy]

@jneumaier are you still interested in making PR? What do you think about my suggestion?

[jneumaier]

@FriendsOfGalaxy I like your idea. Currently I have no time for a PR, will try to find some in the future.

The stat comparison might be done here:

galaxy-integration-uplay/src/local_helper.py

Lines 74 to 87 in 41cd311

	def _read_status_from_state_file(game_path):
	try:
	if os.path.exists(os.path.join(game_path, 'uplay_install.state')):
	with open(os.path.join(game_path, 'uplay_install.state'), 'rb') as f:
	if f.read()[0] == 0x0A:
	return GameStatus.Installed
	else:
	return GameStatus.NotInstalled
	# State file doesn't exit
	else:
	return GameStatus.NotInstalled
	except Exception as e:
	log.warning(f"Issue reading install state file for {game_path}: {repr(e)}")
	return GameStatus.NotInstalled

[FriendsOfGalaxy]

Yes, we have to decide where to store previous stats. This function is used here: https://github.com/FriendsOfGalaxy/galaxy-integration-uplay/search?q=get_game_installed_status

• in process watcher (why?)
• in local games parser

Looks like un-spaghetting candidate..

One way to do it cleanly would be to create "file watcher" class that do only this:

galaxy-integration-uplay/src/local_client.py

Lines 112 to 129 in 41cd311

	def ownership_changed(self):
	path = self.ownership_path
	try:
	stat = os.stat(path)
	except TypeError:
	log.warning('Undecided Ownership file path, uplay client might not be installed')
	self.refresh()
	except FileNotFoundError:
	log.warning(f'Ownership file at {path} path not present, user never logged in to uplay client.')
	self.refresh()
	except Exception as e:
	log.exception(f'Stating {path} has failed: {str(e)}')
	self.refresh()
	else:
	if stat.st_mtime != self.last_modification_times:
	self.last_modification_times = stat.st_mtime
	return True
	return False

Other way (more functional) - gether all files stats in one step and then pipe them to another function like get_game_installed_status.

To double check - I just wrote this without much investigation.