-
Notifications
You must be signed in to change notification settings - Fork 0
Add imgui and item editor #10
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
Conversation
…oss-compiler debugging support.
…n = 1` for consistent wide character handling.
…e `_USE_32BIT_TIME_T`, and automate DLL copying post-build.
…eLists.txt` for editor support.
…tor-specific functionality.
… paths, and streamline DLL copying logic in `CMakeLists.txt`.
…Lists.txt`. which get included in `Include.h`
…watch for C# changes, rebuild on updates, and link DLL to `Main`.
…dio launch configuration file.
…d clarify IDE-specific steps.
…actices and improve readability.
…o detect and respond to new/removed `.cs` files dynamically.
…th cross-platform build and run instructions
…c discovery and improve maintainability.
Summary of ChangesHello @Mosch0512, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant new feature: an in-game editor built with ImGui. The primary focus is on a robust Item Editor that allows real-time modification and export of game item attributes. Alongside this, a new internationalization system has been integrated to support multi-language content for both the game and the editor. The entire editor framework is designed with conditional compilation, ensuring that it is completely excluded from release builds to maintain performance and binary size. This change greatly enhances development and content creation workflows by providing powerful in-game tooling and a flexible translation system. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This is an impressive and extensive pull request that adds an in-game editor using ImGui, refactors item data handling into a more robust system, and introduces a well-designed translation framework. The use of data-driven design with X-Macros for item attributes is excellent for maintainability. The integration with the existing codebase, especially for input blocking and console redirection, is handled thoughtfully. My feedback focuses on further improving code quality and robustness by suggesting the use of RAII for memory management, refactoring duplicated code, making error handling less disruptive for a debug tool, and correcting a few minor issues in file paths and documentation.
Source Main 5.2/source/DataHandler/ItemData/ItemDataExporter.cpp
Outdated
Show resolved
Hide resolved
647353a to
64bc941
Compare
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This is an impressive pull request that adds a significant new feature: an in-game editor using ImGui. The implementation is very thorough, covering everything from the core editor logic, a metadata-driven item editor, console redirection, and robust input blocking to integrate smoothly with the game. The refactoring of item data handling using X-Macros and the introduction of a new translation system are also major improvements to the codebase's maintainability and extensibility. My feedback is focused on a few areas to improve memory safety by using modern C++ smart pointers and to increase the robustness of the new JSON parser. Overall, this is a high-quality and very well-executed feature addition.
Source Main 5.2/source/DataHandler/ItemData/ItemDataSaverLegacy.cpp
Outdated
Show resolved
Hide resolved
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This is an impressive pull request that adds a powerful in-game editor using ImGui and significantly refactors the item data handling system. The new architecture is well-structured, with clear separation of concerns for data handling, UI, and core logic. The use of X-Macros for item attributes is an excellent choice for maintainability. The console redirection and translation systems are also great additions.
My review focuses on improving robustness and safety, particularly around memory management and string handling in the new C++ code. I've identified a few critical areas that could lead to buffer overflows or memory leaks, and some medium-severity suggestions to improve code quality and maintainability. Overall, this is a fantastic contribution.
I am having trouble creating individual review comments. Click here to see my feedback.
Source Main 5.2/source/DataHandler/ItemData/ItemDataLoader.cpp (20-22)
This swprintf call is unsafe and can lead to a buffer overflow if the fileName is too long. It's recommended to use a safe alternative like swprintf_s or, even better, C++ string streams (std::wstringstream) to construct the error message. This vulnerability also exists on lines 63 and 72.
std::wstringstream ss;
ss << fileName << L" - File not exist.";
ItemDataFileIO::ShowErrorAndExit(ss.str().c_str());
Source Main 5.2/source/DataHandler/ItemData/ItemDataSaver.cpp (59)
This function uses a raw new[] allocation for originalItems (and for Buffer on line 109), with manual delete[] calls at multiple exit points. While the current logic appears to handle cleanup correctly, this approach is error-prone and can easily lead to memory leaks if new return paths are added in the future. Using smart pointers like std::unique_ptr would make the code safer and cleaner by leveraging RAII for automatic memory management.
Source Main 5.2/source/DataHandler/ItemData/ItemDataSaverLegacy.cpp (26)
This function uses a raw new[] allocation with a manual delete[]. This pattern is risky and can lead to memory leaks, especially if the function's logic becomes more complex. It's highly recommended to use std::unique_ptr to ensure the buffer is automatically deallocated, making the code safer and more modern C++.
Source Main 5.2/source/MuEditor/Config/MuEditorConfig.cpp (24)
Saving the configuration in the destructor can be problematic. It can lead to I/O at unexpected times (e.g., during program shutdown) and can cause issues if Save() were to throw an exception (which is disallowed from destructors). It's safer to have an explicit Save() call triggered by a specific user action or during a controlled shutdown sequence. The CMuEditorCore::Shutdown function already handles saving preferences, making this destructor call redundant.
Source Main 5.2/source/MuEditor/Core/MuEditorCore.cpp (90-99)
The logic for loading translation files from multiple possible paths is duplicated for editor.json and metadata.json. This could be extracted into a helper function to reduce code duplication and improve maintainability.
Source Main 5.2/source/Translation/i18n.cpp (13-75)
This custom JSON parser is very simple and may be brittle. It doesn't account for whitespace between tokens (e.g., "key" : "value") and only handles a limited set of escape sequences. While it works for the current translation files, it could easily break with slightly different formatting. For improved robustness, consider either enhancing the parser to handle whitespace and more escape sequences, or using a lightweight, header-only JSON library.
Source Main 5.2/source/ZzzInfomation.cpp (378-457)
The removal of the old OpenItemScript function and its replacement with the new ItemDataHandler is an excellent refactoring. It greatly improves modularity, readability, and maintainability by centralizing the data loading logic. Great work on this cleanup!
…g git submodules.
…ng, with locking to prevent parallel process issues.
5166682 to
865712b
Compare
…its architecture, domains, conditional compilation, file structure, usage, and performance considerations.
…e switching with fallback support.
…factor popups and buttons to use dynamic localization, and improve column visibility menu functionality.
…tem::create_directory`, and enhance error tracking in item editor
…r handling, and update project file for better translation file management.
…r improved maintainability and reduced redundancy.
…encies from `CMuEditorConsoleUI`.
…nd column visibility through a centralized config, replacing legacy file-based logic.
…ITEM_ATTRIBUTE_FILE_LEGACY` to reflect accurate 30-byte size.
Modified ShowErrorAndExit to display error dialog with icon but continue running, allowing the editor to handle errors gracefully rather than forcing termination. Error logging is preserved.
…lity Replaced manual character array manipulation with idiomatic std::string operations for escaping quotes in CSV export. This improves code readability and safety while maintaining the same functionality.
Replaced raw pointer allocation with std::unique_ptr for automatic memory management. This eliminates manual delete[] calls and prevents memory leaks in error paths. Updated all call sites accordingly.
Extracted common loading logic from LoadLegacyFormat and LoadNewFormat into a single template function LoadFormat<TFileFormat>. This reduces code duplication and improves maintainability while preserving all functionality including format-specific error messages.
Changed path separator from backslash to forward slash in MuEditorConfig.h to ensure portability across Windows and Unix-like systems. Windows APIs handle forward slashes correctly in paths.
Corrected documentation to accurately describe ConsoleStreamBuf as actively used for C++ stream redirection (cout/cerr), rather than "currently unused". Added clarification of all console capture sources.
…and `ItemDataSaverLegacy`.
…n string handling in error messages.
… remove redundant destructor-based save call
… and improve readability.
02386a3 to
562245e
Compare
No description provided.