update opParam of MSG_SELECT_SUM#2491
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
You don't maintain WindBot nor YGOPro2, so please don't ask anybody to do that. |
It's you who opened this PR, so you are expected to maintain the whole related ecosystem. |
Nobody is. Who merge this PR may consider about that but them aren't required to do that. |
There was a problem hiding this comment.
Pull Request Overview
The PR introduces a helper to centralize parsing of opParam into two operands and updates multiple sum‐selection methods to use this helper.
- Added
get_sum_paramstoClientFieldfor consistent bit‐field extraction, including special handling when the high bit flags a single parameter. - Replaced repeated manual masking/shifting of
opParamin various sum and tribute checks with calls toget_sum_params. - Updated
client_field.cppto implement the new helper and cleaned up duplicated logic.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| gframe/client_field.h | Declared get_sum_params to centralize opParam parsing |
| gframe/client_field.cpp | Implemented get_sum_params and refactored repeated bit logic |
Comments suppressed due to low confidence (3)
gframe/client_field.h:122
- [nitpick] The method name
get_sum_paramsuses snake_case while otherClientFieldmethods use PascalCase (e.g.,CheckSelectSum). Consider renaming toGetSumParamsto match the existing naming convention.
void get_sum_params(unsigned int opParam, int& op1, int& op2);
gframe/client_field.cpp:1294
- Add a comment or docstring explaining the purpose of
get_sum_params, especially the special-case logic when the high bit indicates a single-parameter encoding.
void ClientField::get_sum_params(unsigned int opParam, int& op1, int& op2) {
gframe/client_field.h:122
- No tests cover the new
get_sum_paramslogic. Consider adding unit tests for typical cases and edge cases (e.g., when the high bit is set) to ensure correctness.
void get_sum_params(unsigned int opParam, int& op1, int& op2);
| @@ -119,6 +119,7 @@ class ClientField: public irr::IEventReceiver { | |||
| bool ShowSelectSum(bool panelmode); | |||
| bool CheckSelectSum(); | |||
| bool CheckSelectTribute(); | |||
There was a problem hiding this comment.
[nitpick] get_sum_params appears to be an internal helper; consider making it private (or static) to limit its visibility and clarify its usage scope.
| bool CheckSelectTribute(); | |
| bool CheckSelectTribute(); | |
| private: |
Fluorohydride/ygopro-core#493