Client main refactor #256
Client main refactor #256
Conversation
…atCommandEvent handling
- Updated `c_cpp_properties.json` with new "Win32" configuration and set `mergeConfigurations` to true. - Modified `settings.json` to add new C/C++ settings for case sensitivity, error squiggles, and default standards. - Added new hidden presets for Ninja Multi-Config with MSVC and Clang compilers in `CMakePresets.json`. - Refactored `imgui-system.cpp`: - Initialized `inifilename` and `logfilename` with local constants. - Removed keyboard mapping setup. - Added `GlfwKeyToImGuiKey` function for key code mapping. - Updated `KeyboardEvent::On` function for robust key event handling. - Used `SetTexID` in `CreateDeviceObjects` and restored last bound texture. - Changed C++ standard to C++20 in `helpers.cmake` for `add_program` and `add_interface_lib`. - Updated `vcpkg.json` to new `builtin-baseline` commit hash.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the client main.cpp file to improve code organization and introduces several infrastructure improvements including C++20 adoption and ImGui updates.
- Extracted main function logic into a new Application class for better organization
- Updated project to use C++20 standard and modernized ImGui key handling
- Updated VCPKG baseline and added development environment configurations
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| client/main.cpp | Significantly simplified by moving logic to Application class |
| client/application.hpp/.cpp | New Application class encapsulating game initialization and main loop |
| client/imgui-system.hpp/.cpp | Updated constructor and modernized ImGui key mapping system |
| cmake/helpers.cmake | Upgraded C++ standard from 17 to 20 |
| vcpkg.json | Updated baseline to newer version |
| server/save-game.cpp | Added whitespace formatting to JSON output |
| Various config files | Added VS Code settings and CMake presets for development |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| IMGUISystem::IMGUISystem() { ImGui::CreateContext(); } | ||
|
|
||
| IMGUISystem::IMGUISystem(GLFWwindow* _window) { | ||
| ImGui::CreateContext(); | ||
| void IMGUISystem::Initialize(GLFWwindow* _window) { | ||
| IMGUISystem::window = _window; | ||
| auto& io = ImGui::GetIO(); | ||
| inifilename = (Path::GetUserSettingsPath() / "imgui.ini").toString(); | ||
| logfilename = (Path::GetUserCachePath() / "imgui_log.txt").toString(); | ||
| io.IniFilename = inifilename.c_str(); | ||
| const std::string ini_filename = (Path::GetUserSettingsPath() / "imgui.ini").toString(); | ||
| const std::string log_filename = (Path::GetUserCachePath() / "imgui_log.txt").toString(); | ||
| io.IniFilename = ini_filename.c_str(); | ||
| #if defined(DEBUG) || defined(_DEBUG) | ||
| io.LogFilename = logfilename.c_str(); | ||
| io.LogFilename = log_filename.c_str(); |
There was a problem hiding this comment.
The local string variables ini_filename and log_filename go out of scope at the end of the Initialize function, making the c_str() pointers invalid. ImGui may access these pointers later, leading to undefined behavior. Consider storing these as member variables or using a different approach to ensure the strings remain valid.
| "connect", | ||
| "connect [username][ip] : Connect to a server [ip] with the provided [username]", | ||
| [&](const std::string& args) { | ||
| std::vector<std::string> splitArgs = SplitString(args, " "); |
There was a problem hiding this comment.
The SplitString function call includes a space delimiter parameter, but in the original code at line 108 in main.cpp, SplitString was called without the delimiter parameter. This change in API usage could cause different splitting behavior if the default delimiter differs from a space character.
| if (splitArgs.size() >= 2) { | ||
| std::string username = splitArgs[0]; | ||
| std::string ip = splitArgs[1]; | ||
| spdlog::get("console_log")->info("Connecting to {}", ip); |
There was a problem hiding this comment.
Hard-coded logger name 'console_log' creates tight coupling. Consider using the existing log member variable instead of retrieving the logger by name.
| spdlog::get("console_log")->info("Connecting to {}", ip); | |
| log->info("Connecting to {}", ip); |
1 participant
Description
This is a refactor to organize and cleanup the main file. Additionally this fueled a desire to test against clang on MSVC, which led to updating the VCPKG baseline in an effort to fix protobuf (this failed), but this led to an update of IMGUI.
The IMGUI update is just a patch towards the correct fix of using the supplied backends:
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: