Skip to content
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

feat: Add game type alias #487

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

podrivo
Copy link
Contributor

@podrivo podrivo commented Jan 18, 2024

This adds an alias for GIDs, so it will work for the official current supported GID and also previous or new GIDs.
Got this idea from this issue #465.

Noticed that Hidden and Dangerious2 had the GID updated, but also a few others had the GID updated as well (see PR #415), such as Age of Chivalry, Action: Source, Ark: Survival Evolved, Left 4 Dead etc. This will enable backwards compatibility for those previous GIDs.

Also, should I include alias GIDs on lib/games.js and update GAMES_LIST.md on this PR or should I open another?

Let me know if this makes sense. Thank you! (:

Example:

hiddendangerous2: {
  name: 'Hidden & Dangerous 2',
  alias: 'had2',
  release_year: 2003,
  options: {
    port: 11001,
    port_query_offset: 3,
    protocol: 'gamespy1'
  }
}

@CosminPerRam
Copy link
Member

CosminPerRam commented Jan 18, 2024

This is definitely something that has to be done, thanks.

Also, should I include alias GIDs on lib/games.js and update GAMES_LIST.md on this PR or should I open another?

Would advocate to do so here, so we are sure we cover every possible problem that could appear while doing so.

I wouldn't name this field as alias, I for one think oldId would be more appropriate to what it really is.

While adding this 'translation' layer, you could find cases where:

  • Newly supported Game has id X
  • Older supported Game with new id is Y but old id is X

And what would we want to do here? Prioritize newly ids or older ones? This'll go multiple ways:

  1. Look only for new ids.
  2. Prioritize new ids, if none is found, look for the old ones.
  3. Prioritize old ids, if none is found, look for the newer ones.
  4. Look only for old ids.

We'll have to decide the default behaviour, also this 'translation' layer could (and I guess will) be removed after a new major version (v6).

@podrivo
Copy link
Contributor Author

podrivo commented Jan 18, 2024

Nice! I'll start adding GIDs from previous commits to this PR, so we keep everything together! (:

About the name, I'm not strongly attached to alias, but I would argue that this is not time based, as in new/old, but only as a different name format. More as like how users would use and less like how we maintain it.

Almost like having a full and a short name. So for example: Counter Strike 2, the full main format would be counterstrike2 and the short format would be cs2. Users would probably default to use cs2 as it's shorter and easier to remember, but for us maintaining it, counterstrike2 would make more sense and fit more inside our guidelines (which we should also make sure it's up to date to the new commits).

This would exempt us from thinking about new/old and prioritization when looking for GIDs, and it would work in all major versions, even if we decide to change our naming guidelines. Which would probably mean that we could have more than one alias in the future.

Let me know what you think!

@podrivo
Copy link
Contributor Author

podrivo commented Jan 18, 2024

I'm late to the party overall, and so I'm just putting everything together and checking issues and pulls around.
So I was checking this #374, this #415 and this #435.

Will consider these links as well, and think this through!

@CosminPerRam
Copy link
Member

Let me know what you think!

I agree that cs2 is nicer to remember and type rather than counterstrike2, but, as stated partially in #415:
At the end of the day the ids dont matter that much, as if one service wants to utilize this library and would let the user choose the game that they wanna query, they wont let them choose ids, they'll choose names (why would they choose ids?), so what exactly is the id it doesn't really matters here.
Could be totally wrong about this.

As for having a 'short name', it could be done, but I don't really see much value in having this and maintaining it.

@podrivo
Copy link
Contributor Author

podrivo commented Jan 19, 2024

Considering everything, I think this is just a minor detail in the big scheme of games ids. I do like the idea of being able to use cs2 as well as counterstrike2. There's just something that bugs me having csgo, css but counterstrike2. It feels off haha. And I think for some users it will be more intuitive having the social/cultural name. I'm sure there's more examples.

But for now, let's focus on the other open discussions, and when it's time we can talk more about it. I don't want to bring even more things to consider, so I'll close this for now, as it's all linked and we can reference later.

Thank you! (:

@podrivo podrivo reopened this Feb 10, 2024
@podrivo podrivo marked this pull request as ready for review February 10, 2024 00:19
@podrivo
Copy link
Contributor Author

podrivo commented Feb 24, 2024

It would be good to have more insight on how to proceed about this implementation! (:

@a-sync
Copy link
Contributor

a-sync commented Mar 2, 2024

Suggestion: merge the old id check and the alias check into checkAliases option for example. Keep a single field in the games list like extra.aliases and make it a (string or) string array.

Could have both old ids and aliases and would make it extendable.

p.s. unless there are conflicting old/new IDs i don't see a reason why not just check for all possible values and loose the option, its not that big of an overhead imo

@podrivo
Copy link
Contributor Author

podrivo commented Mar 2, 2024

@a-sync Thanks for joining! ✨

That's the original idea, but @CosminPerRam has a different view about how to deal with this.
I was waiting for his direction on how to proceed.

@CosminPerRam
Copy link
Member

unless there are conflicting old/new IDs i don't see a reason why not just check for all possible values and loose the option, its not that big of an overhead imo

It indeed isn't a big overhead, but we cant guarantee that eventually we wouldn't be adding a new id that conflicts with the old ones, thus breaking old behaviour, so I think its better to separate them so that the users of the lib are aware that they will eventually be removed.

@a-sync
Copy link
Contributor

a-sync commented Mar 11, 2024

we cant guarantee that eventually we wouldn't be adding a new id that conflicts with the old ones, thus breaking old behaviour,

i disagree on this one. why would you add/allow a new id that conflicts with an existing one?
(its the same issue when (new) games have conflicting acronyms IDs eg.: tf2)

so I think its better to separate them so that the users of the lib are aware that they will eventually be removed.

this can be done gradually by separating old / alias fields and deprecating the old IDs, before removing them completely eventually (eg. major version change)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants