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

Remove games.txt and replace it with an in-code solution #407

Merged
merged 6 commits into from
Nov 19, 2023

Conversation

CosminPerRam
Copy link
Member

@CosminPerRam CosminPerRam commented Nov 19, 2023

We would define a game in the games.txt file in the following format: id | pretty name for readme | protocol | options | extra.

Storing this data in a file then reading it turned out to be problematic, there has been many cases where this caused issues, such as using this in NextJS (or using bundlers in general), where you would need this hack around it:

const config = {
    experimental: {
        serverComponentsExternalPackages: ["gamedig"],
    },
};

This was also inefficient, although it would be read only once (and therefore be lazily loaded), this wasn't exactly a good practice and wouldn't provide any realistic advantages instead of just having them stored in a code file.

One advantage (at least, kind of?) would have been readability, as a game was defined in one line, it was easy to see its properties, but this would get janky with longer lines and not exactly the best for contributors.

This PR removes said game definitions file and replaces it with an in-code solution, in theory it will speed up only the first lookup but reduce memory usage (EDIT: actually not really as I think they would be referenced in said array, not deep copied), as before the game could have been found in 2 places, the gamesByKey object and the games array (which this one was used just to automatically generate the GAMES_LIST.md file).

@CosminPerRam CosminPerRam marked this pull request as ready for review November 19, 2023 00:57
@CosminPerRam CosminPerRam merged commit ce4cddb into master Nov 19, 2023
4 checks passed
@Douile
Copy link
Contributor

Douile commented Nov 19, 2023

It would be nice if the changelog mentioned the removal of GameResolver. Also It's removal will require updating the typescript type definitions: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/gamedig/lib/GameResolver.d.ts

@CosminPerRam
Copy link
Member Author

Sure thing, removal of GameResolver has been added to changelog in f472d8b0e4d39415daf98c49232c153a5aaf9431.

CosminPerRam added a commit that referenced this pull request Dec 3, 2023
@CosminPerRam CosminPerRam deleted the feat/games_txt_replacement branch January 18, 2024 22:41
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.

2 participants