Add status command#396
Conversation
There was a problem hiding this comment.
First of all, thank you for volunteering to contribute to the project! You're our first and only external contributor, and I hope you learned something new working on your implementation.
Unfortunately, I don't think we'll be able to use it at the moment. The only game server we're currently running is Stationeers, and we're not actively using it. We also would not want to create an arbitrary game server scanner by accepting a host:port pair as input. The expected interface for this would have been something like /status stationeers or something like that, although we wouldn't currently be making any use of this command anyway. Apologies if the issue was not clear, but we were also not expecting to hand this work off to an external contributor :D
So I figured if you wanted to take a stab at contributing something, we could have made use of an interface for game server status and the command feature-flagged off for now. My comment on #224 might have not been 100% clear.
If you want, I can still give feedback on your code as it is, but I don't think it would make sense for us to merge it.
|
Okay thank you, any feedback would be appreciated. |
There was a problem hiding this comment.
The code overall looks good! It is not too broad in scope, straightforward, and easy to understand, which is important.
As mentioned in the prior comment, the goal of the ticket was to provide Discord users with a way of checking game server status so they don't have to ping admins if they're having trouble connecting to the server. Ideally, this command would have internal configuration for the game servers we use, and would be extensible to provide information contextually for the given server, including if the server is online AND reachable via the internet. It would also accept minimal input from Discord to ensure the command could not be abused to do anything other than its exact specified intent.
Besides the spec of the ticket being a little off, the code also does not include any tests, which we use to ensure that commands keep working during our long idle maintenance periods where we do not add any new features to the bot.
| @discord.app_commands.command() | ||
| async def status( | ||
| interaction: discord.Interaction, | ||
| address: str, |
There was a problem hiding this comment.
Taking the address as an argument would effectively turn memebot into a general-purpose status-checker for any open game server. Ideally, we'd be able to configure memebot internally to understand our private game servers, and users in our Discord server would be able to query them by name only.
| async def status( | ||
| interaction: discord.Interaction, | ||
| address: str, | ||
| game: str, |
There was a problem hiding this comment.
If we accepted a discord.app_commands.Choice, the API of the command would prevent users from ever requesting an unsupported game server.
| await interaction.followup.send( | ||
| f"**{host}:{port}** — Online\n" | ||
| f"Players: {info['players']['online']}/{info['players']['max']}\n" | ||
| f"Version: {info['version']['name']}\n" | ||
| f"MOTD: {description}" | ||
| ) |
There was a problem hiding this comment.
This is nice of a proof-of-concept, but ideally we'd format this a little more natively for Discord, including an embed and maybe emojis for color coding.
| f"Version: {info['version']['name']}\n" | ||
| f"MOTD: {description}" | ||
| ) | ||
| except Exception: |
There was a problem hiding this comment.
Normally, we would prefer not to catch the bare Exception, but it seems opengsq does not document the exceptions raised by their protocol. Here, it would be nice to have context on whether the server was unreachable or opngsq received unexpected data so that we could act accordingly.
2 participants
Added /status command for checking game server status. Supports Minecraft and Source/Steam servers. Ready for testing on a server with unrestricted outbound ports. Closes #224