UncleGoogle / galaxy-integration-humblebundle

Humble Bundle integration for GOG Galaxy 2.0
GNU General Public License v3.0
184 stars 19 forks source link

Multi bundle split fix #157

Closed Gwindalmir closed 3 years ago

Gwindalmir commented 3 years ago

There are many bundles, such as the Deep Silver Bundle, which include the word "and" in the title string for the multi-game key bundle. One example is: Metro 2033, Risen, and Sacred Citadel This caused Galaxy to add a game called "and Sacred Citadel" to my list.

I added an extra check in the split code to check for the word "and", in lowercase. From what I've seen, that title isn't localized, and is always in English.

There's a second minor fix in there, which is for the blacklist. The Star Wars games recently had their titles changed when Disney bought the franchise and started releasing updates again. Battlefront and Battlefront II are examples. The current blacklist is case sensitive. My fix is to perform a case insensitive comparison of the blacklist.

Gwindalmir commented 3 years ago

Hi @Gwindalmir, thanks for the PR!

There are many bundles

Any other example? I'm asking because the matching algorithm in GOG works only for the first-time ever import. Once the plugin import game name such as "and Sacred Citadel" then it has to be fixed manually by sending ticket to GOG. So if it is one-time only accident, then maybe it is not worth to add. Otherwise lets merge it and hope Humble stuff uses , and and not forgets the coma...

Yes, there were several games that tripped up on that, at least two. My library is all corrupted now though, I have games that don't appear anymore (like Operator). Here's what shows in the logs:

However, the only other one I see in my library currently is and Total War: ARENA Beta Access Key.

I've commented on some minor things to change. Please rebase to keep this 2 separate commits for 2 logical changes (I'll fast forward it when merging) or put the blacklist fix in separate PR.

Uh what? I'm not very good with git.

One more think is maybe to add another "split" test(s) to ensure there is no false positives? Having parametization for names (and expected results) like "game A, Ori and Blind Forest", "Forever, and ever" (welp, we've already agreed on such side effect counting on GOG to spam "ever"), "game A, and game B, and game C" (to show the whole logic., "game A, And another request 2" (to show immune to capitalized text) ect.

I can do that.

UncleGoogle commented 3 years ago

Thanks for examples. Indeed that is a bigger problem.

However, the only other one I see in my library currently is and Total War: ARENA Beta Access Key.

Unfortunately they have to be fixed by GOG stuff manually. So to recover those titles we have to write ticket in their mantis bug tracker.

I've commented on some minor things to change. Please rebase to keep this 2 separate commits for 2 logical changes (I'll fast forward it when merging) or put the blacklist fix in separate PR.

Uh what? I'm not very good with git.

No worries, you can ignore this request, its ok however you do that.

One more think is maybe to add another "split" test(s) to ensure there is no false positives? Having parametization for names (and expected results) like "game A, Ori and Blind Forest", "Forever, and ever" (welp, we've already agreed on such side effect counting on GOG to spam "ever"), "game A, and game B, and game C" (to show the whole logic., "game A, And another request 2" (to show immune to capitalized text) ect.

I can do that.

👍

UncleGoogle commented 3 years ago

Hi @Gwindalmir, kind reminder on your PR if you would like to finish.

Gwindalmir commented 3 years ago

Hi @Gwindalmir, kind reminder on your PR if you would like to finish.

Thanks, I haven't had a lot of time to wrap this up. I'll find some time this week to do that.

Gwindalmir commented 3 years ago

Ok, I think I did everything you asked, include the rebase. Sorry for the delay in wrapping this up.