update forced in MSG_SELECT_CHAIN#2797
Merged
mercury233 merged 2 commits intoJun 29, 2025
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR updates the handling of MSG_SELECT_CHAIN to include a per‐option “forced” flag and adjusts buffer offsets and client logic accordingly. Key changes include:
- Adjust buffer skip calculations in various
Analyzemethods to match the new message layout. - Extend flag handling to treat
EDESC_OPERATIONandEDESC_RESETas bitmasks, and add a forced‐flag bit into the high byte. - Update auto‐chain logic to detect and auto‐select the forced option.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| gframe/tag_duel.cpp | Updated buffer skip for MSG_SELECT_CHAIN |
| gframe/single_mode.cpp | Mirrored buffer skip change in single‐player mode |
| gframe/single_duel.cpp | Mirrored buffer skip change in duel analysis |
| gframe/replay_mode.cpp | Mirrored buffer skip change in replay analysis |
| gframe/event_handler.cpp | Switched comparisons to bitmask checks for desc flags |
| gframe/duelclient.cpp | Refactored parsing to read per‐option forced flag, updated auto‐chain logic |
Comments suppressed due to low confidence (1)
gframe/duelclient.cpp:1765
- Add a unit or integration test to verify that when a forced option is present,
chkAutoChaincorrectly selects the forced index and ignores other settings.
if(mainGame->chkAutoChain->isChecked() && mainGame->dField.chain_forced && !(mainGame->always_chain || mainGame->chain_when_avail)) {
| for (int i = 0; i < count; ++i) { | ||
| int flag = BufferIO::ReadUInt8(pbuf); | ||
| int forced = BufferIO::ReadUInt8(pbuf); | ||
| flag |= forced << 8; |
There was a problem hiding this comment.
[nitpick] It may help future readers to document that the high byte of flag now carries the forced‐option bit (e.g. define an EDESC_FORCED mask), rather than using a raw shift.
Suggested change
| flag |= forced << 8; | |
| flag |= (forced * EDESC_FORCED); |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem:
In the Standby phase, Dark Contract with the Witch can select among 2 effects, ① is optional, ③ is forced.
But the client didn't know which is forced. If
chkAutoChainis enabled, the first effect will be selected to activate.Solution:
Update
MSG_SELECT_CHAIN, setforcedto each chain options.require Fluorohydride/ygopro-core#753