Skip to content

Deduplicate Shuffle Logic - #249

Open
foohyfooh wants to merge 7 commits into
IceYGO:masterfrom
foohyfooh:general_improvements_2_deduplicate_shuffle
Open

Deduplicate Shuffle Logic#249
foohyfooh wants to merge 7 commits into
IceYGO:masterfrom
foohyfooh:general_improvements_2_deduplicate_shuffle

Conversation

@foohyfooh

Copy link
Copy Markdown
Contributor

Move shuffle changes out of PR #248

Use the shuffle functions directly in places where it won't affect the readability of the existing line too much
Comment thread Game/AI/Decks/YubelExecutor.cs Outdated
Comment thread Game/AI/Decks/ThunderDragonExecutor.cs
Comment thread Game/AI/AIUtil.cs
- Fix Fisher-Yates shuffle algorithm
- Fix Yubel recursive reference to ShuffleList

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

Centralizes duplicated deck shuffle logic in AIUtil.

Changes:

  • Adds shared list/card shuffle helpers.
  • Migrates 16 deck executors to shared helpers.
  • Simplifies two Thunder Dragon swaps.

The deduplication is useful, but the shared shuffle is biased and three tuple swaps violate required C# 6 compatibility.

Reviewed changes

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

Show a summary per file
File Description
Game/AI/AIUtil.cs Adds shared shuffle helpers.
Game/AI/Decks/YubelExecutor.cs Uses shared shuffling.
Game/AI/Decks/VoicelessExecutor.cs Removes local shuffle helper.
Game/AI/Decks/TrickstarExecutor.cs Uses shared target/place shuffling.
Game/AI/Decks/ThunderDragonExecutor.cs Uses shared shuffling and tuple swaps.
Game/AI/Decks/SwordsoulExecutor.cs Replaces local card shuffling.
Game/AI/Decks/SalamangreatExecutor.cs Centralizes zone-order shuffling.
Game/AI/Decks/SacredBeastExecutor.cs Delegates shuffling to AIUtil.
Game/AI/Decks/RyzealExecutor.cs Delegates shuffling to AIUtil.
Game/AI/Decks/MalissExecutor.cs Delegates both shuffle helpers.
Game/AI/Decks/LabrynthExecutor.cs Delegates shuffling to AIUtil.
Game/AI/Decks/ExosisterExecutor.cs Replaces local target shuffling.
Game/AI/Decks/DogmatikaExecutor.cs Delegates card shuffling.
Game/AI/Decks/BE2025Executor.cs Removes duplicate shuffle implementations.
Game/AI/Decks/ArchfiendExecutor.cs Removes duplicate shuffle implementations.
Game/AI/Decks/ApophisExecutor.cs Delegates shuffling to AIUtil.
Game/AI/Decks/AlbazExecutor.cs Delegates shuffling to AIUtil.
Suppressed comments (1)

Game/AI/Decks/ThunderDragonExecutor.cs:1377

  • This tuple/deconstruction swap is C# 7 syntax and is unsupported by the repository's required C# 6/older Mono toolchain. Use the prior temporary-variable swap so the project remains buildable on supported compilers.
                            (link_materials[i], link_materials[0]) = (link_materials[0], link_materials[i]);

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread Game/AI/AIUtil.cs Outdated
Comment on lines +583 to +584
int index = Program.Rand.Next(result.Count);
(result[n], result[index]) = (result[index], result[n]);
ClientCard temp = copy_cards[0];
copy_cards[0] = copy_cards[i];
copy_cards[i] = temp;
(copy_cards[i], copy_cards[0]) = (copy_cards[0], copy_cards[i]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It do pass the build, will test it later.
mercury233/libWindbot@788e68b

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