gamedig / rust-gamedig

Game Server Query Library.
https://crates.io/crates/gamedig
MIT License
38 stars 11 forks source link

[Test] Add best effort test to validate game ID rules #111

Closed Douile closed 8 months ago

Douile commented 11 months ago

An attempt to implement the rules specified in #108 as a programmatic test.

Douile commented 10 months ago

The games that currently fail are either wrong (based on current rules as I understand):

Or ambiguous:


Further work (that I won't include in this PR):

CosminPerRam commented 10 months ago

Listed games are indeed wrongly identified. Rule #6 is formed a bit strange, will reword it.

how the protocol version should be encoded into ID is not specified

Well said, will also modify. I will make a PR with these fixes and I will link it here.

Douile commented 9 months ago

I'm not sure on darkest hour there isn't a rule that specifies it should be how we currently have it but the ID we expect is not great.

CosminPerRam commented 9 months ago

Darkest Hour: Europe '44-'45 should be dhe4445:

  1. More than 2 words
  2. [...] If a number is not in the first position, its entire numeric digits will be used instead of the acronym of that number's digits, so for '44-'45 it will be 4445 as I understand it.

Sorry for not coming back to this, I said that I'll try to modify it myself but I ended up not having the right time to put into it.

Douile commented 9 months ago

Ah okay that makes sense. but it's going to be hard to implement I think. Yeah I don't blame you this is a bit spaghetti I should probably refactor.

Douile commented 9 months ago

After a further look at the messy code the reason this is ambiguous is because some games use - in-between words and we count them as different words (Counter-Strike: Global Offensive). Therefore the numbers at the end of Darkest Hour get split into two words.

I guess I could add a band-aid solution of if there are two numbers at the end of the word list combine them, this is a "best effort" after all. The nuance comes down to human interpretation and the code should be changed to match that.

Douile commented 8 months ago

I'm not sure why CI isn't running here.

CosminPerRam commented 8 months ago

CI passes, good to merge?

Douile commented 8 months ago

Assuming you are okay with these assumptions made about IDs:

Also should I remove the node-gamedig test?

CosminPerRam commented 8 months ago

Sounds good to me!