diff --git a/src/Channel.cc b/src/Channel.cc index 4603c033..6ddab4d0 100644 --- a/src/Channel.cc +++ b/src/Channel.cc @@ -241,13 +241,21 @@ Channel::Message Channel::recv(bool print_contents) { }; } -void Channel::send(uint16_t cmd, uint32_t flag, const void* data, size_t size, - bool print_contents) { +void Channel::send(uint16_t cmd, uint32_t flag, bool print_contents) { + this->send(cmd, flag, nullptr, 0, print_contents); +} + +void Channel::send(uint16_t cmd, uint32_t flag, const std::vector> blocks, bool print_contents) { if (!this->connected()) { channel_exceptions_log.warning("Attempted to send command on closed channel; dropping data"); return; } + size_t size = 0; + for (const auto& b : blocks) { + size += b.second; + } + string send_data; size_t logical_size; size_t send_data_size = 0; @@ -317,10 +325,11 @@ void Channel::send(uint16_t cmd, uint32_t flag, const void* data, size_t size, throw runtime_error("outbound command too large"); } - if (send_data.size() < send_data_size) { - send_data.append(reinterpret_cast(data), size); - send_data.resize(send_data_size, '\0'); + send_data.reserve(send_data_size); + for (const auto& b : blocks) { + send_data.append(reinterpret_cast(b.first), b.second); } + send_data.resize(send_data_size, '\0'); if (print_contents && (this->terminal_send_color != TerminalFormat::END)) { if (use_terminal_colors && this->terminal_send_color != TerminalFormat::NORMAL) { @@ -347,6 +356,11 @@ void Channel::send(uint16_t cmd, uint32_t flag, const void* data, size_t size, evbuffer_add(buf, send_data.data(), send_data.size()); } +void Channel::send( + uint16_t cmd, uint32_t flag, const void* data, size_t size, bool print_contents) { + this->send(cmd, flag, {make_pair(data, size)}, print_contents); +} + void Channel::send(uint16_t cmd, uint32_t flag, const string& data, bool print_contents) { this->send(cmd, flag, data.data(), data.size(), print_contents); } diff --git a/src/Channel.hh b/src/Channel.hh index 98af38da..a12e501d 100644 --- a/src/Channel.hh +++ b/src/Channel.hh @@ -78,7 +78,9 @@ struct Channel { Message recv(bool print_contents = true); // Sends a message with an automatically-constructed header. - void send(uint16_t cmd, uint32_t flag = 0, const void* data = nullptr, size_t size = 0, bool print_contents = true); + void send(uint16_t cmd, uint32_t flag = 0, bool print_contents = true); + void send(uint16_t cmd, uint32_t flag, const void* data, size_t size, bool print_contents = true); + void send(uint16_t cmd, uint32_t flag, const std::vector> blocks, bool print_contents = true); void send(uint16_t cmd, uint32_t flag, const std::string& data, bool print_contents = true); template void send(uint16_t cmd, uint32_t flag, const CmdT& data) { diff --git a/src/ChatCommands.cc b/src/ChatCommands.cc index 698bef90..840689de 100644 --- a/src/ChatCommands.cc +++ b/src/ChatCommands.cc @@ -815,13 +815,11 @@ static void server_command_edit(shared_ptr s, shared_ptr l, s->send_lobby_join_notifications(l, c); } -// TODO: implement this -// TODO: make sure the bank name is filesystem-safe +// TODO: implement this (and make sure the bank name is filesystem-safe) /* static void server_command_change_bank(shared_ptr, shared_ptr, shared_ptr c, const std::u16string&) { check_version(c, GameVersion::BB); - - TODO + ... } */ // TODO: This can be implemented on the proxy server too. @@ -960,8 +958,6 @@ static void server_command_ban(shared_ptr s, shared_ptr l, usecs *= 60 * 60 * 24 * 365; } - // TODO: put the length of time in this message. or don't; presumably the - // person deserved it s->license_manager->ban_until(target->license->serial_number, now() + usecs); send_message_box(target, u"$C6You were banned by a moderator."); target->should_disconnect = true; diff --git a/src/Episode3/DataIndex.cc b/src/Episode3/DataIndex.cc index e066e486..afc51a13 100644 --- a/src/Episode3/DataIndex.cc +++ b/src/Episode3/DataIndex.cc @@ -1717,8 +1717,6 @@ uint64_t DataIndex::card_definitions_mtime() const { const string& DataIndex::get_compressed_map_list() const { if (this->compressed_map_list.empty()) { - // TODO: Write a version of prs_compress that takes iovecs (or something - // similar) so we can eliminate all this string copying here. StringWriter entries_w; StringWriter strings_w; diff --git a/src/Main.cc b/src/Main.cc index 644f9291..fd1c31e7 100644 --- a/src/Main.cc +++ b/src/Main.cc @@ -716,8 +716,6 @@ int main(int argc, char** argv) { } else if (!seed.empty()) { round1_seed = stoul(seed, nullptr, 16); } else { - // TODO: We can support brute-forcing character file encryption, but I'm - // lazy and this would probably not be useful for anyone. throw runtime_error("either --sys or --seed must be given"); } diff --git a/src/ReceiveCommands.cc b/src/ReceiveCommands.cc index b9baba18..72ef7f40 100644 --- a/src/ReceiveCommands.cc +++ b/src/ReceiveCommands.cc @@ -34,10 +34,6 @@ const char* ADD_NEXT_CLIENT_DISCONNECT_HOOK_NAME = "add_next_game_client"; static shared_ptr proxy_options_menu_for_client( shared_ptr s, shared_ptr c) { shared_ptr ret(new Menu(MenuID::PROXY_OPTIONS, u"Proxy options")); - // Note: The descriptions are instead in the map above, because this menu is - // dynamically created every time it's sent to the client. This is just one - // way in which the menu abstraction is currently insufficient (there is a - // TODO about this in README.md). ret->items.emplace_back(ProxyOptionsMenuItemID::GO_BACK, u"Go back", u"Return to the\nProxy Server menu", 0); auto add_option = [&](uint32_t item_id, bool is_enabled, const char16_t* text, const char16_t* description) -> void { @@ -291,9 +287,6 @@ void on_disconnect(shared_ptr s, shared_ptr c) { s->remove_client_from_lobby(c); } - // TODO: Track play time somewhere for BB players - // c->game_data.player()->disp.play_time += ((now() - c->play_time_begin) / 1000000); - // Note: The client's GameData destructor should save their player data // shortly after this point } @@ -2925,7 +2918,6 @@ static void on_00E2_BB(shared_ptr, shared_ptr c, auto& cmd = check_size_t(data, sizeof(KeyAndTeamConfigBB) - 4, sizeof(KeyAndTeamConfigBB)); c->game_data.account()->key_config = cmd; - // TODO: We should probably send a response here, but I don't know which one! } static void on_89(shared_ptr s, shared_ptr c, diff --git a/src/ReceiveSubcommands.cc b/src/ReceiveSubcommands.cc index f7b3cbd0..d82b62c0 100644 --- a/src/ReceiveSubcommands.cc +++ b/src/ReceiveSubcommands.cc @@ -236,12 +236,10 @@ static void on_sync_joining_player_item_state(shared_ptr, c->log.info("Byteswapped and recompressed item sync data (%zX bytes)", out_compressed_data.size()); } - // TODO: It'd be nice to not copy the data so many times here. - StringWriter out_w; - out_w.put(out_cmd); - out_w.write(out_compressed_data); - - send_command(target, command, flag, out_w.str()); + vector> blocks; + blocks.emplace_back(make_pair(&out_cmd, sizeof(out_cmd))); + blocks.emplace_back(make_pair(out_compressed_data.data(), out_compressed_data.size())); + send_command(target, command, flag, blocks); } } } @@ -1010,8 +1008,6 @@ static void on_equip_unequip_item(shared_ptr, throw logic_error("item tracking not enabled in BB game"); } - // TODO: Should we forward this command on BB? The old version of newserv - // didn't, but that seems wrong. forward_subcommand(l, c, command, flag, data, size); } @@ -1607,14 +1603,7 @@ static void on_identify_item_bb(shared_ptr, c->game_data.player()->disp.meseta -= 100; c->game_data.identify_result = c->game_data.player()->inventory.items[x]; c->game_data.identify_result.data.data1[4] &= 0x7F; - - // TODO: move this into a SendCommands.cc function - G_IdentifyResult_BB_6xB9 res; - res.header.subcommand = 0xB9; - res.header.size = sizeof(res) / 4; - res.header.client_id = c->lobby_client_id; - res.item_data = c->game_data.identify_result.data; - send_command_t(l, 0x60, 0x00, res); + send_item_identify_result(l, c); } else { forward_subcommand(l, c, command, flag, data, size); diff --git a/src/SendCommands.cc b/src/SendCommands.cc index 44920ac4..aea82e1e 100644 --- a/src/SendCommands.cc +++ b/src/SendCommands.cc @@ -58,6 +58,11 @@ const unordered_set bb_crypt_initial_client_commands({ string("\xDC\x00\xDB\x00\x00\x00\x00\x00", 8), }); +void send_command(std::shared_ptr c, uint16_t command, + uint32_t flag, const std::vector>& blocks) { + c->channel.send(command, flag, blocks); +} + void send_command(shared_ptr c, uint16_t command, uint32_t flag, const void* data, size_t size) { c->channel.send(command, flag, data, size); @@ -617,20 +622,14 @@ void send_patch_file(shared_ptr c, shared_ptr f) { send_command_t(c, 0x06, 0x00, open_cmd); for (size_t x = 0; x < f->chunk_crcs.size(); x++) { - // TODO: The use of StringWriter here is... unfortunate. Write a version of - // Channel::send that takes iovecs or something to avoid these dumb massive - // string copies. - StringWriter w; auto data = f->load_data(); size_t chunk_size = min(f->size - (x * 0x4000), 0x4000); - S_WriteFileHeader_Patch_07 write_cmd_header = { - x, f->chunk_crcs[x], chunk_size}; - w.put(write_cmd_header); - w.write(data->data() + (x * 0x4000), chunk_size); - while (w.size() & 7) { - w.put_u8(0); - } - send_command(c, 0x07, 0x00, w.str()); + + vector> blocks; + S_WriteFileHeader_Patch_07 cmd_header = {x, f->chunk_crcs[x], chunk_size}; + blocks.emplace_back(&cmd_header, sizeof(cmd_header)); + blocks.emplace_back(data->data() + (x * 0x4000), chunk_size); + send_command(c, 0x07, 0x00, blocks); } S_CloseCurrentFile_Patch_08 close_cmd = {0}; @@ -2000,6 +1999,18 @@ void send_destroy_item(shared_ptr l, shared_ptr c, send_command_t(l, 0x60, 0x00, cmd); } +void send_item_identify_result(shared_ptr l, shared_ptr c) { + if (c->version() != GameVersion::BB) { + throw logic_error("cannot send item identify result to non-BB client"); + } + G_IdentifyResult_BB_6xB9 res; + res.header.subcommand = 0xB9; + res.header.size = sizeof(res) / 4; + res.header.client_id = c->lobby_client_id; + res.item_data = c->game_data.identify_result.data; + send_command_t(l, 0x60, 0x00, res); +} + void send_bank(shared_ptr c) { if (c->version() != GameVersion::BB) { throw logic_error("6xBC can only be sent to BB clients"); @@ -2522,7 +2533,7 @@ void send_ep3_update_spectator_count(shared_ptr l) { // this function just to check a behavior flag) // Note: We can't use send_command_t(l, ...) here because that would send the - // same command to l and to all wather lobbies. The commands should have + // same command to l and to all watcher lobbies. The commands should have // different values depending on who's in each watcher lobby, so we have to // manually send to each client here. for (auto c : l->clients) { diff --git a/src/SendCommands.hh b/src/SendCommands.hh index f581a776..e07c92f3 100644 --- a/src/SendCommands.hh +++ b/src/SendCommands.hh @@ -32,6 +32,9 @@ extern const std::unordered_set bb_crypt_initial_client_commands; // pointer is given but size is accidentally not given (e.g. if the type of // data in the calling function is changed from string to void*). +void send_command(std::shared_ptr c, uint16_t command, + uint32_t flag, const std::vector>& blocks); + void send_command(std::shared_ptr c, uint16_t command, uint32_t flag, const void* data, size_t size); @@ -313,6 +316,7 @@ void send_create_inventory_item(std::shared_ptr l, std::shared_ptr l, std::shared_ptr c, uint32_t item_id, uint32_t amount); +void send_item_identify_result(std::shared_ptr l, std::shared_ptr c); void send_bank(std::shared_ptr c); void send_shop(std::shared_ptr c, uint8_t shop_type); void send_level_up(std::shared_ptr l, std::shared_ptr c); diff --git a/src/Text.hh b/src/Text.hh index 8f718172..ddff1ecf 100644 --- a/src/Text.hh +++ b/src/Text.hh @@ -354,12 +354,6 @@ struct parray { } } __attribute__((packed)); -// TODO: It appears that these actually do not have to be null-terminated in PSO -// commands some of the time. As an example, creating a game with a name with -// the maximum length results in a C1 command with no null byte between the game -// name and the password. We should be able to handle this by making ptexts not -// required to be null-terminated in storage - this will still be safe if we -// limit all operations by Count. template struct ptext : parray { ptext() {