-
Notifications
You must be signed in to change notification settings - Fork 143
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
feat: Add version
as a top level field
#532
Conversation
Nice! Makes sense to me. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
But until merging this, we'd have to decide a default value for that field, add a line in the changelog and further research into the other games that might have (or not) this, such as cs2d
and the others.
@@ -21,6 +21,7 @@ export default class mafia2mp extends Core { | |||
state.numplayers = parseInt(this.readString(reader)) | |||
state.maxplayers = parseInt(this.readString(reader)) | |||
state.raw.gamemode = this.readString(reader) | |||
state.version = state.raw.gamemode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean always having a
I've tested a few other protocols, but I haven't tested all. It would be great to have others join in to help testing! (: |
Yes, as it's a stabilized field it should have a default value to always be present. Lines 23 to 25 in 3a17184
It's alright, this is quite a big task anyways (: |
A change in the readme is also required to add the |
Wondering if
|
Unfortunately I didn't had the time to contribute to the long list of protocols to check for versions. |
No worries! I wasn't able to test some protocols, but most of them I was able to check.
I have to sync with the master and add the |
I guess we're good to go! 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot, great stuff!
* add top level version on existing entries * start adding version on new protocols WIP * add version to more games * more games with version * add more games * more version * even more games with version * add 'delete state.raw.version' * fix delete version * Update CHANGELOG.md * add version in Results.js * more games * add new game * more games * add version on README * add new game * other game * new game * add unreal2 version * add ventrilo version * add eldewrito eldewrito * add beammp version * fix starmade version * add new version in samp protocol * docs: tweak the changelog line a bit --------- Co-authored-by: CosminPerRam <cosmin.p@live.com>
This adds a top level
state.version
on protocols that havestate.raw.version
in their responses.It will close #468 and #528.
It would be good to have other take a look as well!
—
List of protocols checked and notes:
—
valve
protocol.mafia2mp
protocol.quake2
protocol. / gamedig --type protocol-quake1 osqw.com.br:28501tribes1
protocol. / gamedig --type protocol-starsiege 96.126.117.157:29007samp
protocol. / gamedig --type protocol-vcmp 51.178.65.136:8114quake3
protocol.valve
protocol.