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

Feature/updated gamedig implementation #4949

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

Conversation

elonmir
Copy link

@elonmir elonmir commented Jul 17, 2024

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Tick the checkbox if you understand [x]:

  • I have read and understand the pull request rules.

Description

Add new list of games from gamedig lib

Fixes #4414

Type of change

I updated the gamedig dependency and reworked the usage of the lib. The code now takes the gamelist directly and reformats the data structure as expected till now. Furthermore the query method needed to be updated to reflect the change from the library.

Please delete any options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas (including JSDoc for methods)
  • My changes generates no new warnings
  • My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

Screenshot 2024-07-17 at 22 36 49
Screenshot 2024-07-17 at 22 39 11

Please do not use any external image service. Instead, just paste in or drag and drop the image here, and it will be uploaded automatically.

@github-actions github-actions bot added the pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again label Jul 17, 2024
@elonmir
Copy link
Author

elonmir commented Jul 17, 2024

Sidenote, I prepared a feature branch for the current master branch as well, if the PR should be approved and be based on the master branch. Just in case, see https://github.com/elonmir/uptime-kuma/tree/feature/update-gamedig-v2 for the merge conflicts.

@CommanderStorm
Copy link
Collaborator

Hi,
First of all (just that we are clear): Gamedig 5 is definitely a v2.0 only feature.
The reason is first that it is a feature and second the node version compatibility.
I have not looked where the merge conflict notes by the action is coming from, but please resolve that ^^

There is a migration missing to migrate to gamedig v5:

  • Almost all games ids have been changed to follow a standard, see CONTRIBUTING.md#naming.
  • Removed minecraftping (as it was deprecated and the same thing as minecraft) and minecraftpe (deprecated, which is now the same as mbe (Minecraft Bedrock Edition)).

If you have trouble with that, feel free to reach out ^^

@github-actions github-actions bot removed the pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again label Jul 17, 2024
@elonmir

This comment has been minimized.

@elonmir
Copy link
Author

elonmir commented Jul 17, 2024

There is a migration missing to migrate to gamedig v5:

  • Almost all games ids have been changed to follow a standard, see [CONTRIBUTING.md#naming].
  • Removed minecraftping (as it was deprecated and the same thing as minecraft) and minecraftpe (deprecated, which is now the same as mbe (Minecraft Bedrock Edition)).

If you have trouble with that, feel free to reach out ^^

Ah... I see you mean db migration to accomodate the new naming convention?

@elonmir
Copy link
Author

elonmir commented Jul 17, 2024

I just dug in, regarding the migration, seems the only way would be to parse the original games.txt and create a mapping with the correct hyphenation method.

I will take a look tomorrow with a clear head, because that's gonna be a little pita...

@elonmir
Copy link
Author

elonmir commented Jul 18, 2024

Is there any known lib to use for the hyphenated-words? The shortening of multiple words seems very arbitrarily chosen. I am going too try to merge the old and new list via pandas on the full name of the games, hoping that those stayed the same. The alternative would be to introduce a breaking change.

@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Jul 19, 2024

I don't think you need a library for this.

Regarding introducing a braking change:
=> Unless we have to, I am not going to introduce that toil..
=> Please just write out the name before and the name after the migration for those games that changed

@CommanderStorm CommanderStorm added area:monitor Everything related to monitors pr:please address review comments this PR needs a bit more work to be mergable labels Jul 26, 2024
@CosminPerRam
Copy link

CosminPerRam commented Sep 2, 2024

Hey, sorry for bumping in.

The shortening of multiple words seems very arbitrarily chosen.

In v4, the person that did the PR pretty much decided on their own what a new game's id should be.
In v5 we made a rules system for composing them, this way we make sure that we treat all ids equally and that we can test them).

Please see node-gamedig#514 for more info on this.

I am going too try to merge the old and new list via pandas on the full name of the games, hoping that those stayed the same.

They did not stayed the same unfortunately (the main thing here is that we extracted the release year out of the name).

I'm sorry that we have introduced such a big nuisance for the project, I'm looking forward on making the library better in every aspect that it could be done (and I'm still learning as a maintainer on how to manage everything) and thanks a lot for the integration, it's really nice to see it here.

});
}
return gameList;
let gamelist = [];
Copy link
Owner

Choose a reason for hiding this comment

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

camelCase

@bitspill
Copy link

bitspill commented Oct 17, 2024

Here's a simple id translation based on the table from the migration at https://github.com/gamedig/node-gamedig/blob/master/MIGRATE_IDS.md

I think something like this could be tied into the db migration to handle id transformations rather than attempting to parse and merge the old games.txt and the new games.js object based on full name or hyphen parsing

const gameDig4to5IdMap = {
  'americasarmypg': 'aapg',
  '7d2d': 'sdtd',
  'as': 'actionsource',
  'ageofchivalry': 'aoc',
  'arkse': 'ase',
  'arcasimracing': 'asr08',
  'arma': 'aaa',
  'arma2oa': 'a2oa',
  'armacwa': 'acwa',
  'armar': 'armaresistance',
  'armare': 'armareforger',
  'armagetron': 'armagetronadvanced',
  'bat1944': 'battalion1944',
  'bf1942': 'battlefield1942',
  'bfv': 'battlefieldvietnam',
  'bf2': 'battlefield2',
  'bf2142': 'battlefield2142',
  'bfbc2': 'bbc2',
  'bf3': 'battlefield3',
  'bf4': 'battlefield4',
  'bfh': 'battlefieldhardline',
  'bd': 'basedefense',
  'bs': 'bladesymphony',
  'buildandshoot': 'bas',
  'cod4': 'cod4mw',
  'callofjuarez': 'coj',
  'chivalry': 'cmw',
  'commandos3': 'c3db',
  'cacrenegade': 'cacr',
  'contactjack': 'contractjack',
  'cs15': 'counterstrike15',
  'cs16': 'counterstrike16',
  'cs2': 'counterstrike2',
  'crossracing': 'crce',
  'darkesthour': 'dhe4445',
  'daysofwar': 'dow',
  'deadlydozenpt': 'ddpt',
  'dh2005': 'deerhunter2005',
  'dinodday': 'ddd',
  'dirttrackracing2': 'dtr2',
  'dmc': 'deathmatchclassic',
  'dnl': 'dal',
  'drakan': 'dootf',
  'dys': 'dystopia',
  'em': 'empiresmod',
  'empyrion': 'egs',
  'f12002': 'formulaone2002',
  'flashpointresistance': 'ofr',
  'fivem': 'gta5f',
  'forrest': 'theforrest',
  'graw': 'tcgraw',
  'graw2': 'tcgraw2',
  'giantscitizenkabuto': 'gck',
  'ges': 'goldeneyesource',
  'gore': 'gus',
  'hldm': 'hld',
  'hldms': 'hlds',
  'hlopfor': 'hlof',
  'hl2dm': 'hl2d',
  'hidden': 'thehidden',
  'had2': 'hiddendangerous2',
  'igi2': 'i2cs',
  'il2': 'il2sturmovik',
  'insurgencymic': 'imic',
  'isle': 'theisle',
  'jamesbondnightfire': 'jb007n',
  'jc2mp': 'jc2m',
  'jc3mp': 'jc3m',
  'kingpin': 'kloc',
  'kisspc': 'kpctnc',
  'kspdmp': 'kspd',
  'kzmod': 'kreedzclimbing',
  'left4dead': 'l4d',
  'left4dead2': 'l4d2',
  'm2mp': 'm2m',
  'mohsh': 'mohaas',
  'mohbt': 'mohaab',
  'mohab': 'moha',
  'moh2010': 'moh',
  'mohwf': 'mohw',
  'minecraftbe': 'mbe',
  'mtavc': 'gtavcmta',
  'mtasa': 'gtasamta',
  'ns': 'naturalselection',
  'ns2': 'naturalselection2',
  'nwn': 'neverwinternights',
  'nwn2': 'neverwinternights2',
  'nolf': 'tonolf',
  'nolf2': 'nolf2asihw',
  'pvkii': 'pvak2',
  'ps': 'postscriptum',
  'primalcarnage': 'pce',
  'pc': 'projectcars',
  'pc2': 'projectcars2',
  'prbf2': 'prb2',
  'przomboid': 'projectzomboid',
  'quake1': 'quake',
  'quake3': 'q3a',
  'ragdollkungfu': 'rdkf',
  'r6': 'rainbowsix',
  'r6roguespear': 'rs2rs',
  'r6ravenshield': 'rs3rs',
  'redorchestraost': 'roo4145',
  'redm': 'rdr2r',
  'riseofnations': 'ron',
  'rs2': 'rs2v',
  'samp': 'gtasam',
  'saomp': 'gtasao',
  'savage2': 's2ats',
  'ss': 'serioussam',
  'ss2': 'serioussam2',
  'ship': 'theship',
  'sinep': 'sinepisodes',
  'sonsoftheforest': 'sotf',
  'swbf': 'swb',
  'swbf2': 'swb2',
  'swjk': 'swjkja',
  'swjk2': 'swjk2jo',
  'takeonhelicopters': 'toh',
  'tf2': 'teamfortress2',
  'terraria': 'terrariatshock',
  'tribes1': 't1s',
  'ut': 'unrealtournament',
  'ut2003': 'unrealtournament2003',
  'ut2004': 'unrealtournament2004',
  'ut3': 'unrealtournament3',
  'v8supercar': 'v8sc',
  'vcmp': 'vcm',
  'vs': 'vampireslayer',
  'wheeloftime': 'wot',
  'wolfenstein2009': 'wolfenstein',
  'wolfensteinet': 'wet',
  'wurm': 'wurmunlimited',
}

const GameDigId4to5 = (id) => gameDig4to5IdMap[id] ?? id

@louislam
Copy link
Owner

@bitspill Thanks. Did not know it can be upgraded to v5 smoothly.

@louislam louislam modified the milestones: 2.0.0, 2.1.0 Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:monitor Everything related to monitors needs:resolve-merge-conflict pr:please address review comments this PR needs a bit more work to be mergable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GameDig 5.x Support
5 participants