-
Notifications
You must be signed in to change notification settings - Fork 0
Refactoring data handler #22
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
…ed safety and clarity to work with cpp 20
…SULT` for clarity and cpp20 compliance
…FileIO infrastructure same for legacy
… X-macro dependencies
…tency and standardized format handling.
…istency and clarity
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 focuses on modernizing the codebase by adopting C++20 features and significantly refactoring the data handling architecture. It introduces a generic, metadata-driven system for saving, loading, and tracking changes in game data, replacing older, less flexible implementations. Additionally, it improves code quality through widespread const-correctness enhancements and updates UI constant definitions for better type safety and precision. 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 pull request introduces a significant refactoring of the data handling and saving mechanisms, moving towards a more generic and maintainable architecture. Key improvements include the replacement of X-macros with a metadata-driven field comparison system, the introduction of a CommonDataSaver for generalized data persistence, and a unified DataFileIO for file operations. These changes greatly enhance code readability, reduce duplication, and provide a flexible framework for managing different data types (items, skills) with support for legacy formats and change tracking. The documentation for the translation system has also been updated to reflect the new file structure.
| { | ||
| DWORD calculatedChecksum = GenerateCheckSum2(buffer, bufferSize, 0xE2F1); | ||
| int bufferSize = config.itemSize * config.itemCount; | ||
| DWORD calculatedChecksum = GenerateCheckSum2(const_cast<BYTE*>(buffer), bufferSize, config.checksumKey); |
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.
The VerifyChecksum function passes const_cast<BYTE*>(buffer) to GenerateCheckSum2. This suggests that GenerateCheckSum2 might modify the buffer, which is highly unusual for a checksum calculation function. If GenerateCheckSum2 does not modify the buffer, its first parameter should be const BYTE* to enforce const-correctness. If it does modify the buffer, this is a significant side-effect that should be documented and potentially re-evaluated, as checksum functions are typically read-only operations.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The convertToFile lambda uses const_cast<TRuntime&>(config.runtimeData[i]). This implies that the convertToFile function might modify the TRuntime object, despite config.runtimeData being declared as const TRuntime*. If convertToFile truly needs to modify TRuntime, then config.runtimeData should not be const. If it does not, convertToFile should take const TRuntime& to ensure const-correctness and prevent accidental modifications.
| float oldVal = *(const float*)oldPtr; | ||
| float newVal = *(const float*)newPtr; | ||
| if (oldVal != newVal) | ||
| { |
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.
No description provided.