Skip to content

fix show_decktop in field::send_to#833

Merged
mercury233 merged 1 commit into
Fluorohydride:masterfrom
mercury233:patch-show-decktop
Mar 30, 2026
Merged

fix show_decktop in field::send_to#833
mercury233 merged 1 commit into
Fluorohydride:masterfrom
mercury233:patch-show-decktop

Conversation

@mercury233

Copy link
Copy Markdown
Collaborator

问题

#821

修复

show_decktop 重新定义为 “卡组被动过,需要检查”

case 4(卡片移动前)

  • 移除了提前发送 MSG_DECK_TOP 的复杂逻辑
  • 简化为:当有卡从卡组离开时,标记 show_decktop

case 6(卡片实际移动)

  • 移除了无效条件判断:
    pcard->current.position == POS_FACEUP_DEFENSE
    (原因:移入卡组的卡在 move_cardadd_card 中会被强制设为 POS_FACEDOWN,该条件永远不成立)
  • 简化为:当有卡移入卡组时,标记 show_decktop

case 9(所有移动完成后)

  • 仅在确实需要时才发送 MSG_DECK_TOP
    • deck_reversed 生效
    • 或卡组顶部卡被 reverse_in_deck 翻面 (POS_FACEUP_DEFENSE)
  • 增加空卡组保护
  • 使用循环统一处理两个玩家

Issue

See #821

Fix

show_decktop is redefined as: "the deck has been modified and needs to be checked."

Case 4 (Before card movement)

  • Removed the complex logic that sent MSG_DECK_TOP in advance
  • Simplified to: mark show_decktop when a card leaves the deck

Case 6 (During actual card movement)

  • Removed the invalid condition check:
    pcard->current.position == POS_FACEUP_DEFENSE
    (Reason: cards moved into the deck are forcibly set to POS_FACEDOWN in add_card within move_card, so this condition never holds)
  • Simplified to: mark show_decktop when a card enters the deck

Case 9 (After all movements are completed)

  • MSG_DECK_TOP is sent only when necessary:
    • when deck_reversed is active
    • or when the top card of the deck is flipped by reverse_in_deck (POS_FACEUP_DEFENSE)
  • Added empty deck protection
  • Unified handling for both players using a loop

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts field::send_to’s deck-top update behavior when GLOBALFLAG_DECK_REVERSE_CHECK is enabled, aiming to ensure MSG_DECK_TOP is only emitted when the deck-top visibility truly needs recalculation based on the actual deck top.

Changes:

  • Simplifies deck-top tracking by treating show_decktop as “deck modified, needs check” and marking it when cards leave/enter the deck.
  • Removes an always-false condition when cards move into the deck and centralizes MSG_DECK_TOP emission in step/case 9.
  • Adds an empty-deck guard and unifies the per-player MSG_DECK_TOP emission via a loop.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread operations.cpp
Comment on lines +4249 to +4256
if(core.deck_reversed || (ptop->current.position == POS_FACEUP_DEFENSE)) {
pduel->write_buffer8(MSG_DECK_TOP);
pduel->write_buffer8(p);
pduel->write_buffer8(0);
if(ptop->current.position != POS_FACEUP_DEFENSE)
pduel->write_buffer32(ptop->data.code);
else
pduel->write_buffer32(ptop->data.code | 0x80000000);

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MSG_DECK_TOP is now always emitted with an offset/count of 0 (the third byte). Previously this logic computed how many consecutive cards were removed from the top of each deck during this send_to (similar to discard_deck and move_to_field), and sent that offset so clients can correctly shift any known/revealed deck-top state when multiple top cards leave the deck in one operation. With the offset hardcoded to 0, removing 1+ top cards via send_to will no longer convey the shift amount.

Suggested fix: track per-player “top cards removed” count in exargs (compute it in case 4 using the cards’ original LOCATION_DECK sequences before movement), and write that count instead of 0 when emitting MSG_DECK_TOP in case 9 (skipping emission when the deck becomes empty).

Copilot uses AI. Check for mistakes.
Comment thread operations.cpp
if(ptop->current.position != POS_FACEUP_DEFENSE)
pduel->write_buffer32(ptop->data.code);
else
pduel->write_buffer32(ptop->data.code | 0x80000000);

@vuryleo vuryleo Mar 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect there’s a game scenario where we need to hint to the client the identity of the top face-down card.
Even in scenarios like revealing 寄生虫パラサイド under 天変地異, we should only hint the game state to the client, not expose any code-level information.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vuryleo vuryleo Mar 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if(ptop->current.position != POS_FACEUP_DEFENSE)
	pduel->write_buffer32(ptop->data.code);

Sorry I should reply on these lines.

@mercury233 mercury233 marked this pull request as draft March 25, 2026 05:41
@mercury233

Copy link
Copy Markdown
Collaborator Author

Have been waiting for a ruling. Before KONAMI responds to the ruling, I suggest we merge this first to fix the potential crash issue.

@mercury233 mercury233 marked this pull request as ready for review March 28, 2026 23:49
@mercury233

Copy link
Copy Markdown
Collaborator Author

Got answer from KONAMI. They also do not know whether, when there is a face-up card in the deck, it is possible to determine that card’s sequence within the deck.

@mercury233 mercury233 merged commit 0ebe6af into Fluorohydride:master Mar 30, 2026
6 checks passed
@mercury233 mercury233 deleted the patch-show-decktop branch March 30, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants