Closed claushofmann closed 3 years ago
@claushofmann please take a look on my suggestions after code review.
@FriendsOfGalaxy I have incorporated your suggestions into the pull request.
@claushofmann did you check if extended id with @
is required only to trigger launching? Do install_game
, uninstall_game
get_local_size
ect. operate normally?
uninstall_game
and get_local_size
seem to work fine (if I am not mistaken, uninstall_game
only opens the Windows uninstallation console anyway). I unfortunately wasn't able to test install_game
though.
I'll test install_game
in that case.
I'm thinking about passing the whole new id to galaxy in owned_games
interface and then cut part before @
when we need it. This way we don't need cache, what could be cleaner solution (and do need to think about edge cases, like trying to play game that was synced by Galaxy from different machine where the plugin cache is not populated yet...)
I'll test
install_game
in that case.
Thanks.
I'm thinking about passing the whole new id to galaxy in
owned_games
interface and then cut part before@
when we need it. This way we don't need cache.
I agree that this would be a cleaner solution, and if I understand you correctly it was also my first approach when implementig it. However, when I was testing it, Galaxy presumably had some problems with the @
ending, and didn't display the cover art. I don't know what the reason for this is though.
Galaxy has some problems with newly created ids. But they should be finE after some time. From my experience most often only developer sees the "no cover arts". Or users of some niche plugins as well.
Ah, okay. In this case I think it makes sense to move forward with the approach of passing the game ID with the @
to Galaxy. I was wondering anyway how Galaxy handles cover art. I assumed that it is somehow using the game ID and grabbing it from somewhere in the internet.
Ok, so I can share my findings:
externalType: SUBSCRIPTION
does not work when trying to install them with extended id. Exactly the same prompt about "unknown licence game" is shown as you've posted in the linked issueexternalType: SUBSCRIPTION
- don't know yet, I'm downloading one...entitlementSource
which has values like STEAM
, EPIC
, ORIGIN-STORE-WEB-RU
but I'm not sure about using this. Sims4 have "entitlementSource" : "Ebisu-Platform"
and do not have externalType
key.firstParties
under one Origin offer ID. It is a list of ids from external partners; for Jedi Fallen Order it is 3 Epic and 2 Steam ids (for steam https://steamdb.info/app/1172380/ and https://steamdb.info/app/1179140/). Conclusion: you can buy the same origin offer id from different sources. So it makes sens to have separate id sending to Galaxy. On the other hand it is messing up with ids: it was all about offerId so far and now we would include some other infomation in game_id
. So now I don't know what is better - messing with ids in gog database or just using the cache as your PR ver1.We may not know more edge cases. Maybe the safest option would be to:
ExternalType(enum.Enum)
with STEAM and EPIC (anything else that is known?)game_id
and use cache to extend it to play with origin client for launch_game
and install_game
. We may reconsider this choice to send remote id to Galaxy when the solution is more mature / we understand origin internals better. This also mitigates issue with not-showing covers for people just after plugin update.What about caching less? Like
["OFR.1231231": {"externalType": "EPIC"}}, ...]
or just externalTypeCache
:
{"OFR.1231231": "EPIC", ...}
Why not all entitlements cache? I remember some issues with loading/sending big cache in Steam plugin and now I'm more cautious about extending cache without strong reason. On the other hand offers cache is twice as long as entitlement.
What do you think?
Update
I've check what origin web does and looks like they actually call
origin2://game/launch?offerIds=Origin.OFR.50.0001454&autoDownload=1&externalType=SUBSCRIPTION
and it works to show installation prompt. So they do not use origin2://game/install
but origin2://game/launch
!
For Jedi Fallen Order it works as well, contrary to what I said about steam game in this comment https://github.com/FriendsOfGalaxy/galaxy-integration-origin/issues/29#issuecomment-776041816
Maybe I've messed something with 2 different accounts.
Wow nice, you really found out a lot. :D
externalType: SUBSCRIPTION
: When does Origin use this? If the game is rented via EA Play? externalType: SUBSCRIPTION
works with
origin2://game/launch?offerIds=Origin.OFR.50.0001454&autoDownload=1&externalType=SUBSCRIPTION
then I think we should also use
origin2://game/launch?offerIds={offerId}&autoDownload=1&externalType={externalType}
instead of
origin2://game/launch?offerIds={offerId}@{externalType}&autoDownload=1
especially given that the former format is also what the Origin website uses. I found the notation using the @
becuase (at least for games from the Epic Game Store) there is a .mfst
file in the games' ProgramData
which is called (for example, in the case of Star Wars Battlefront 2) Origin.OFR.50.0002015@epic
. I also thought about using this for figuring out the external store, but then I found that this info is also in entitlements
, and thought this was a cleaner solution.entitlementSource
field, but don't know what it does. Since Origin is using externalType
in their Origin calls I think we should use externalType
of entitlement
externalTypes
is good, with the downside being that if more external stores are added by Origin, they also have to be added in the enum.entitlement
, I cached the full entitlements in my PR because it is done in a same fashion with offerings
where most of the data in there is not used at all by the plugin. Furhtermore I thought that if someone in the future needed an additional field from entitlement
it could be tedious to update the cache (or does Galaxy delete the cache when a plugin is updated?)
- Regarding
externalType: SUBSCRIPTION
: When does Origin use this? If the game is rented via EA Play?
yes.
- If launching the game with
externalType: SUBSCRIPTION
works withorigin2://game/launch?offerIds=Origin.OFR.50.0001454&autoDownload=1&externalType=SUBSCRIPTION
then I think we should also useorigin2://game/launch?offerIds={offerId}&autoDownload=1&externalType={externalType}
instead oforigin2://game/launch?offerIds={offerId}@{externalType}&autoDownload=1
I agree.
especially given that the former format is also what the Origin website uses. I found the notation using the
@
becuase (at least for games from the Epic Game Store) there is a.mfst
file in the games'ProgramData
which is called (for example, in the case of Star Wars Battlefront 2)Origin.OFR.50.0002015@epic
. I also thought about using this for figuring out the external store, but then I found that this info is also inentitlements
, and thought this was a cleaner solution.
ah, ok, I've just wanted to ask :)
- I have also seen the
entitlementSource
field, but don't know what it does. Since Origin is usingexternalType
in their Origin calls I think we should useexternalType
ofentitlement
yup +1
- I think the idea of using an Enum for all known / supported
externalTypes
is good, with the downside being that if more external stores are added by Origin, they also have to be added in the enum.
yes... but for now I've changed my mind - launching SUBSCRIPTION
types of games works as well, as per how they can be launched by web. Origin has crashed a few times to me, so maybe something else broken at that time. Let me check again.
For now I know at least one case when the game could not start with externalType=SUBSCRIPTION
- when it is owned by other first-party like EPIC and included in EA subscription plan as well. Looks like this limitation was made by EA by design. That is why I think that the plugin should not send those games to Galaxy via get_subscription_games
interface which offerId
AND different externalType
is already included in set of games given by get_owned_games
. Otherwise you can try to run non-working game from galaxy - like from the image below (I own STEAM version)
It is a bit messy to have dependency like that in the code, but I see no choice (except of leaving it as is).
- Regarding the caching of
entitlement
, I cached the full entitlements in my PR because it is done in a same fashion withofferings
where most of the data in there is not used at all by the plugin. Furhtermore I thought that if someone in the future needed an additional field fromentitlement
it could be tedious to update the cache (or does Galaxy delete the cache when a plugin is updated?)
No, cache is not removed on plugin update. You've convinced me - let us leave it as is.
Let me merge your change then. I'll test it more and handle other edgecases before release, to not bother you with this PR anymore.
Thanks!
Finally, after longer period of time and refactor problems, I've changed the id to include the externalType. This way detecting local games does not require significant changes. I hope it will behave well.
Fixes problem mentioned in #29