Conversation
|
This might add a bit more to the memory consumption, or might not even be possible/hard to do, and also belong in it's own PR, but perhaps an even better way of command handling would be to:
Idk, maybe this is too 'involved' for, what is supposed to be, a simple barebones server, but it is a suggestion nonetheless |
|
In my opinion, add a config to enable commands and whitelisting. |
p2r3
left a comment
p2r3
left a comment
There was a problem hiding this comment.
At a glance, seems fine, just a few nitpicks. Haven't yet tested in-game, I hope I'll be able to get around to that soon.
| // #define WHITELIST | ||
|
|
||
| #ifdef WHITELIST | ||
| #define MAX_WHITELISTED_PLAYERS 8 |
There was a problem hiding this comment.
I feel like WHITELIST is redundant if we've already got a whitelist limit. You could just check if MAX_WHITELISTED_PLAYERS is greater than 0.
| if (message_len <= 1) { | ||
| goto cleanup; | ||
| } | ||
|
|
||
| char* command = strtok((char *)recv_buffer, " "); | ||
| #ifdef WHITELIST | ||
| if (!strcmp(command, "!whitelist")) { | ||
| handleWhitelistCommand(player); | ||
| return; | ||
| } | ||
| #endif | ||
| if (!strcmp((char *) command, "!msg")) { | ||
| handleMessageCommand(player); | ||
| return; | ||
| } | ||
| if (!strcmp((char *) command, "!help")) { | ||
| handleHelpCommand(player); | ||
| return; | ||
| } | ||
|
|
||
| cleanup: | ||
| // Handle fall-through case | ||
| sc_systemChat(player->client_fd, "§7Unknown command. Type !help for help.", 40); |
There was a problem hiding this comment.
This might be cleaner with an if-else chain? The early exits don't make too much sense if the conditions are all the same size and don't nest any further. Would also eliminate the need for a goto label.
There was a problem hiding this comment.
Add spaces after if tokens, before opening bracket.
| } | ||
|
|
||
| #ifdef WHITELIST | ||
| // Returns whether the username is on the server whitelist and if so, the index |
Commands on their own are fairly lightweight. Whitelisting is already behind a
That's outside the scope of this issue/pull request. |
|
I think we should store properties like whitelist, and in a file other than world data. |
|
@siaeyy If you're suggesting we store configuration options in a file, I strongly disagree. The target platform is embedded systems, many of which do not have filesystems, and certainly not efficient ones. |
With a more advanced beef dump protocol we could also provide support to those platforms but yeah I changed my mind, probably doesnt worth it :D |
5 participants
Closes #92
This PR also reworks the command system to make it easier to add new commands in the future.
The memory impact of this change is 128 bytes for storing the 8 whitelisted players and another byte for toggling the whitelist on and off.
Tested on a ESP32-WROOM-32 dev board.