DavidoTek / ProtonUp-Qt

Install and manage GE-Proton, Luxtorpeda & more for Steam and Wine-GE & more for Lutris with this graphical user interface.
https://davidotek.github.io/protonup-qt
GNU General Public License v3.0
1.17k stars 39 forks source link

Heroic: Correctly display Browser games in Games List #272

Closed sonic2kk closed 11 months ago

sonic2kk commented 11 months ago

Overview

Heroic v2.9.0 added a feature for adding Browser games to your library. This works by giving the game a name, path, and a URL. Heroic will then run this game in its Browser, which is Chromium (with a spoofed browser agent to act like Google Chrome afaik).

ProtonUp-Qt can display these, but this PR fixes up their entries to make them a bit more useful.

Main branch: image

This PR: image

Background

Browser games are treated internally like sideloaded apps, with their information stored in the sideload_apps.json file. They also still have an app_name. So there were no code changes required to get Browser games to display. Right now, ProtonUp-Qt will display these browser games in the Heroic Games List, but with a couple of problems:

  1. Compatibility Tool is listed as "Not Found", because Browser games actually have a Wine field but the Wine name in the Heroic JSON file is "Not Found" - Not sure why they store it like this, because there is no option (currently at least) to run a game in a browser with Wine or anything like that.
  2. Install Location is listed as ".", because sideload apps have a folder_name key to store their installation folder name, that is actually what the folder_name is in the JSON.

So the problems here are actually that ProtonUp-Qt is reading the information properly for Browser games, but that the information is not really useful for Browser games.

Implementation

To make Browser game listings a bit more useful, a few changes were made:

Properly Display Browser "Compatibility Tool" Name

Check on the HeroicGame.platform string when setting the "Compatibility Tool" name. This was not done initially out of concern that it may not be as reliable as checking for Wine information to differentiate between a Windows and native title. But now that we have Browser titles, I think it's safe to check on the platform string.

When we're using Windows we still build the compatibility tool string the same as normal, but in the else block, instead of hardcoding to "Native" we now set the game.platform string. This does mean that native titles will say "Linux" instead of "Native" now, but I don't think that's a big problem, and it saves on conditional logic to check "if platform is linux then set string and tooltip to Native". Doing it with the platform string is much more straightforward.

Use browserUrl for the Install Location

The Install Location column has a bit of logic to try its best to find the Heroic installation path if we can't find it. But for Browser games we know this won't be set at all, so instead, when we're building the list of Heroic games with get_heroic_game_list, we check for browserUrl (set properly for Browser games and non-existent for all other sideload apps) before we check for folder_name (set to "." for browser games and because it has a value, it would be truthy, so the check for browserUrl has to go before it, since that will be falsey for non-Browser sideload games).

Double-Click action for Browser Games

When double clicking the "Install Location" for sideload/gog/legendary games, if the path exists, it will be opened in the file browser with xdg-open. But since the Browser URL set above doesn't fails at the os.path.isdir check, double-clicking would do nothing because we don't set the data on the list item.

To fix this, I added an optional parameter ignore_invalid_path to set_item_data_directory which is used to call setData on the list item passed to it even if the path doesn't exist. The default value for this parameter is False.

Amending Tooltips

For the Compatibility Tool, as I hinted at earlier, the tooltip has been adjusted from saying Platform: Native to Platform: {game.platform}. It uses the platform string instead of being hardcoded.

For the Install Location, the tooltip was updated from saying Double-click to browse to Double-click to open in browser, to more accurately reflect what it does.


That sums up the changes and rationale here. All feedback is welcome on this :-)

Thanks!

DavidoTek commented 11 months ago

Compatibility Tool is listed as "Not Found",

They probably didn't want to break existing implementations by removing the wine field

set properly for Browser games and non-existent for all other sideload apps

Interesting, here the field browserUrl won't exist for some entries. Luckly, if they should ever add it to all entries for consistency, the logic will still work as Python's or handles '' that same as None ;)


Thanks! :tada:

sonic2kk commented 11 months ago

Something that occurred to me when addressing the translation comment, I wonder if we should have another enum for Heroic platform types? Something like:

class HeroicPlatform(enum):
    WINDOWS = 'Windows'
    LINUX = 'Native'  # Or 'Linux', no preference here :-)
    BROWSER =  'Browser'

Then we can check on game.platform == HeroicPlatform.WINDOWS and so on instead of a "raw" string comparison. Probably doesn't make much of a difference, just a thought I had.

DavidoTek commented 11 months ago

I wonder if we should have another enum for Heroic platform types?

That seems like a good idea. Feel free to add it if you want.

Otherwise I think this is ready to merge. Thanks.

sonic2kk commented 11 months ago

Did a bit more testing and it would be more involved than I thought, so it can be merged as-is and refactored later :-)