GameEngineChecker re-implementation in C##748
Conversation
| private readonly Regex _steamLinkRegex = new Regex(@"store\.steampowered\.com/app/(?<appId>\d+)", RegexOptions.Compiled); | ||
| private readonly Regex _wikipediaLinkRegex = new Regex(@"wikipedia\.org/wiki/(?<pageName>[^/]+)", RegexOptions.Compiled); | ||
|
|
||
| public Task<Uri> GetLink(Game game, CancellationToken cancellationToken) |
There was a problem hiding this comment.
This method intentionally returns Task. It should make it easier to add a dependency on some Steam API service that would help generating a link with Steam App Id.
| try | ||
| { | ||
| var request = new HttpRequestMessage(HttpMethod.Get, link); | ||
| request.Headers.TryAddWithoutValidation("User-Agent", "Playnite.GameEngineChecker Extension 3.x (https://github.com/darklinkpower/PlayniteExtensionsCollection/)"); |
There was a problem hiding this comment.
Take note, that User-Agent is now required as per https://www.pcgamingwiki.com/wiki/PCGamingWiki:API#Requirements
I was testing with a link to my github, but for this merge it should point to your repo.
| var responseString = await response.Content.ReadAsStringAsync(); | ||
| _logger.Debug($"Response from PC Gaming Wiki: Status: {response.StatusCode}; Body {responseString}"); | ||
|
|
||
| response.EnsureSuccessStatusCode(); |
There was a problem hiding this comment.
In all my testing had not received 429 HTTP code. My rate limiter seems to work.
| using System.Threading.Tasks; | ||
|
|
||
| namespace GameEngineChecker.Services | ||
| { |
There was a problem hiding this comment.
This basically works by counting request count per 'window'. In our case it's 30 per 60 seconds: https://www.pcgamingwiki.com/wiki/PCGamingWiki:API#Requirements
It also has batchSize parameter, that indicates the total number of requests we intend to do. If our batchSize is larger than 30 (max per 60 seconds), then we intentionally spread out the requests evenly within the 60 seconds.
Otherwise we don't slow down requests, unless somehow we hit the limit.
This means, that huge amount of games get processed slowly but steadily, while small amounts get processed immediately.
| private static readonly TimeSpan PcGamingWikiRateLimitWindow = TimeSpan.FromSeconds(60); | ||
| private static readonly ILogger Logger = LogManager.GetLogger(); | ||
| private readonly Tagger _tagger; | ||
| private readonly RateLimiter _rateLimiter; |
There was a problem hiding this comment.
Both Tagger and RateLimiter are fields and are supposed to work as singletons (-ish). They both have semaphores to ensure only one action is performed at the time.
Tagger- it is modifying Playnite database (adding new tags). I had an issue in the past, where letting two Tasks to modify the DB would end up in creating duplicate Tags.RateLimiter- this one protects us from abusing PCGW API. That is why no matter how many processes run, I want to ensure we adhere to PCGW limits, hence this is global.
| _enginesParser = _fixture.Freeze<IEnginesParser>(); | ||
| _tagger = _fixture.Freeze<ITagger>(); | ||
| _sut = _fixture.Create<GameEngineCheckerService>(); | ||
| } |
There was a problem hiding this comment.
I love AutoFixture and FakeItEasy (or Moq) because they make it easy to do TDD. But it's ok, if you want to ditch AutoFixture.
|
|
||
| namespace GameEngineChecker.Tests.Services | ||
| { | ||
| public class RateLimiterTests |
There was a problem hiding this comment.
These tests are a bit counterintuitive. It's all because how rate liming window works. Basically I get 2 bursts if I am cancelling after the rate limiting window duration (it manages to squeeze in 2 windows, because there are tiny delays before cancellation happens).
|
|
||
| namespace GameEngineChecker.Tests | ||
| { | ||
| internal class TestableItemCollection<T> : IItemCollection<T> where T : DatabaseObject, new() |
There was a problem hiding this comment.
This is a helper class I use to test various Playnite Database collections (Tags in this case). It is intentionally incomplete, since I only want to implement minimal code my extensions use or something needed for test setup.
Sadly Playnite's ItemCollection is not public, so I cannot use it ;(
|
Hello, thanks for the hard work. Rewriting the extension has been on my TODO list for a long time, it was one of the first extensions I made when I was just starting to learn some programming with PowerShell a few years ago. If you're okay with it, I think it might make sense for you to take over the extension now that you've rewritten it. What do you think? It's been difficult working on and maintaining so many addons including themes and extensions by myself so if you could take this one over it would be a huge help. I also think it's fair to say this is your extension now since you were the one who rewrote it. Let me know what you think. If you agree it would just be a matter of updating the information in the extension manifest in the addons manifests repository. Just let Crow know about it and send the PR with the updated information. |
|
It's probably more reasonable for me to take over, rather than dump all this code on your lap. In a way I feel bad taking away one of your gems, but at the same time I can feel your pain with the amount of extensions you're handling. Sometimes my 4 extensions feels like it's too many 😄 . And then there's P11... Though I would like to keep you name as a co-author in the extension manifest. I hope it is ok. I will also understand, if you want to put some distance to avoid getting questions, then I could drop your name. There are a few things I will still have to do (like setting up the repo/pipeline, setting up translations, etc.), and I want to rest my eyes a bit, so I will probably start the move towards the end of the week. |
Thanks, I appreciate the gesture. More than wanting to put distance or avoid questions, I think it's more fair to only keep your name since you are the one who worked on it and will be maintaining it. I don't work on these extensions for ego or recognition, rather only to give to the community, so in my opinion it's perfectly fine and better to only keep your name and information. |
2 participants
I have verified that:
masterbranch.Why?
GameEngineChecker is one of my favourite extensions. Because I was missing it so much, I've done some tests regarding broken Steam API calls, and in general how it worked and decided to give it a go. Addresses #715
What's different
Behind the scenes
Most of this was written using TDD (hence usage of AutoFixture and FakeItEasy). I don't think this has a single line of AI code, since I wrote it for fun.
Testing
I've done quite a few test runs scanning my whole library. Did few adjustments after running into bugs or unexpected behaviours. I feel confident it's stable and performs well.
Sorry
I realize that this is a huge pull request. And that it might not be fun to take on so much someone else's code. I've rewritten it for myself, so I could use it again. And I wanted to share it with you, since I tried to write fairly well written code :}. Not sure what's the best path forward. I'm also 100% ok, if we do not merge this :).
Thanks!
Thank you for all the amazing work you do! Playnite is so much better because of you!