Skip to content

Fix deck enchant selection state and confirmation - #8

Open
Ryson-32 wants to merge 2 commits into
CompleteDotTech:codex/endpoint-audit-fixesfrom
Ryson-32:codex/fix-deck-enchant-selection-pr85
Open

Fix deck enchant selection state and confirmation#8
Ryson-32 wants to merge 2 commits into
CompleteDotTech:codex/endpoint-audit-fixesfrom
Ryson-32:codex/fix-deck-enchant-selection-pr85

Conversation

@Ryson-32

@Ryson-32 Ryson-32 commented Jun 2, 2026

Copy link
Copy Markdown

Builds on Gennadiyev#85 and fixes the remaining deck enchant card selection issue.

Problem:

  • NDeckEnchantSelectScreen stores selected cards in its private _selectedCards set.
  • The card holder selected flags do not reflect that state, so selected_cards / selected_count can stay empty even when cards are visibly selected.
  • confirm_selection also needs a special path for this screen, because the final completion goes through the game's own ConfirmSelection / CheckIfSelectionComplete flow.

Fix:

  • Read selected cards from _selectedCards.
  • Read selector prefs from _prefs when needed.
  • Treat 0 selected cards as not confirmable for this enchant screen.
  • Confirm NDeckEnchantSelectScreen through the game's own ConfirmSelection.

Verified locally:

  • Release build passes with 0 warnings / 0 errors.
  • python scripts/audit_endpoints.py --skip-live passes.
  • uv run --project mcp python scripts/test_mcp_server.py passes.
  • Live Kifuda flow works: selected Bloodletting, Dominate, and Feed; selected_count=3; confirm returned to the shop; Kifuda was added; all three selected cards gained the enchant text.

Copilot AI review requested due to automatic review settings June 2, 2026 08:13

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request adds support for deck enchant selection screens (NDeckEnchantSelectScreen) by introducing reflection helpers to retrieve selection limits and selected card models, and updating the state builder to track these states. A review comment suggests a safer way to dynamically invoke the ConfirmSelection method to prevent a potential TargetParameterCountException if the method signature changes in the future.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread McpMod.Actions.cs Outdated
Comment on lines +791 to +792
var parameters = confirmMethod.GetParameters();
confirmMethod.Invoke(screen, parameters.Length == 0 ? null : new[] { confirmButton });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If the ConfirmSelection method signature has more than one parameter (e.g., due to future game updates or optional parameters), passing a single-element array new[] { confirmButton } will throw a TargetParameterCountException. It is safer to dynamically construct the arguments array to match the expected parameter count of the method.

            var parameters = confirmMethod.GetParameters();
            var args = parameters.Length == 0 ? null : new object?[parameters.Length];
            if (args != null)
            {
                args[0] = confirmButton;
            }
            confirmMethod.Invoke(screen, args);

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

Note

Copilot was unable to run its full agentic suite in this review.

Adds support for “deck enchant” card selection screens by improving selection-state detection, min/max selection handling, preview container discovery, and confirm behavior.

Changes:

  • Track selected cards via reflected _selectedCards and surface is_selected in built card state.
  • Read MinSelect/MaxSelect more robustly (directly or via _prefs) and incorporate them into confirm eligibility.
  • Add deck-enchant preview containers and custom confirm logic invoking ConfirmSelection via reflection.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
McpMod.StateBuilder.cs Enhances card selection state building for deck-enchant screens (selected flags, min/max select, preview detection, confirm eligibility).
McpMod.Helpers.cs Adds reflection helpers to read selector constraints and selected cards consistently.
McpMod.Actions.cs Adds deck-enchant-specific confirm flow that bypasses missing/disabled UI confirm buttons.

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

Comment thread McpMod.Actions.cs Outdated
Comment on lines +791 to +792
var parameters = confirmMethod.GetParameters();
confirmMethod.Invoke(screen, parameters.Length == 0 ? null : new[] { confirmButton });
Comment thread McpMod.Actions.cs Outdated

private static Dictionary<string, object?>? TryConfirmDeckEnchantSelection(NCardGridSelectionScreen screen)
{
if (screen.GetType().Name != "NDeckEnchantSelectScreen")
Comment thread McpMod.Helpers.cs
Comment on lines +210 to +224
try
{
var property = type.GetProperty(memberName, Flags);
if (property != null && property.GetIndexParameters().Length == 0)
return property.GetValue(source);
}
catch { }

try
{
var field = type.GetField(memberName, Flags);
if (field != null)
return field.GetValue(source);
}
catch { }
@Ryson-32

Ryson-32 commented Jun 2, 2026

Copy link
Copy Markdown
Author

Addressed the first two bot review comments in follow-up commit 1a9daee:

  • Replaced repeated "NDeckEnchantSelectScreen" string checks with a centralized IsDeckEnchantSelectScreen(...) helper using the concrete type.
  • Updated the reflected ConfirmSelection invocation to explicitly support only 0- and 1-parameter signatures, and return a clear error for unsupported parameter counts.

Re-ran local verification:

  • Release build: 0 warnings / 0 errors
  • python scripts/audit_endpoints.py --skip-live: pass
  • uv run --project mcp python scripts/test_mcp_server.py: pass

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.

2 participants