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 GameDig monitor #2566

Merged
merged 4 commits into from
Jan 24, 2023
Merged

Add GameDig monitor #2566

merged 4 commits into from
Jan 24, 2023

Conversation

WhyKickAmooCow
Copy link
Contributor

@WhyKickAmooCow WhyKickAmooCow commented Jan 8, 2023

⚠️⚠️⚠️ 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

This pull request adds support for using GameDig as a monitor, this allows rich game server monitoring rather than the apparently unreliable method of using the TCP monitor.

Fixes #1053
Fixes #479
Fixes #525

Type of change

  • User interface (UI)
  • 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 generate no new warnings
  • My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

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.

@louislam louislam added this to the 1.20.0 milestone Jan 8, 2023
@louislam
Copy link
Owner

louislam commented Jan 13, 2023

https://github.com/gamedig/node-gamedig/blob/master/games.txt

This monitor will be perfect if the game field is a dropdown instead of a free text field.

Moreover, autofill in the port if it is empty.

@louislam louislam added the question Further information is requested label Jan 13, 2023
@WhyKickAmooCow
Copy link
Contributor Author

WhyKickAmooCow commented Jan 13, 2023

Autofill in the port may not be necessary. When you tell gamedig the game type, it will automatically resolve the port to use (defaults and through SRV records). It would probably be good to see if we could make port optional for gamedig.

A game dropdown would be great though, I guess the dropdown list would have to be an object with an ID and a displayname so it is easy to select a game by name without knowing the ID.

@cetteup
Copy link
Contributor

cetteup commented Jan 19, 2023

A gamedig monitor would we great.

@WhyKickAmooCow Are you working on the game dropdown?

gamedig's games.txt already contains an id and a human readable name, the id is what you'd pass to the query as type.

Regarding the port: I'd say letting gamedig guess the port every single time is not ideal. Especially because gamedig uses a port offset for some games, which it always applies (see lib/QueryRunner.js#L33). For a Battlefield 4 server using query port 47200, gamedig

  • attempts to query the server on given port (47200) + query port offset (22000)
  • attempts to query server on given port (47200)
  • attempts to query server on default game port (25200) + query port offset

Server admins generally know their query port settings (or can find out rather easily). Thus I would say a much better option is to fill the port field with a default (which can be determined based on data from games.txt) and use the givenPortOnly option for the query. If the port is wrong, the monitor runs a single query that fails. If the port is correct (and the game server is up), the monitor runs a single query that succeeds.

Numer of queries sent (Battlefield 4 example)

givenPortOnly Server up Server down
false (default) 2 3
true 1 1

@louislam
Copy link
Owner

A gamedig monitor would we great.

@WhyKickAmooCow Are you working on the game dropdown?

gamedig's games.txt already contains an id and a human readable name, the id is what you'd pass to the query as type.

Regarding the port: I'd say letting gamedig guess the port every single time is not ideal. Especially because gamedig uses a port offset for some games, which it always applies (see lib/QueryRunner.js#L33). For a Battlefield 4 server using query port 47200, gamedig

* attempts to query the server on given port (47200) + query port offset (22000)

* attempts to query server on given port (47200)

* attempts to query server on default game port (25200) + query port offset

Server admins generally know their query port settings (or can find out rather easily). Thus I would say a much better option is to fill the port field with a default (which can be determined based on data from games.txt) and use the givenPortOnly option for the query. If the port is wrong, the monitor runs a single query that fails. If the port is correct (and the game server is up), the monitor runs a single query that succeeds.

Numer of queries sent (Battlefield 4 example)
givenPortOnly Server up Server down
false (default) 2 3
true 1 1

Thanks, maybe I try to add today.

@louislam
Copy link
Owner

Nice, it is more easy to use with the dropdown now.

image

@louislam louislam merged commit 3eab6e8 into louislam:master Jan 24, 2023
Comment on lines +495 to +499
const state = await Gamedig.query({
type: this.game,
host: this.hostname,
port: this.port
});
Copy link
Contributor

Choose a reason for hiding this comment

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

If you add givenPortOnly: true here, you can avoid the multiple query issue I described.

@cetteup
Copy link
Contributor

cetteup commented Jan 24, 2023

Really nice! Do you plan on adding the givenPortOnly option to the query? I'll gladly open a PR for that if need be, but it seems a bit overkill for one line of code.

@louislam
Copy link
Owner

Oh I forgot to add this. Should be ok now: 9cc3bd0

@cetteup
Copy link
Contributor

cetteup commented Jan 24, 2023

Brilliant! Really looking forward to the next release with gamedig support.

@dgibbs64
Copy link

Just seen this has been added. Great news. Thank you

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

Successfully merging this pull request may close these issues.

Expanding game server monitoring A2S and other game servers Add Minecraft Server checking
4 participants