Refactoring data handler #23
Conversation
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 significantly refactors the core data handling logic within the project, moving from specialized, macro-heavy implementations to a more generic, template-based, and metadata-driven architecture. The changes aim to improve code reusability, maintainability, and clarity across different data types, particularly for item and skill management, by centralizing common operations like file I/O, saving, and change tracking. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant refactoring of the data handling system, moving from a macro-based, type-specific approach to a more generic, template-based, and metadata-driven design. Key improvements include a new generic DataFileIO for unified file I/O operations, a CommonDataSaver for orchestrating data saving with change tracking and legacy format support, and a FieldMetadata system that replaces X-macros for field comparison with modern C++20 constexpr arrays. This refactoring greatly enhances modularity, maintainability, and extensibility across the codebase by centralizing common logic and making data structure definitions more explicit.
| * Extract UTF-8 name from a struct with wchar_t Name field | ||
| */ | ||
| template<typename TStruct> | ||
| inline std::string GetNameUtf8(int index, const TStruct& data, int maxNameLen) |
There was a problem hiding this comment.
The maxNameLen parameter is passed to GetNameUtf8 but is not used within the function. This can be misleading and suggests either the parameter is redundant or its intended use is missing. Consider removing it if it's not needed, or implementing its logic if it has a purpose.
| inline std::string GetNameUtf8(int index, const TStruct& data, int maxNameLen) | |
| inline std::string GetNameUtf8(int index, const TStruct& data) |
| /** | ||
| * Compare two wide-character strings | ||
| */ | ||
| inline void CompareWideString(const char* fieldName, const wchar_t* oldVal, const wchar_t* newVal, |
There was a problem hiding this comment.
The maxLen parameter is passed to CompareWideString but is not used within the function. Similar to GetNameUtf8, this parameter is either redundant or its intended logic is missing. The current implementation uses fixed-size buffers (256 bytes), which might be sufficient for current MAX_ITEM_NAME and MAX_SKILL_NAME values, but using maxLen could provide more robustness against future changes or larger string sizes.
inline void CompareWideString(const char* fieldName, const wchar_t* oldVal, const wchar_t* newVal, std::stringstream& changes, bool& hasChanged)| memset(&fileStruct, 0, fileStructSize); | ||
|
|
||
| // Convert runtime to file format (cast away const for legacy Copy functions) | ||
| config.convertToFile(fileStruct, const_cast<TRuntime&>(config.runtimeData[i])); |
There was a problem hiding this comment.
The const_cast on config.runtimeData[i] suggests a potential const-correctness issue. config.runtimeData is const TRuntime*, implying the runtime data should not be modified during the conversion to file format. If the convertToFile lambda (and subsequently CopyItemAttributeToDestination or CopySkillAttributeToDestination) does not modify its src parameter, then src should be const TRuntime& in the lambda's signature to enforce const-correctness and avoid the const_cast.
…FileIO infrastructure same for legacy
… X-macro dependencies
…tency and standardized format handling.
…istency and clarity
Mosch0512
force-pushed
the
refactoring-data-handler
branch
from
aabd7fe to
659d259
Compare
…mparisons and adjust UTF-8 conversion logic accordingly.
…t` references for improved const-correctness.
… lambda callbacks.
…omparison methods.
…consistency and type clarity This ensures consistent type usage and proper conversion when calling functions that expect different integer types.
…directory structure
…nsistent type handling
…SkillAttributeToDestination` functions
No description provided.