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

Add CONTRIBUTING.md. #108

Merged
merged 15 commits into from
Oct 2, 2023
Merged

Add CONTRIBUTING.md. #108

merged 15 commits into from
Oct 2, 2023

Conversation

CosminPerRam
Copy link
Member

@CosminPerRam CosminPerRam commented Sep 30, 2023

Adds initial CONTRIBUTING file with some basics I came up with.
More importantly, also the naming of the new games id system (suggested one).
I'm very open to discussions regarding anything and everything in this file.

@CosminPerRam CosminPerRam added the repository Repository changes. label Sep 30, 2023
Copy link
Collaborator

@Douile Douile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice work, especially on the naming rules, I'm not sure how you managed to get them quite so concise.

CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@Douile
Copy link
Collaborator

Douile commented Oct 2, 2023

It might also be worth adding rules for the "full name" of games e.g.

Pretty game names should be added as they appear on steam (or other storefront if not listed there) with the release year appended in brackets (except when the release year is already part of the name). If the regular game doesn't support game queries but a mod does append the mod name after the game name but before the release year separated from the game name using a - (e.g. Grand Theft Auto V - FiveM (2013)).

@CosminPerRam
Copy link
Member Author

CosminPerRam commented Oct 2, 2023

Great catch, generalized it a bit.

Copy link
Collaborator

@Douile Douile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I mentioned in discord we might also want to add a rule about using mod names as IDs when the base game doesn't support queries.

CONTRIBUTING.md Outdated
Comment on lines 60 to 63
7. If a game has multiple id entries (by specifying the edition (Minecraft:
Java and Bedrock) or the release version (Counter-Strike 1: 1.5 and 1.6)), one
more entry by the base name (`minecraft` and `counterstrike` respectively) can
be added, where it queries in a group said included entries.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mentions the case for the auto query but doesn't tell us what to do for individual protocols. Should we stop adding ids for individual protocols e.g. minecraftjava?

Or should that already be included as a word anyway. I have been leaving out because the protocol is wrapped in (), which is similar to how the year is extra info, but games could included () in their titles anyway.

Perhaps adding the protocol should be specified in name rules.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modified, what do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's good for the ID I think, we should also mention whether the protocol is to be added to the pretty name though.

CONTRIBUTING.md Outdated Show resolved Hide resolved
Douile added a commit to Douile/rust-gamedig that referenced this pull request Oct 2, 2023
An attempt to implement the rules specified in gamedig#108 as a programmatic
test.
@CosminPerRam CosminPerRam merged commit e7567c6 into main Oct 2, 2023
1 check passed
@CosminPerRam CosminPerRam mentioned this pull request Oct 4, 2023
Douile added a commit to Douile/rust-gamedig that referenced this pull request Oct 7, 2023
An attempt to implement the rules specified in gamedig#108 as a programmatic
test.
Douile added a commit to Douile/rust-gamedig that referenced this pull request Oct 17, 2023
An attempt to implement the rules specified in gamedig#108 as a programmatic
test.
Douile added a commit to Douile/rust-gamedig that referenced this pull request Oct 21, 2023
An attempt to implement the rules specified in gamedig#108 as a programmatic
test.
Douile added a commit to Douile/rust-gamedig that referenced this pull request Oct 28, 2023
An attempt to implement the rules specified in gamedig#108 as a programmatic
test.
@cainthebest cainthebest deleted the contributing branch November 8, 2023 22:08
Douile added a commit to Douile/rust-gamedig that referenced this pull request Dec 12, 2023
An attempt to implement the rules specified in gamedig#108 as a programmatic
test.
CosminPerRam pushed a commit that referenced this pull request Dec 19, 2023
* [Test] Add best effort test to validate game ID rules

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

* [Test] Refactor ID rules to check if a mod exists after - in game name

This allows fivem to pass the check following rule 8, but could also
cause a false pass in some cases.

* [Test] Add unit tests for ID rule checker

Adds unit tests based on the examples in CONTRIBUTING.md to confirm in
those cases we would allow ID to pass. However these tests don't check
any error cases.

* test/id: Correctly extract protocol names

* games/defs: Fix unreal tournament IDs

* tests: Require game definitions to run ID tests

* tests: Improve comments on ID tests

* tests/id: Combine - seperated numbers

* games/defs: Fix darkest hour ID

* Add/Update badge

---------

Co-authored-by: GitHub Action <action@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repository Repository changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants