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 /hasmap chat command and support for extensions in /vote map #297

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ Version 1.9.0 (not released yet)
- Add level filename to "Level Initializing" console message
- Properly handle WM_PAINT in dedicated server, may improve performance (DF bug)
- Fix crash when `verify_level` command is run without a level being loaded
- Add `/hasmap` server chat command
- Add handling of level filename extensions with `/vote level` server chat command

Version 1.8.0 (released 2022-09-17)
-----------------------------------
Expand Down
27 changes: 27 additions & 0 deletions game_patch/multi/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "../main/main.h"
#include <common/utils/list-utils.h>
#include "../rf/player/player.h"
#include "../rf/misc.h"
#include "../rf/multi.h"
#include "../rf/parse.h"
#include "../rf/weapon.h"
Expand Down Expand Up @@ -227,6 +228,16 @@ void handle_next_map_command(rf::Player* player)
send_chat_line_packet(msg.c_str(), player);
}

void handle_has_map_command(rf::Player* player, std::string_view level_name)
{
auto [is_valid, checked_level_name] = is_level_name_valid(level_name);

auto availability = is_valid ? "available" : "NOT available";
auto msg = std::format("Level {} is {} on the server.", checked_level_name, availability);

send_chat_line_packet(msg.c_str(), player);
}

void handle_save_command(rf::Player* player, std::string_view save_name)
{
auto& pdata = get_player_additional_data(player);
Expand Down Expand Up @@ -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") {
Copy link
Owner

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

handle_has_map_command(sender, cmd_arg);
}
else {
return false;
}
Expand Down Expand 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)
Copy link
Owner

Choose a reason for hiding this comment

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

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_exist would 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

is_level_loaded is kind of confusing. does_level_exist is imo much better.

Copy link
Owner

Choose a reason for hiding this comment

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

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

{
std::string level_name{level_name_input};

if (level_name.size() < 4 || level_name.compare(level_name.size() - 4, 4, ".rfl") != 0) {
Copy link
Owner

Choose a reason for hiding this comment

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

Please use string_ends_with from string-utils.h

level_name += ".rfl";
}

bool is_valid = rf::get_file_checksum(level_name.c_str()) != 0;

return {is_valid, level_name};
}

FunHook<void(rf::Player*)> multi_spawn_player_server_side_hook{
0x00480820,
[](rf::Player* player) {
Expand Down
1 change: 1 addition & 0 deletions game_patch/multi/server_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ extern std::string g_prev_level;

void cleanup_win32_server_console();
void handle_vote_command(std::string_view vote_name, std::string_view vote_arg, rf::Player* sender);
std::pair<bool, std::string> is_level_name_valid(std::string_view level_name_input);
void server_vote_do_frame();
void init_server_commands();
void extend_round_time(int minutes);
Expand Down
13 changes: 9 additions & 4 deletions game_patch/multi/votes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,15 @@ struct VoteLevel : public Vote

bool process_vote_arg([[maybe_unused]] std::string_view arg, rf::Player* source) override
{
m_level_name = std::format("{}.rfl", arg);
if (!rf::get_file_checksum(m_level_name.c_str())) {
send_chat_line_packet("Cannot find specified level!", source);
auto [is_valid, level_name] = is_level_name_valid(arg);

if (!is_valid) {
auto msg = std::format("\xA6 Cannot start vote: level {} is not available on the server!", level_name);
send_chat_line_packet(msg.c_str(), source);
return false;
}

m_level_name = std::move(level_name);
return true;
}

Expand All @@ -260,7 +264,8 @@ struct VoteLevel : public Vote

void on_accepted() override
{
send_chat_line_packet("\xA6 Vote passed: changing level", nullptr);
auto msg = std::format("\xA6 Vote passed: changing level to {}", m_level_name);
send_chat_line_packet(msg.c_str(), nullptr);
rf::multi_change_level(m_level_name.c_str());
}

Expand Down