Add /hasmap chat command and support for extensions in /vote map
#297
Conversation
|
I do not really see why Perhaps
|
| { | ||
| std::string level_name{level_name_input}; | ||
|
|
||
| if (level_name.size() < 4 || level_name.compare(level_name.size() - 4, 4, ".rfl") != 0) { |
There was a problem hiding this comment.
Please use string_ends_with from string-utils.h
| @@ -328,6 +339,9 @@ bool handle_server_chat_command(std::string_view server_command, rf::Player* sen | |||
| else if (cmd_name == "stats") { | |||
| send_private_message_with_stats(sender); | |||
| } | |||
| else if (cmd_name == "hasmap" || cmd_name == "haslevel") { | |||
There was a problem hiding this comment.
I wonder if we should merge words like this, instead of using _. For short commands it works but with 3 or more words it will be unreadable 🤔 WDYT? But we already have nextmap so I see why you made it like this.
There was a problem hiding this comment.
Indeed I made it this way to match nextmap.
I think using _ would be OK (perhaps even preferred) if these commands were entered via console (because then tab complete would make it much easier). Entered as chat commands though, I think making them more simple to type very quickly is important, and _ is a bit annoying for that purpose.
For short chat commands like this at least, I think omitting the _ is best... and I also don't think making long chat commands would be good anyway, so the question of how to handle 3 words probably won't come up.
| @@ -554,6 +568,19 @@ static bool check_player_ac_status([[maybe_unused]] rf::Player* player) | |||
| return true; | |||
| } | |||
|
|
|||
| std::pair<bool, std::string> is_level_name_valid(std::string_view level_name_input) | |||
There was a problem hiding this comment.
Two things:
- I think this name is not good - validation of something (name in this case) usually means to check if it follows some rules, like not having denied characters, having a proper length, etc. In this case it's not really a validation but checking for existence. The name
does_level_existwould be more clear - I think this function has too broad responsibility. I would rather prefer a function that adds extension (not necessary rfl) if it's missing (may be useful in other contexts) and another to check existence (this one is probably not needed because it's a single line). To be honest all that code could be inlined in the command too. It will be trivial after you use
string_ends_with.
There was a problem hiding this comment.
Valid point (lol) on the use of the word valid. Something like is_level_loaded might fit a little better than does_level_exist though - WDYT?
I would rather prefer a function that adds extension (not necessary rfl) if it's missing (may be useful in other contexts)
I like this idea - I don't know of another use case off the top of my head but I could see it coming up down the road.
There was a problem hiding this comment.
is_level_loaded is kind of confusing. does_level_exist is imo much better.
There was a problem hiding this comment.
is_level_loaded doesn't make sense. Loaded level is the current one. For example to check if any is loaded we use rf::level.flags & rf::LEVEL_LOADED.
Idk why does_level_exist is bad, but alternative could be is_level_available
|
I think it better to have a |
I agree that this would be better, but my understanding is that this would need to on a technical level just be a console command that sends a chat packet, or it would need to have a new packet created. Am I correct there? I wouldn't be opposed to a new packet being created for this, though that would be substantially more work than just handling it via a chat command. Is it worth it? WDYT? |
Yes or maybe we could mark server chat packets so they are not displayed in UI.
I think it is pretty straightforward. Another idea is to just use |
I think this could be an idea worth exploring in the future - having a set of rcon commands that can be used by anyone, separate from the set that in effect require elevation via entering an rcon password, not unlike standard user privileges vs. elevated privileges via sudo or UAC. I do think that is a bit out of scope for this PR though and ought to be handled separately. I don't think having |
This PR:
is_level_name_validutility function, which takes a level filename as input (with or without extension), adds the .rfl extension if missing, and returns whether the server has that level installed./hasmapserver chat command (alias:/haslevel) - takes a level filename as input and outputs whether the map is installed on the server (and can therefore be voted for and played on)./vote maplogic - adds support for specifying extensions when voting for maps. (previously,/vote map dm02.rflwouldn't work, it will now)Vote passedmessage for change level votes to now print the level being switched to.