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

GLK: ADRIFT: Add detection for new games (B-F) #4797

Closed
wants to merge 5 commits into from

Conversation

MarcoBorrini99
Copy link
Contributor

GLK: ADRIFT: Add detection for new games

This commit add/fixes entries from the following sources:

This PR can already be merged.

@@ -35,10 +35,14 @@ const PlainGameDescriptor ADRIFT_GAME_LIST[] = {
{ "30seconds", "30 Second" },
{ "3monkeys", "Three Monkeys, One Cage" },
{ "achtung", "Achtung Panzer!" },
{ "adrbedlam", "Bedlam" },
{ "adrbluesky", "Blue Sky" },
{ "adrcrimescene", "The Crime Scene" },
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency, these entries should have "adrift" after the gamename:
bedlamadrift , blueskyadrift etc.
You could also change the "asylum" entry to "asylumadrift"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. I'll fix when I'll add another commit to this PR for [D - F] block

Copy link
Contributor

Choose a reason for hiding this comment

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

You could also do the changes, stage the file and commit --amend
Then force push to your branch and the PR will update automatically

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really don't want to make a mess here because I'm still learning git!

@@ -695,8 +888,8 @@ const GlkDetectionEntry ADRIFT_GAMES[] = {
// ADRIFT One-Hour Game Competition 3
DT_ENTRY0("1hgforum2", "5a534ac4e39a319022d145094c46930a", 11185),
DT_ENTRY0("1hgcrm", "d97d1ff8f01a61fb477b76df65c77795", 15432),
DT_ENTRY1("1hgasdfa", "Competition Release", "fccb2fb890d554263d5f55bc02220ab8", 6440),
DT_ENTRY1("1hgdemonhunter", "Competition Release", "ca37aaf35fb15a40a7f5f8caa1475112", 4169),
DT_ENTRY1("1hgasdfa", "Comp Rel", "fccb2fb890d554263d5f55bc02220ab8", 6440),
Copy link
Contributor

Choose a reason for hiding this comment

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

Not really a fan of this change. "Comp Rel" is not really clear, I'd keep "Competition Release".
Same thing for the other entries, I'd rather use "Release" instead of just "Rel"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tag2015 PR updated. Can it be merged now if ok?

@MarcoBorrini99 MarcoBorrini99 changed the title GLK: ADRIFT: Add detection for new games (B-C) GLK: ADRIFT: Add detection for new games (B-F) Mar 12, 2023
@tag2015
Copy link
Contributor

tag2015 commented Mar 13, 2023

I haven't had the time to look at the new entries yet, but I'd like to mention two things:

  1. You really should rebase your branch, so the "merge" commits go away, otherwise we need to merge the commits manually or squash which is really not ideal. If you don't know how to do it ask on Discord and someone will help
  2. Make sure that all commits build properly. In this PR there are two broken commits and the follow-up "fix" commit, which is something to avoid if possible

@MarcoBorrini99
Copy link
Contributor Author

I haven't had the time to look at the new entries yet, but I'd like to mention two things:

  1. You really should rebase your branch, so the "merge" commits go away, otherwise we need to merge the commits manually or squash which is really not ideal. If you don't know how to do it ask on Discord and someone will help

Ok, I'll try to join Discord channel

  1. Make sure that all commits build properly. In this PR there are two broken commits and the follow-up "fix" commit, which is something to avoid if possible.

Of course, I only missed once to check the building and that time was faulty...

@tag2015
Copy link
Contributor

tag2015 commented Mar 15, 2023

Thanks a lot for your work! Merged manually

@tag2015 tag2015 closed this Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants