-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
completely rewrite GameCache #28
completely rewrite GameCache #28
Conversation
@Autistic-Unicorn may I once more call you in as tester because of your massive steam library? just let it run and let me know if it breaks and how the performance feels (i didn't touch the achievement import yet, so no improvements there unfortunately) |
Unfortunately I can't test it right now. I'm gonna get into it as soon as I'm gonna have time tomorrow :) |
I tried and it crashed while trying to import games from the old cache |
@Autistic-Unicorn no worries, I'm off to bed already and won't be able to respond within a timely manner anyway^^ @engiefox can you post the log lines containing the exception (preferably including the backtrace)? I'm on my phone atm and cannot open the log files there |
|
I believe that's what you want, the steam log cuts off at the bottom, let me know if you need something else. There's also these lines in the client log: |
Ah, frick. There's a typo in there. If you're willing to monkey patch the files, please replace I'll push a proper fix tomorrow |
No longer crashes, but nothing is syncing. The steam folder is empty and no games show on recent. |
Oof. That's something I cannot fix from my phone. Apparently rebasing to the upstream version of this branch broke more than I anticipated. I'll get a fix out when I'm back from work tomorrow. |
@engiefox @Autistic-Unicorn please try now. my latest commit should fix the errors you reported |
It instantly started to import games into GOG Galaxy, now lets see if it gonna import them all and all other stuff |
There is lots of this error in logs. I think it shouldn't be like that
|
That's "fine". Galaxy tries to import achievements while we're still importing the games list. The errors you see is just us telling Galaxy to wait until the import is done; eventually, these error messages should stop and achievement import continues/starts as normal |
Lmk how it goes and I'll look to merge it in if successful. |
For now it only synced around 2500 games and doesn't do anything more than that. No game time imported, just constantly checks friends and nothing more in logs. It also crashed once without shoving anything wrong. The logs don't show much. It first does a lot of "updating apps for package" and then it stops and nothing else happens (about importing games, there are logs about checking friends info which is still failing to upload into gog)
|
I implemented a very aggressive caching mechanism: for all packages we only update those apps which we do not already have in our cache. That's where the line |
@don-de-marco just ran with your new build, seems to run just fine. synced games from an existing cache and everything. about to run without a cache, one moment |
it hardly spent any time at all syncing with games already in the cache, definitely a lot nicer than waiting for it to finish on every startup. |
i deleted the .db file from C:\ProgramData\GOG.com\Galaxy\storage\plugins but the games are still listed in the app. What the heck? |
That happens if you don't use "Disconnect" for Steam first sometimes. Reconnect and then disconnect and it should work. |
That's expected. Galaxy keeps an internal list of all games we imported so that it can show you your games without the actual plugin being up and running. If you want to get rid of that, click disconnect in the plugin menu and Galaxy will reset that information for you |
I think this build might still have the no 2FA bug. I expressly recall testing that when first getting it working because i was sick of giving steam an email code. But apparently it didn't work, lol. If the fix works on the bugfix branch i can merge them both into the dev branch. If the no 2fa bugfix works I'll have to bump the release to 1.0.7 so we'll need to make sure all our alpha builds are at least 1.1 |
Running without cache also works for games at least. Achieves and playtime aren't syncing, but I'm giving it a moment. |
(technical side note incoming) Now that I'm thinking of it, I might have caused an inconvenience with my current implementation. I should probably remove the |
Not sure why but it got timed out again. I tried logging in twice and same thing happens.
|
when im gonna get some time im gonna get rid of all gog galaxy files and im gonna do a completely clean install. Maybe there is some issue on my side. |
I don't think the issue is on your end. Can you try the commit I just pushed? Maybe it's related to the aggressive caching I introduced |
After 3min it failed the import due to timeout. Happens every time i restart the gog. It looks like the plugin sees the games but none of them get imported into gog.
|
@don-de-marco you may want to squash merge in the changes i had to make to the login process to fix the broken No 2FA option. I'll do a PR on this branch for you. That said, i'm merging it into better proto deps, so maybe it'll just throw up a conflict and i can fix it in merge. Edit: engiefox may have found another bug as a result of fixing this. it won't appear for another ~6 months for the rest of the users, which is why i didn't catch it. i've made the user info cache a little more resistant to bad data, so it should be fixed. wait a moment. |
Tested the 2nd one and it didn't help. Still just the six achievements show. |
Thanks for testing @engiefox! Fix will be out shortly @Autistic-Unicorn can you make an issue for this and attach some logs there? I feel this one needs more thorough investigation |
alrighty, one final set of changes. @engiefox @Autistic-Unicorn if you would be so kind and break it rq :) |
I don't seem to see any issues. What exactly should I expect? |
Hopefully exactly this :'D Do Achievements sync fine still? |
Yes, still the longest task, but it's tolerable. About 5 minutes from completing login to fully synced. |
Game import is probably the most impressive, less than 2 minutes to sync +800 games. |
Thanks. That one gave me the most headaches too :D If @Autistic-Unicorn doesn't dig out some bugs during the afternoon, I'll declare this completed @ABaumher |
Crash after running the plugin for a few hours, crash show on the client log ~line 658 13:31:26 |
I'm hoping the bug engiefox found is just emblematic of our code still needing cleanup overall and not a bug in your code but i'll hold off until you give it a look. I also merged versions 1.0.6 and 1.0.7 into better proto deps (as well as the batch file fix, which i'mo not pushing to Nebula because it'll break the approvals and i don't feel like redoing them. I don't believe there will be any errors as a result of this (no conflicts != no errors at runtime, ofc), but i may be wrong. |
@engiefox honestly, I think you just have too many games installed. From your log I can see that Galaxy wants to import ~800 games. After exactly 20 seconds (and about 650 games done), Galaxy cuts us off because it took too long. I don't see any opportunities on how to improve our code, it's pretty straight-forward already. |
3ad7bbf
to
da373e4
Compare
@ABaumher I rebased this branch onto your current |
Ngl this is basically your branch now. I've unfortunately been dealing with more maintenance that writing any code - and a full-time job that's actually full time now (i miss the time before this project when i was on-call lol) I'll get back to teardown when i can. As for the games installed issue - afaik we can update this in |
The game size import is handled by the plugin, not us. We just provide a means on how to get the game size for a given game id. The rest is out of our hands. |
The timeout comes from Galaxy btw, it's not triggered within our plugin. So as long as we're not able to tell Galaxy that we need more time importing the game sizes, there's nothing I know of that we can do about that |
Feel free to merge this btw. I have no changes pending |
Is fully synchronous. running in executor makes it async and the code wouldn't be blocking. Granted, it might need to be done in the process pool form if this is blocked by GIL |
This code just collects some file paths and is done quickly. The most work is done in |
we're doing a bunch of file opens. Those files are never closed. That seems like it could be causing issues |
While I agree that not closing these files is bad style, it doesn't do us any harm because open files only consume a bit of memory but no CPU power |
No description provided.