From eb703ee96ea364ff8e5f1d737ba8df10ae51566f Mon Sep 17 00:00:00 2001 From: Tyler Wilding Date: Mon, 3 Jun 2024 00:14:52 -0400 Subject: [PATCH] REPL related improvements and fixes (#3545) Motivated by - https://github.com/open-goal/opengoal-vscode/pull/358 This addresses the following: - Fixes #2939 spam edge-case - Stop picking a different nREPL port based on the game mode by default, this causes friction for tools in the average usecase (having a REPL open for a single game, and wanting to connect to it). `goalc` spins up fine even if the port is already bound to. - For people that need/want this behaviour, adding per-game configuration to the `repl-config.json` is on my todo list. - Allows `goalc` to permit redefining symbols, including functions. This is defaulted to off via the `repl-config.json` but it allows you to for example, change the definition of a function without having to restart and rebuild the entire game. ![Screenshot 2024-06-02 124558](https://github.com/open-goal/jak-project/assets/13153231/28f81f6e-b7b8-4172-9787-f96e4ab1305b) - Updates the welcome message to include a bunch of useful metadata up-front. Cleaned up all the startup logs that appear when starting goalc, many of whom's information is now included in the welcome message. - Before: ![image](https://github.com/open-goal/jak-project/assets/13153231/814c2374-4808-408e-9ed6-67114902a1d9) - After: ![Screenshot 2024-06-01 235954](https://github.com/open-goal/jak-project/assets/13153231/f3f459fb-2cbb-46ba-a90f-318243d4b3b3) --- common/CMakeLists.txt | 2 +- common/cross_sockets/XSocketClient.cpp | 4 +- common/cross_sockets/XSocketServer.cpp | 16 +++- common/cross_sockets/XSocketServer.h | 2 +- common/goos/Reader.cpp | 1 - common/goos/Reader.h | 2 +- common/log/log.cpp | 6 +- common/repl/config.cpp | 9 ++ common/repl/config.h | 11 +++ common/repl/nrepl/ReplServer.cpp | 89 +++++++++++------ common/repl/nrepl/ReplServer.h | 1 + common/repl/{util.cpp => repl_wrapper.cpp} | 96 +++++++++++++------ common/repl/{util.h => repl_wrapper.h} | 17 ++-- common/util/FileUtil.cpp | 24 +++-- common/util/FileUtil.h | 2 +- game/kernel/jak2/kboot.cpp | 2 +- game/kernel/jak3/kboot.cpp | 2 +- .../tools/subtitle_editor/subtitle_editor.cpp | 2 +- .../subtitle_editor_repl_client.cpp | 3 - goal_src/goal-lib.gc | 3 - goal_src/goos-lib.gs | 4 +- goalc/build_actor/common/MercExtract.cpp | 2 +- goalc/build_actor/common/MercExtract.h | 2 +- goalc/compiler/Compiler.cpp | 2 +- goalc/compiler/Compiler.h | 2 +- .../compiler/compilation/CompilerControl.cpp | 2 +- goalc/compiler/compilation/Define.cpp | 17 +++- goalc/main.cpp | 39 +++----- goalc/make/MakeSystem.cpp | 4 +- goalc/make/MakeSystem.h | 2 + scripts/examples/nrepl-test.py | 6 +- 31 files changed, 237 insertions(+), 139 deletions(-) rename common/repl/{util.cpp => repl_wrapper.cpp} (67%) rename common/repl/{util.h => repl_wrapper.h} (76%) diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt index caba17a241..69c86b5216 100644 --- a/common/CMakeLists.txt +++ b/common/CMakeLists.txt @@ -51,7 +51,7 @@ add_library(common repl/config.cpp repl/nrepl/ReplClient.cpp repl/nrepl/ReplServer.cpp - repl/util.cpp + repl/repl_wrapper.cpp serialization/subtitles/subtitles_v1.cpp serialization/subtitles/subtitles_v2.cpp serialization/subtitles/subtitles.cpp diff --git a/common/cross_sockets/XSocketClient.cpp b/common/cross_sockets/XSocketClient.cpp index 0c6c791e0b..57d01eeffa 100644 --- a/common/cross_sockets/XSocketClient.cpp +++ b/common/cross_sockets/XSocketClient.cpp @@ -5,6 +5,8 @@ #include "common/cross_sockets/XSocket.h" #include "common/log/log.h" +#include "fmt/core.h" + // clang-format off #ifdef _WIN32 #define NOMINMAX @@ -13,9 +15,7 @@ #include #include #endif -#include "common/repl/nrepl/ReplServer.h" -#include "fmt/core.h" // clang-format on XSocketClient::XSocketClient(int _tcp_port) { diff --git a/common/cross_sockets/XSocketServer.cpp b/common/cross_sockets/XSocketServer.cpp index 8c83ca2b47..bd563b91d1 100644 --- a/common/cross_sockets/XSocketServer.cpp +++ b/common/cross_sockets/XSocketServer.cpp @@ -35,7 +35,7 @@ void XSocketServer::shutdown_server() { close_server_socket(); } -bool XSocketServer::init_server() { +bool XSocketServer::init_server(bool failure_may_occur) { listening_socket = open_socket(AF_INET, SOCK_STREAM, 0); if (listening_socket < 0) { listening_socket = -1; @@ -76,19 +76,27 @@ bool XSocketServer::init_server() { addr.sin_port = htons(tcp_port); if (bind(listening_socket, (sockaddr*)&addr, sizeof(addr)) < 0) { - lg::error("[XSocketServer:{}] failed to bind", tcp_port); + if (failure_may_occur) { + lg::debug("[XSocketServer:{}] failed to bind", tcp_port); + } else { + lg::error("[XSocketServer:{}] failed to bind", tcp_port); + } close_server_socket(); return false; } if (listen(listening_socket, 0) < 0) { - lg::error("[XSocketServer:{}] failed to listen", tcp_port); + if (failure_may_occur) { + lg::debug("[XSocketServer:{}] failed to listen", tcp_port); + } else { + lg::error("[XSocketServer:{}] failed to listen", tcp_port); + } close_server_socket(); return false; } server_initialized = true; - lg::info("[XSocketServer:{}] initialized", tcp_port); + lg::debug("[XSocketServer:{}] initialized", tcp_port); post_init(); return true; } diff --git a/common/cross_sockets/XSocketServer.h b/common/cross_sockets/XSocketServer.h index 495868c3c9..bcb1660b53 100644 --- a/common/cross_sockets/XSocketServer.h +++ b/common/cross_sockets/XSocketServer.h @@ -20,7 +20,7 @@ class XSocketServer { XSocketServer(const XSocketServer&) = delete; XSocketServer& operator=(const XSocketServer&) = delete; - bool init_server(); + bool init_server(bool failure_may_occur = false); void shutdown_server(); void close_server_socket(); diff --git a/common/goos/Reader.cpp b/common/goos/Reader.cpp index 3c5d666d1a..f438a410d0 100644 --- a/common/goos/Reader.cpp +++ b/common/goos/Reader.cpp @@ -12,7 +12,6 @@ #include "Reader.h" #include "common/log/log.h" -#include "common/repl/util.h" #include "common/util/FileUtil.h" #include "common/util/FontUtils.h" diff --git a/common/goos/Reader.h b/common/goos/Reader.h index 32b51f075b..c20071e1a3 100644 --- a/common/goos/Reader.h +++ b/common/goos/Reader.h @@ -18,7 +18,7 @@ #include "common/goos/Object.h" #include "common/goos/TextDB.h" -#include "common/repl/util.h" +#include "common/repl/repl_wrapper.h" #include "common/util/Assert.h" namespace goos { diff --git a/common/log/log.cpp b/common/log/log.cpp index bce3d2e597..dfe623b0bc 100644 --- a/common/log/log.cpp +++ b/common/log/log.cpp @@ -163,7 +163,7 @@ void set_file(const std::string& filename, file_util::find_files_in_dir(fs::path(complete_filename).parent_path(), std::regex(fmt::format("{}\\.(\\d\\.)?log", filename))); for (const auto& file : old_log_files) { - lg::info("removing {}", file.string()); + lg::debug("removing {}", file.string()); fs::remove(file); } // remove the oldest log file if there are more than LOG_ROTATE_MAX @@ -172,9 +172,9 @@ void set_file(const std::string& filename, // sort the names and remove them existing_log_files = file_util::sort_filepaths(existing_log_files, true); if (existing_log_files.size() > (LOG_ROTATE_MAX - 1)) { - lg::info("removing {} log files", existing_log_files.size() - (LOG_ROTATE_MAX - 1)); + lg::debug("removing {} log files", existing_log_files.size() - (LOG_ROTATE_MAX - 1)); for (int i = 0; i < (int)existing_log_files.size() - (LOG_ROTATE_MAX - 1); i++) { - lg::info("removing {}", existing_log_files.at(i).string()); + lg::debug("removing {}", existing_log_files.at(i).string()); fs::remove(existing_log_files.at(i)); } } diff --git a/common/repl/config.cpp b/common/repl/config.cpp index aaf311c831..57274ef96e 100644 --- a/common/repl/config.cpp +++ b/common/repl/config.cpp @@ -7,15 +7,21 @@ namespace REPL { void to_json(json& j, const Config& obj) { j = json{ + {"nreplPort", obj.nrepl_port}, {"gameVersionFolder", obj.game_version_folder}, {"numConnectToTargetAttempts", obj.target_connect_attempts}, {"asmFileSearchDirs", obj.asm_file_search_dirs}, {"keybinds", obj.keybinds}, {"perGameHistory", obj.per_game_history}, + {"permissiveRedefinitions", obj.permissive_redefinitions}, }; } void from_json(const json& j, Config& obj) { + // TODO - make a camelCase variant of json_serialize/deserialize macros + if (j.contains("nreplPort")) { + j.at("nreplPort").get_to(obj.nrepl_port); + } if (j.contains("gameVersionFolder")) { j.at("gameVersionFolder").get_to(obj.game_version_folder); } @@ -55,6 +61,9 @@ void from_json(const json& j, Config& obj) { if (j.contains("perGameHistory")) { j.at("perGameHistory").get_to(obj.per_game_history); } + if (j.contains("permissiveRedefinitions")) { + j.at("permissiveRedefinitions").get_to(obj.permissive_redefinitions); + } // if there is game specific configuration, override any values we just set if (j.contains(version_to_game_name(obj.game_version))) { from_json(j.at(version_to_game_name(obj.game_version)), obj); diff --git a/common/repl/config.h b/common/repl/config.h index 96b0247efc..ff1857be72 100644 --- a/common/repl/config.h +++ b/common/repl/config.h @@ -26,11 +26,14 @@ struct KeyBind { void to_json(json& j, const KeyBind& obj); void from_json(const json& j, KeyBind& obj); +// TODO - per-game config struct Config { GameVersion game_version; Config(GameVersion _game_version) : game_version(_game_version){}; // this is the default REPL configuration + int nrepl_port = 8181; + int temp_nrepl_port = -1; std::string game_version_folder; int target_connect_attempts = 30; std::vector asm_file_search_dirs = {}; @@ -45,6 +48,14 @@ struct Config { {KeyBind::Modifier::CTRL, "B", "Displays the most recently caught backtrace", "(:di)"}, {KeyBind::Modifier::CTRL, "N", "Full build of the game", "(mi)"}}; bool per_game_history = true; + bool permissive_redefinitions = false; + + int get_nrepl_port() { + if (temp_nrepl_port != -1) { + return temp_nrepl_port; + } + return nrepl_port; + } }; void to_json(json& j, const Config& obj); void from_json(const json& j, Config& obj); diff --git a/common/repl/nrepl/ReplServer.cpp b/common/repl/nrepl/ReplServer.cpp index 267d5fa872..b2274a4f5f 100644 --- a/common/repl/nrepl/ReplServer.cpp +++ b/common/repl/nrepl/ReplServer.cpp @@ -27,7 +27,18 @@ ReplServer::~ReplServer() { void ReplServer::post_init() { // Add the listening socket to our set of sockets - lg::info("[nREPL:{}:{}] awaiting connections", tcp_port, listening_socket); + lg::debug("[nREPL:{}:{}] awaiting connections", tcp_port, listening_socket); +} + +void ReplServer::error_response(int socket, const std::string& error) { + std::string msg = fmt::format("[ERROR]: {}", error); + auto resp = write_to_socket(socket, msg.c_str(), msg.size()); + if (resp == -1) { + lg::warn("[nREPL:{}] Client Disconnected: {}", tcp_port, address_to_string(addr), + ntohs(addr.sin_port), socket); + close_socket(socket); + client_sockets.erase(socket); + } } void ReplServer::ping_response(int socket) { @@ -48,7 +59,6 @@ std::optional ReplServer::get_msg() { // Add the server's main listening socket (where we accept clients from) FD_SET(listening_socket, &read_sockets); - int max_sd = listening_socket; for (const int& sock : client_sockets) { if (sock > max_sd) { @@ -60,12 +70,11 @@ std::optional ReplServer::get_msg() { } // Wait for activity on _something_, with a timeout so we don't get stuck here on exit. - struct timeval timeout; - timeout.tv_sec = 0; - timeout.tv_usec = 100000; + struct timeval timeout = {0, 100000}; auto activity = select(max_sd + 1, &read_sockets, NULL, NULL, &timeout); - - if (activity < 0) { // TODO - || error! + if (activity < 0 && errno != EINTR) { + lg::error("[nREPL:{}] select error, returned: {}, errno: {}", tcp_port, activity, + strerror(errno)); return std::nullopt; } @@ -74,44 +83,46 @@ std::optional ReplServer::get_msg() { socklen_t addr_len = sizeof(addr); auto new_socket = accept_socket(listening_socket, (sockaddr*)&addr, &addr_len); if (new_socket < 0) { - // TODO - handle error + if (new_socket != -1) { + lg::error("[nREPL:{}] accept error, returned: {}, errono: {}", tcp_port, new_socket, + strerror(errno)); + } } else { lg::info("[nREPL:{}]: New socket connection: {}:{}:{}", tcp_port, address_to_string(addr), ntohs(addr.sin_port), new_socket); - // Say hello ping_response(new_socket); // Track the new socket if ((int)client_sockets.size() < max_clients) { client_sockets.insert(new_socket); } else { - // TODO - Respond with NO + // Respond with NO and close the socket + lg::warn("[nREPL:{}]: Maximum clients reached. Rejecting connection.", tcp_port); + error_response(new_socket, "Maximum clients reached. Rejecting connection."); + close_socket(new_socket); } } } - // otherwise (and no matter what) check all the clients to see if they have sent us anything - // else its some IO operation on some other socket - // - // RACE - the first client wins - - // TODO - there are ways to do this with iterators but, couldn't figure it out! - std::vector sockets_to_scan(client_sockets.begin(), client_sockets.end()); - for (const int& sock : sockets_to_scan) { + // Check all clients for activity + for (auto it = client_sockets.begin(); it != client_sockets.end();) { + int sock = *it; if (FD_ISSET(sock, &read_sockets)) { // Attempt to read a header - // TODO - should this be in a loop? auto req_bytes = read_from_socket(sock, header_buffer.data(), header_buffer.size()); - if (req_bytes == 0) { - // Socket disconnected + if (req_bytes <= 0) { // TODO - add a queue of messages in the REPL::Wrapper so we can print _BEFORE_ the prompt // is output - lg::warn("[nREPL:{}] Client Disconnected: {}", tcp_port, address_to_string(addr), - ntohs(addr.sin_port), sock); - + if (req_bytes == 0) { + lg::warn("[nREPL:{}] Client Disconnected: {}", tcp_port, address_to_string(addr)); + } else { + lg::warn("[nREPL:{}] Error reading from socket on {}: {}", tcp_port, + address_to_string(addr), strerror(errno)); + } // Cleanup the socket and remove it from our set close_socket(sock); - client_sockets.erase(sock); + it = client_sockets.erase(it); // Erase and move to the next element + continue; } else { // Otherwise, process the message auto* header = (ReplServerHeader*)(header_buffer.data()); @@ -119,7 +130,12 @@ std::optional ReplServer::get_msg() { int expected_size = header->length; int got = 0; int tries = 0; + bool skip_to_next_socket = false; while (got < expected_size) { + if (want_exit_callback()) { + lg::warn("[nREPL:{}] Terminating nREPL early", tcp_port); + return std::nullopt; + } tries++; if (tries > 100) { break; @@ -131,11 +147,25 @@ std::optional ReplServer::get_msg() { tcp_port, got, expected_size, buffer.size()); return std::nullopt; } - auto x = read_from_socket(sock, buffer.data() + got, expected_size - got); - if (want_exit_callback()) { - return std::nullopt; + auto bytes_read = read_from_socket(sock, buffer.data() + got, expected_size - got); + if (bytes_read <= 0) { + if (bytes_read == 0) { + lg::warn("[nREPL:{}] Client Disconnected: {}", tcp_port, address_to_string(addr)); + } else { + lg::warn("[nREPL:{}] Error reading from socket on {}: {}", tcp_port, + address_to_string(addr), strerror(errno)); + } + close_socket(sock); + it = client_sockets.erase(it); // Erase and move to the next element + skip_to_next_socket = true; + break; } - got += x > 0 ? x : 0; + got += bytes_read; + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } + + if (skip_to_next_socket) { + continue; } switch (header->type) { @@ -149,6 +179,7 @@ std::optional ReplServer::get_msg() { } } } + ++it; } return std::nullopt; } diff --git a/common/repl/nrepl/ReplServer.h b/common/repl/nrepl/ReplServer.h index 4cd5e09ba0..9372d5f22f 100644 --- a/common/repl/nrepl/ReplServer.h +++ b/common/repl/nrepl/ReplServer.h @@ -27,5 +27,6 @@ class ReplServer : public XSocketServer { fd_set read_sockets; std::set client_sockets = {}; + void error_response(int socket, const std::string& error); void ping_response(int socket); }; diff --git a/common/repl/util.cpp b/common/repl/repl_wrapper.cpp similarity index 67% rename from common/repl/util.cpp rename to common/repl/repl_wrapper.cpp index 7484e074a5..fd2b517dec 100644 --- a/common/repl/util.cpp +++ b/common/repl/repl_wrapper.cpp @@ -1,4 +1,4 @@ -#include "util.h" +#include "repl_wrapper.h" #include "common/util/FileUtil.h" #include "common/util/json_util.h" @@ -8,36 +8,66 @@ #include "fmt/color.h" #include "fmt/core.h" #include "third-party/replxx/include/replxx.hxx" -// TODO - expand a list of hints (ie. a hint for defun to show at a glance how to write a function, -// or perhaps, show the docstring for the current function being used?) namespace REPL { void Wrapper::clear_screen() { repl.clear_screen(); } -void Wrapper::print_welcome_message() { - // TODO - dont print on std-out - // Welcome message / brief intro for documentation - std::string ascii; - ascii += " _____ _____ _____ _____ __ \n"; - ascii += "| |___ ___ ___| __| | _ | | \n"; - ascii += "| | | . | -_| | | | | | | |__ \n"; - ascii += "|_____| _|___|_|_|_____|_____|__|__|_____|\n"; - ascii += " |_| \n"; - fmt::print(fmt::emphasis::bold | fg(fmt::color::orange), ascii); - - fmt::print("Welcome to OpenGOAL {}.{}!\n", versions::GOAL_VERSION_MAJOR, - versions::GOAL_VERSION_MINOR); - fmt::print("Run {} or {} for help with common commands and REPL usage.\n", - fmt::styled("(repl-help)", fmt::emphasis::bold | fg(fmt::color::cyan)), - fmt::styled("(repl-keybinds)", fmt::emphasis::bold | fg(fmt::color::cyan))); - fmt::print("Run "); - fmt::print(fmt::emphasis::bold | fg(fmt::color::cyan), "(lt)"); - fmt::print(" to connect to the local target.\n"); - fmt::print("Run "); - fmt::print(fmt::emphasis::bold | fg(fmt::color::cyan), "(mi)"); - fmt::print(" to rebuild the entire game.\n\n"); +void Wrapper::print_welcome_message(const std::vector& loaded_projects) { + std::string message; + message += fmt::format(fmt::emphasis::bold | fg(fmt::color::orange), " ..:::::..\n"); + message += fmt::format(fmt::emphasis::bold | fg(fmt::color::orange), " .:-----------:.\n"); + message += fmt::format(fmt::emphasis::bold | fg(fmt::color::orange), " .-----."); + message += fmt::format(fmt::emphasis::bold, " Welcome to OpenGOAL {}.{} [{}]", + versions::GOAL_VERSION_MAJOR, versions::GOAL_VERSION_MINOR, + fmt::format(fg(fmt::color::gray), "{}", build_revision())); + if (!username.empty() && username != "#f" && username != "unknown") { + message += fmt::format(fg(fmt::color::light_green), " {}", username); + } + message += "!\n"; + message += fmt::format(fmt::emphasis::bold | fg(fmt::color::orange), " .---."); + if (repl_config.game_version == GameVersion::Jak1) { + message += fmt::format(" [{}]: ", fmt::format(fg(fmt::color::orange), "jak1")); + } else if (repl_config.game_version == GameVersion::Jak2) { + message += fmt::format(" [{}]: ", fmt::format(fg(fmt::color::purple), "jak2")); + } else if (repl_config.game_version == GameVersion::Jak3) { + message += fmt::format(" [{}]: ", fmt::format(fg(fmt::color::gold), "jak3")); + } else { + message += fmt::format(" [{}]: ", fmt::format(fg(fmt::color::magenta), "jakx")); + } + const auto loaded_projects_str = fmt::format("{}", fmt::join(loaded_projects, ",")); + message += fmt::format(fg(fmt::color::gray), "{}\n", loaded_projects_str); + message += fmt::format(fmt::emphasis::bold | fg(fmt::color::orange), " . --- ."); + message += + fmt::format(" Project Path: {}\n", + fmt::format(fg(fmt::color::gray), file_util::get_jak_project_dir().string())); + message += fmt::format(fmt::emphasis::bold | fg(fmt::color::orange), " - :===: -"); + message += " nREPL:"; + if (!nrepl_alive) { + message += fmt::format(fg(fmt::color::red), "DISABLED\n"); + } else { + message += fmt::format(fg(fmt::color::light_green), " Listening on {}\n", + repl_config.get_nrepl_port()); + } + message += fmt::format(fmt::emphasis::bold | fg(fmt::color::orange), " --. .--: :--. .--"); + message += " Source File Search Dirs: "; + const auto search_dir_string = + fmt::format("{}", fmt::join(repl_config.asm_file_search_dirs, ",")); + message += fmt::format("[{}]\n", fmt::format(fg(fmt::color::gray), search_dir_string)); + message += fmt::format(fmt::emphasis::bold | fg(fmt::color::orange), " .=======. =======."); + message += fmt::format(" {} or {} for basic help and usage\n", + fmt::format(fg(fmt::color::cyan), "(repl-help)"), + fmt::format(fg(fmt::color::cyan), "(repl-keybinds)")); + message += fmt::format(fmt::emphasis::bold | fg(fmt::color::orange), " .-=====-. .-=====-"); + message += + fmt::format(" {} to connect to the game\n", fmt::format(fg(fmt::color::cyan), "(lt)")); + message += fmt::format(fmt::emphasis::bold | fg(fmt::color::orange), " .-===========-."); + message += fmt::format(" {} to recompile the active project.\n", + fmt::format(fg(fmt::color::cyan), "(mi)")); + message += fmt::format(fmt::emphasis::bold | fg(fmt::color::orange), " .-===-.\n"); + message += fmt::format(fmt::emphasis::bold | fg(fmt::color::orange), " .\n"); + fmt::print("{}", message); } void Wrapper::print_to_repl(const std::string& str) { @@ -242,20 +272,24 @@ StartupFile load_user_startup_file(const std::string& username, const GameVersio return startup_file; } -REPL::Config load_repl_config(const std::string& username, const GameVersion game_version) { +REPL::Config load_repl_config(const std::string& username, + const GameVersion game_version, + const int nrepl_port) { auto repl_config_path = file_util::get_jak_project_dir() / "goal_src" / "user" / username / "repl-config.json"; + REPL::Config loaded_config(game_version); if (file_util::file_exists(repl_config_path.string())) { try { - REPL::Config config(game_version); auto repl_config_data = parse_commented_json(file_util::read_text_file(repl_config_path), "repl-config.json"); - from_json(repl_config_data, config); - return config; + from_json(repl_config_data, loaded_config); + loaded_config.temp_nrepl_port = nrepl_port; + return loaded_config; } catch (std::exception& e) { - REPL::Config config(game_version); + // do nothing } } - return REPL::Config(game_version); + loaded_config.temp_nrepl_port = nrepl_port; + return loaded_config; } } // namespace REPL diff --git a/common/repl/util.h b/common/repl/repl_wrapper.h similarity index 76% rename from common/repl/util.h rename to common/repl/repl_wrapper.h index 0f2510466a..6ba81ad87a 100644 --- a/common/repl/util.h +++ b/common/repl/repl_wrapper.h @@ -17,18 +17,20 @@ struct StartupFile { }; class Wrapper { - replxx::Replxx repl; - public: std::string username; Config repl_config; StartupFile startup_file; + bool nrepl_alive = false; std::vector examples{}; std::vector> regex_colors{}; Wrapper(GameVersion version) : repl_config(version) {} - Wrapper(const std::string& _username, const Config& config, const StartupFile& startup) - : username(_username), repl_config(config), startup_file(startup) {} + Wrapper(const std::string& _username, + const Config& config, + const StartupFile& startup, + bool nrepl_alive) + : username(_username), repl_config(config), startup_file(startup), nrepl_alive(nrepl_alive) {} replxx::Replxx& get_repl() { return repl; } void init_settings(); void reload_startup_file(); @@ -36,7 +38,7 @@ class Wrapper { // Functionality / Commands void clear_screen(); void print_to_repl(const std::string& str); - void print_welcome_message(); + void print_welcome_message(const std::vector& loaded_projects); void set_history_max_size(size_t len); const char* readline(const std::string& prompt); void add_to_history(const std::string& line); @@ -47,11 +49,14 @@ class Wrapper { std::pair get_current_repl_token(std::string const& context); private: + replxx::Replxx repl; replxx::Replxx::key_press_handler_t commit_text_action(std::string text_to_commit); std::vector keybindings = {}; }; std::string find_repl_username(); StartupFile load_user_startup_file(const std::string& username, const GameVersion game_version); -REPL::Config load_repl_config(const std::string& username, const GameVersion game_version); +REPL::Config load_repl_config(const std::string& username, + const GameVersion game_version, + const int nrepl_port); } // namespace REPL diff --git a/common/util/FileUtil.cpp b/common/util/FileUtil.cpp index b6b4768dcd..f150801cc1 100644 --- a/common/util/FileUtil.cpp +++ b/common/util/FileUtil.cpp @@ -185,9 +185,11 @@ std::optional try_get_jak_project_path() { return try_get_project_path_from_path(get_current_executable_path()); } -std::optional try_get_data_dir() { +std::optional try_get_data_dir(bool skip_logs) { fs::path my_path = get_current_executable_path(); - lg::info("Current executable directory - {}", my_path.string()); + if (!skip_logs) { + lg::debug("Current executable directory - {}", my_path.string()); + } auto data_dir = my_path.parent_path() / "data"; if (fs::exists(data_dir) && fs::is_directory(data_dir)) { return std::make_optional(data_dir); @@ -196,7 +198,7 @@ std::optional try_get_data_dir() { } } -bool setup_project_path(std::optional project_path_override) { +bool setup_project_path(std::optional project_path_override, bool skip_logs) { if (g_file_path_info.initialized) { return true; } @@ -204,16 +206,20 @@ bool setup_project_path(std::optional project_path_override) { if (project_path_override) { g_file_path_info.path_to_data_folder = fs::absolute(project_path_override.value()); g_file_path_info.initialized = true; - lg::info("Using explicitly set project path: {}", - g_file_path_info.path_to_data_folder.string()); + if (!skip_logs) { + lg::debug("Using explicitly set project path: {}", + g_file_path_info.path_to_data_folder.string()); + } return true; } - auto data_path = try_get_data_dir(); + auto data_path = try_get_data_dir(skip_logs); if (data_path) { g_file_path_info.path_to_data_folder = *data_path; g_file_path_info.initialized = true; - lg::info("Using data path: {}", data_path->string()); + if (!skip_logs) { + lg::debug("Using data path: {}", data_path->string()); + } return true; } @@ -221,7 +227,9 @@ bool setup_project_path(std::optional project_path_override) { if (development_repo_path) { g_file_path_info.path_to_data_folder = *development_repo_path; g_file_path_info.initialized = true; - lg::info("Using development repo path: {}", *development_repo_path); + if (!skip_logs) { + lg::debug("Using development repo path: {}", *development_repo_path); + } return true; } diff --git a/common/util/FileUtil.h b/common/util/FileUtil.h index dcaecdd2e8..16875c77bf 100644 --- a/common/util/FileUtil.h +++ b/common/util/FileUtil.h @@ -43,7 +43,7 @@ bool create_dir_if_needed_for_file(const std::string& path); bool create_dir_if_needed_for_file(const fs::path& path); std::string get_current_executable_path(); std::optional try_get_project_path_from_path(const std::string& path); -bool setup_project_path(std::optional project_path_override); +bool setup_project_path(std::optional project_path_override, bool skip_logs = false); void override_user_config_dir(fs::path user_config_dir_override, bool use_overridden_config_dir_for_saves); std::string get_file_path(const std::vector& path); diff --git a/game/kernel/jak2/kboot.cpp b/game/kernel/jak2/kboot.cpp index 6e9d44f9fb..ce50468e94 100644 --- a/game/kernel/jak2/kboot.cpp +++ b/game/kernel/jak2/kboot.cpp @@ -6,7 +6,7 @@ #include "common/goal_constants.h" #include "common/log/log.h" -#include "common/repl/util.h" +#include "common/repl/repl_wrapper.h" #include "common/util/Timer.h" #include "game/common/game_common_types.h" diff --git a/game/kernel/jak3/kboot.cpp b/game/kernel/jak3/kboot.cpp index d2aaba79d5..bcc8acfcaa 100644 --- a/game/kernel/jak3/kboot.cpp +++ b/game/kernel/jak3/kboot.cpp @@ -3,7 +3,7 @@ #include #include "common/log/log.h" -#include "common/repl/util.h" +#include "common/repl/repl_wrapper.h" #include "common/util/Timer.h" #include "game/common/game_common_types.h" diff --git a/game/tools/subtitle_editor/subtitle_editor.cpp b/game/tools/subtitle_editor/subtitle_editor.cpp index 9222174ab6..687bebdb4b 100644 --- a/game/tools/subtitle_editor/subtitle_editor.cpp +++ b/game/tools/subtitle_editor/subtitle_editor.cpp @@ -197,7 +197,7 @@ void SubtitleEditor::draw_repl_options() { ImGui::Text("REPL Connected, should be good to go!"); ImGui::PopStyleColor(); } else { - if (ImGui::Button("Connect to REPL")) { + if (ImGui::Button("Connect to REPL on Port 8181")) { m_repl.connect(); if (!m_repl.is_connected()) { ImGui::PushStyleColor(ImGuiCol_Text, m_error_text_color); diff --git a/game/tools/subtitle_editor/subtitle_editor_repl_client.cpp b/game/tools/subtitle_editor/subtitle_editor_repl_client.cpp index b5da6cc568..57b3c83706 100644 --- a/game/tools/subtitle_editor/subtitle_editor_repl_client.cpp +++ b/game/tools/subtitle_editor/subtitle_editor_repl_client.cpp @@ -8,9 +8,6 @@ SubtitleEditorReplClient::SubtitleEditorReplClient() { int port = 8181; - if (g_game_version == GameVersion::Jak2) { - port = 8182; - } m_repl = std::make_unique(port); } diff --git a/goal_src/goal-lib.gc b/goal_src/goal-lib.gc index 440c942e7c..6f67b58f6a 100644 --- a/goal_src/goal-lib.gc +++ b/goal_src/goal-lib.gc @@ -1263,14 +1263,11 @@ (#cond ((eq? GAME_VERSION 'jak1) (asm-file "goal_src/jak1/compiler-setup.gc") - (seval (fmt #t "Jak 1 Mode\n")) ) ((eq? GAME_VERSION 'jak2) (asm-file "goal_src/jak2/compiler-setup.gc") - (seval (fmt #t "Jak 2 Mode\n")) ) ((eq? GAME_VERSION 'jak3) (asm-file "goal_src/jak3/compiler-setup.gc") - (seval (fmt #t "Jak 3 Mode\n")) ) ) diff --git a/goal_src/goos-lib.gs b/goal_src/goos-lib.gs index 8624807ba5..97a77606cf 100644 --- a/goal_src/goos-lib.gs +++ b/goal_src/goos-lib.gs @@ -471,7 +471,7 @@ ;; *user* is defined when goos starts! (when *user* - (fmt #t "Loading user scripts for user: {}...\n" *user*) + ;; (fmt #t "Loading user scripts for user: {}...\n" *user*) ;; i'm not sure what naming scheme to use here. user//user.gs? ;; the GOAL one is loaded in Compiler.cpp (try-load-file (fmt #f "goal_src/user/{}/user.gs" *user*)) @@ -512,4 +512,4 @@ (define *default-territory* GAME_TERRITORY_SCEA) ;; whether to enable ps3 test levels for jak 2 -(define USE_PS3_LEVELS #f) \ No newline at end of file +(define USE_PS3_LEVELS #f) diff --git a/goalc/build_actor/common/MercExtract.cpp b/goalc/build_actor/common/MercExtract.cpp index f56c48a920..1aceda7140 100644 --- a/goalc/build_actor/common/MercExtract.cpp +++ b/goalc/build_actor/common/MercExtract.cpp @@ -2,7 +2,7 @@ #include "common/log/log.h" -#include +#include "goalc/build_level/common/gltf_mesh_extract.h" void extract(const std::string& name, MercExtractData& out, diff --git a/goalc/build_actor/common/MercExtract.h b/goalc/build_actor/common/MercExtract.h index 6eea2e3ec7..7653b93d03 100644 --- a/goalc/build_actor/common/MercExtract.h +++ b/goalc/build_actor/common/MercExtract.h @@ -2,7 +2,7 @@ #include "common/util/gltf_util.h" -#include +#include "goalc/build_actor/jak1/build_actor.h" struct MercExtractData { gltf_util::TexturePool tex_pool; diff --git a/goalc/compiler/Compiler.cpp b/goalc/compiler/Compiler.cpp index 2dccd8297d..9362880ea8 100644 --- a/goalc/compiler/Compiler.cpp +++ b/goalc/compiler/Compiler.cpp @@ -65,7 +65,7 @@ Compiler::Compiler(GameVersion version, if (m_repl) { m_repl->load_history(); // init repl - m_repl->print_welcome_message(); + m_repl->print_welcome_message(m_make.get_loaded_projects()); auto& examples = m_repl->examples; auto& regex_colors = m_repl->regex_colors; m_repl->init_settings(); diff --git a/goalc/compiler/Compiler.h b/goalc/compiler/Compiler.h index b299ad0c96..994e844cab 100644 --- a/goalc/compiler/Compiler.h +++ b/goalc/compiler/Compiler.h @@ -5,7 +5,7 @@ #include #include "common/goos/Interpreter.h" -#include "common/repl/util.h" +#include "common/repl/repl_wrapper.h" #include "common/type_system/TypeSystem.h" #include "goalc/compiler/CompilerException.h" diff --git a/goalc/compiler/compilation/CompilerControl.cpp b/goalc/compiler/compilation/CompilerControl.cpp index 7fcdd33b83..61929e5408 100644 --- a/goalc/compiler/compilation/CompilerControl.cpp +++ b/goalc/compiler/compilation/CompilerControl.cpp @@ -6,7 +6,7 @@ #include #include -#include "common/repl/util.h" +#include "common/repl/repl_wrapper.h" #include "common/util/DgoWriter.h" #include "common/util/FileUtil.h" #include "common/util/Timer.h" diff --git a/goalc/compiler/compilation/Define.cpp b/goalc/compiler/compilation/Define.cpp index fdde59bf03..a2351fb27c 100644 --- a/goalc/compiler/compilation/Define.cpp +++ b/goalc/compiler/compilation/Define.cpp @@ -66,15 +66,22 @@ Val* Compiler::compile_define(const goos::Object& form, const goos::Object& rest throw_compiler_error(form, "Cannot define {} because it cannot be set.", sym_val->print()); } + auto explicit_no_typecheck = false; + if (args.has_named("no-typecheck")) { + explicit_no_typecheck = get_true_or_false(form, args.named.at("no-typecheck")); + } auto existing_type = m_symbol_types.lookup(sym.as_symbol()); if (!existing_type) { m_symbol_types.set(sym.as_symbol(), in_gpr->type()); } else { - bool do_typecheck = true; - if (args.has_named("no-typecheck")) { - do_typecheck = !get_true_or_false(form, args.named.at("no-typecheck")); - } - if (do_typecheck) { + if (!explicit_no_typecheck && m_repl && m_repl->repl_config.permissive_redefinitions) { + // Permissive redefinitions are allowed + if (in_gpr->type() != *existing_type) { + lg::warn("Redefining {}", sym.as_symbol().name_ptr); + } + m_symbol_types.set(sym.as_symbol(), in_gpr->type()); + } else if (!explicit_no_typecheck) { + // Type check is required typecheck(form, *existing_type, in_gpr->type(), fmt::format("define on existing symbol {}", sym.as_symbol().name_ptr)); } diff --git a/goalc/main.cpp b/goalc/main.cpp index b080243c9d..c5ec7c8d3b 100644 --- a/goalc/main.cpp +++ b/goalc/main.cpp @@ -3,7 +3,7 @@ #include "common/log/log.h" #include "common/repl/nrepl/ReplServer.h" -#include "common/repl/util.h" +#include "common/repl/repl_wrapper.h" #include "common/util/FileUtil.h" #include "common/util/diff.h" #include "common/util/string_util.h" @@ -18,13 +18,13 @@ #include "third-party/CLI11.hpp" void setup_logging(const bool disable_ansi_colors) { - lg::set_file("compiler"); lg::set_file_level(lg::level::info); lg::set_stdout_level(lg::level::info); lg::set_flush_level(lg::level::info); if (disable_ansi_colors) { lg::disable_ansi_colors(); } + lg::set_file("compiler"); lg::initialize(); } @@ -39,13 +39,11 @@ int main(int argc, char** argv) { fs::path project_path_override; // TODO - a lot of these flags could be deprecated and moved into `repl-config.json` - // TODO - auto-find the user if there is only one folder within `user/` CLI::App app{"OpenGOAL Compiler / REPL"}; app.add_option("-c,--cmd", cmd, "Specify a command to run, no REPL is launched in this mode"); app.add_option("-u,--user", username, "Specify the username to use for your user profile in 'goal_src/user/'"); - app.add_option("-p,--port", nrepl_port, - "Specify the nREPL port. Defaults to 8181 for Jak 1 and 8182 for Jak 2"); + app.add_option("-p,--port", nrepl_port, "Specify the nREPL port. Defaults to 8181"); app.add_flag("--user-auto", auto_find_user, "Attempt to automatically deduce the user, overrides '--user'"); app.add_option("-g,--game", game, "The game name: 'jak1' or 'jak2'"); @@ -56,28 +54,17 @@ int main(int argc, char** argv) { CLI11_PARSE(app, argc, argv); GameVersion game_version = game_name_to_version(game); - if (nrepl_port == -1) { - switch (game_version) { - default: - case GameVersion::Jak1: - nrepl_port = 8181; - break; - case GameVersion::Jak2: - nrepl_port = 8182; - break; - } - } if (!project_path_override.empty()) { if (!fs::exists(project_path_override)) { lg::error("Error: project path override '{}' does not exist", project_path_override.string()); return 1; } - if (!file_util::setup_project_path(project_path_override)) { + if (!file_util::setup_project_path(project_path_override, true)) { lg::error("Could not setup project path!"); return 1; } - } else if (!file_util::setup_project_path(std::nullopt)) { + } else if (!file_util::setup_project_path(std::nullopt, true)) { return 1; } @@ -88,8 +75,6 @@ int main(int argc, char** argv) { return 1; } - lg::info("OpenGOAL Compiler {}.{}", versions::GOAL_VERSION_MAJOR, versions::GOAL_VERSION_MINOR); - // Figure out the username if (auto_find_user) { username = REPL::find_repl_username(); @@ -97,7 +82,7 @@ int main(int argc, char** argv) { // Load the user's startup file auto startup_file = REPL::load_user_startup_file(username, game_version); // Load the user's REPL config - auto repl_config = REPL::load_repl_config(username, game_version); + auto repl_config = REPL::load_repl_config(username, game_version, nrepl_port); // Init Compiler std::unique_ptr compiler; @@ -126,16 +111,16 @@ int main(int argc, char** argv) { // Initialize nREPL server socket std::function shutdown_callback = [&]() { return status == ReplStatus::WANT_EXIT; }; - ReplServer repl_server(shutdown_callback, nrepl_port); - bool repl_server_ok = repl_server.init_server(); + ReplServer repl_server(shutdown_callback, repl_config.get_nrepl_port()); + bool nrepl_server_ok = repl_server.init_server(true); std::thread nrepl_thread; // the compiler may throw an exception if it fails to load its standard library. try { compiler = std::make_unique( game_version, std::make_optional(repl_config), username, - std::make_unique(username, repl_config, startup_file)); + std::make_unique(username, repl_config, startup_file, nrepl_server_ok)); // Start nREPL Server if it spun up successfully - if (repl_server_ok) { + if (nrepl_server_ok) { nrepl_thread = std::thread([&]() { while (!shutdown_callback()) { auto resp = repl_server.get_msg(); @@ -161,7 +146,7 @@ int main(int argc, char** argv) { } compiler = std::make_unique( game_version, std::make_optional(repl_config), username, - std::make_unique(username, repl_config, startup_file)); + std::make_unique(username, repl_config, startup_file, nrepl_server_ok)); status = ReplStatus::OK; } // process user input @@ -180,7 +165,7 @@ int main(int argc, char** argv) { // TODO - investigate why there is such a delay when exitting // Cleanup - if (repl_server_ok) { + if (nrepl_server_ok) { repl_server.shutdown_server(); nrepl_thread.join(); } diff --git a/goalc/make/MakeSystem.cpp b/goalc/make/MakeSystem.cpp index 2a0e2d8475..982e8e3dd6 100644 --- a/goalc/make/MakeSystem.cpp +++ b/goalc/make/MakeSystem.cpp @@ -119,8 +119,9 @@ void MakeSystem::load_project_file(const std::string& file_path) { auto data = m_goos.reader.read_from_file({file_path}); // interpret it, which will call various handlers. m_goos.eval(data, m_goos.global_environment.as_env_ptr()); - lg::print("Loaded project {} with {} steps in {} ms\n", file_path, m_output_to_step.size(), + lg::debug("Loaded project {} with {} steps in {} ms\n", file_path, m_output_to_step.size(), (int)timer.getMs()); + m_loaded_projects.push_back(file_path); } goos::Object MakeSystem::handle_defstep(const goos::Object& form, @@ -187,6 +188,7 @@ goos::Object MakeSystem::handle_defstep(const goos::Object& form, * */ void MakeSystem::clear_project() { + m_loaded_projects.clear(); m_output_to_step.clear(); } diff --git a/goalc/make/MakeSystem.h b/goalc/make/MakeSystem.h index 413a072596..cba37ebf77 100644 --- a/goalc/make/MakeSystem.h +++ b/goalc/make/MakeSystem.h @@ -70,6 +70,7 @@ class MakeSystem { } void clear_project(); + std::vector get_loaded_projects() const { return m_loaded_projects; } /*! * Get the prefix that the project has requested for all compiler outputs @@ -91,6 +92,7 @@ class MakeSystem { goos::Interpreter m_goos; std::optional m_repl_config; + std::vector m_loaded_projects; std::unordered_map> m_output_to_step; std::unordered_map> m_tools; diff --git a/scripts/examples/nrepl-test.py b/scripts/examples/nrepl-test.py index 81145e7e7e..33e00201ad 100644 --- a/scripts/examples/nrepl-test.py +++ b/scripts/examples/nrepl-test.py @@ -1,5 +1,7 @@ import socket import struct +from time import sleep + clientSocket = socket.socket(socket.AF_INET, socket.SOCK_STREAM); clientSocket.connect(("127.0.0.1", 8181)) print(clientSocket) @@ -11,5 +13,5 @@ form = "(:status)" header = struct.pack('