Fix hardcoded wield inventory #16632
Open
+92
−3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
**Findings:** - A persistent build failure, caused by a segmentation fault during the compilation of `jsoncpp`, prevented the execution of static analysis and unit tests. - Manual code review of critical files (`main.cpp`, `server.cpp`, network stack, Lua API) revealed that the code is generally well-structured and follows good security practices. - Identified areas for improvement in input validation within the Lua API, command-line argument parsing, and network command handling. **Recommendations:** - Prioritize resolving the `jsoncpp` build failure to enable automated testing and static analysis. - Enhance input validation in the Lua API, particularly for functions that accept player names and positions. - Improve the robustness of command-line argument parsing and network command handling.
Proactive Code Audit and Build Issue Analysis
The player's wielded item was previously hardcoded to always come from the "main" inventory list. This limited the flexibility for mods or future game features that might require a different inventory setup for wieldable items. This change introduces a new member variable, `m_wield_list_name`, to the `Player` class, allowing the wield inventory list to be specified dynamically. - The `m_wield_list_name` is initialized to "main" to ensure backward compatibility with existing behavior. - The `getWieldedItem`, `setWieldIndex`, and `getMaxHotbarItemcount` functions were updated to use this new variable instead of the hardcoded "main" string. - Added `setWieldListName` and `getWieldListName` methods to manage the new variable. - A new unit test has been added to verify that items can be wielded from a custom-named inventory list.
The player's wielded item was previously hardcoded to always come from the "main" inventory list. This limited the flexibility for mods or future game features that might require a different inventory setup for wieldable items. This change introduces a new member variable, `m_wield_list_name`, to the `Player` class, allowing the wield inventory list to be specified dynamically. - The `m_wield_list_name` is initialized to "main" to ensure backward compatibility with existing behavior. - The `getWieldedItem`, `setWieldIndex`, and `getMaxHotbarItemcount` functions were updated to use this new variable instead of the hardcoded "main" string. - Added `setWieldListName` and `getWieldListName` methods to manage the new variable. - A new unit test has been added to verify that items can be wielded from a custom-named inventory list.
The player's wielded item was previously hardcoded to always come from the "main" inventory list. This limited the flexibility for mods or future game features that might require a different inventory setup for wieldable items. This change introduces a new member variable, `m_wield_list_name`, to the `Player` class, allowing the wield inventory list to be specified dynamically. - The `m_wield_list_name` is initialized to "main" to ensure backward compatibility with existing behavior. - The `getWieldedItem`, `setWieldIndex`, and `getMaxHotbarItemcount` functions were updated to use this new variable instead of the hardcoded "main" string. - Added `setWieldListName` and `getWieldListName` methods to manage the new variable. - A new unit test has been added to verify that items can be wielded from a custom-named inventory list.
|
Closely related: #15905. That PR also de-hardcodes the |
|
I would rather de-hardcode it all at once, "main" list, index offset, and combining multiple inventories, like #15905 does. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The player's wielded item was previously hardcoded to always come from the "main" inventory list. This limited the flexibility for mods or future game features that might require a different inventory setup for wieldable items.
This change introduces a new member variable,
m_wield_list_name, to thePlayerclass, allowing the wield inventory list to be specified dynamically.m_wield_list_nameis initialized to "main" to ensure backward compatibility with existing behavior.getWieldedItem,setWieldIndex, andgetMaxHotbarItemcountfunctions were updated to use this new variable instead of the hardcoded "main" string.setWieldListNameandgetWieldListNamemethods to manage the new variable.