Category:Seek CSharp Cleanup

Because even C# deserves a beauty treatment once in a while!

A. Summary of the situation:
This article is inspired by a key comment made by Alan, who pointed out that while the current in-game code works, it could be optimized for better efficiency and elegance. His insight into how the Seek process could be improved was the starting point for this deeper analysis.
The code presented here have been reviewed by a friend, a professional Haskell & Assembly Language developper.

B1. Presentation of the current code and function:
Here is the current implementation of the `StartTargetingForSeek()` function in the Seek process.
The code is functional, as we already all played the game, but it is heavy because it traverses the deck twice (`for (int i = 0; i < GameModel.DeckCount(); i++)`), removing and then re-adding (`GameModel.RemoveDeck(deck)` and `GameModel.AddDeck(card)`) cards.

public void StartTargetingForSeek()
{
    ClearTargets();
    MoveToCastPosition();
    GameModel.UnhighlightCards();
    GameModel.SetGameState(GameState.GameStateType.target);
 
    List<ShadowEraCard> abilityTargets = GetAbilityTargets();
 
    // Traversing the entire deck to remove matching cards
    for (int i = 0; i < GameModel.DeckCount(); i++)
    {
        ShadowEraCard deck = GameModel.GetDeck(i);
        if (abilityTargets.Contains(deck))
        {
            GameModel.RemoveDeck(deck); // Removing the card
            i--; // Adjusting the index after removal
        }
    }
 
    // Re-adding the cards to the deck
    for (int j = 0; j < abilityTargets.Count; j++)
    {
        GameModel.AddDeck(abilityTargets[j]); // Re-inserting the cards
    }
 
    GameModel.HighlightCards(GetAbilityTargets());
    DeckListDisplay.abilityCard = parent;
    DeckListDisplay.ShowDeck(GameModel.CurSide());
}

C1. Cleaned-up version of the code:
The key improvement here is that we only loop through the deck once (`for (int i = 0; i < GameModel.DeckCount(); i++)`), collecting the cards that match the condition in a temporary list (`cardsToMove.Add(deckCard)`), and then removing and re-adding them (`GameModel.RemoveDeck(card)` and `GameModel.AddDeck(card)`) in a single pass, making the code simpler and avoiding the need to adjust the index manually.
By avoiding the manual index adjustment (`i--`), the code becomes less error-prone and easier to understand.

public void StartTargetingForSeek()
{
    ClearTargets();
    MoveToCastPosition();
    GameModel.UnhighlightCards();
    GameModel.SetGameState(GameState.GameStateType.target);
 
    List<ShadowEraCard> abilityTargets = GetAbilityTargets();
    List<ShadowEraCard> cardsToMove = new List<ShadowEraCard>();
 
    // Single loop to collect matching cards
    for (int i = 0; i < GameModel.DeckCount(); i++)
    {
        ShadowEraCard deckCard = GameModel.GetDeck(i);
        if (abilityTargets.Contains(deckCard))
        {
            cardsToMove.Add(deckCard); // Adding the card to a temporary list
        }
    }
 
    // Remove and re-add the cards in a single pass
    foreach (ShadowEraCard card in cardsToMove)
    {
        GameModel.RemoveDeck(card); // Removing the cards
        GameModel.AddDeck(card);    // Re-adding them to the deck
    }
 
    GameModel.HighlightCards(GetAbilityTargets());
    DeckListDisplay.abilityCard = parent;
    DeckListDisplay.ShowDeck(GameModel.CurSide());
}

D1. Optimized version of the code (with added functionality):
With this, based on Alan's insightful suggestion, we introduce the utility function `SwapDeckPositions`, which swaps the positions of two cards (`SwapDeckPositions(i, lastIndex)`), ensuring that the swap only happens when necessary (`if (i != lastIndex)`). This approach modify the need to remove and re-add cards (`GameModel.RemoveDeck(card)` and `GameModel.AddDeck(card)`), reducing the number of operations.

public void StartTargetingForSeek()
{
    ClearTargets();
    MoveToCastPosition();
    GameModel.UnhighlightCards();
    GameModel.SetGameState(GameState.GameStateType.target);
 
    List<ShadowEraCard> abilityTargets = GetAbilityTargets();
 
    // Initialize an index pointing to the last card in the deck
    int lastIndex = GameModel.DeckCount() - 1;
 
    // Single loop to find and swap matching cards
    for (int i = 0; i <= lastIndex; i++)
    {
        ShadowEraCard deckCard = GameModel.GetDeck(i);
        if (abilityTargets.Contains(deckCard) && i != lastIndex) // Ensure swap is necessary
        {
            SwapDeckPositions(i, lastIndex); // Swap the matching card with the last card
            lastIndex--; // Reduce the search area by one
        }
    }
 
    GameModel.HighlightCards(GetAbilityTargets());
    DeckListDisplay.abilityCard = parent;
    DeckListDisplay.ShowDeck(GameModel.CurSide());
}
 
// New function to swap the positions of two cards in the deck
public static void SwapDeckPositions(int index1, int index2)
{
    if (index1 == index2) return; // If both indexes are the same, no need to swap
 
    ShadowEraCard temp = GameModel.GetDeck(index1); // Save the card at index1
    GameModel.SetDeck(index1, GameModel.GetDeck(index2)); // Move the card from index2 to index1
    GameModel.SetDeck(index2, temp); // Place the saved card into index2
}

D2. Justification of the optimized code:
The `SwapDeckPositions` utility function eliminates unnecessary operations, improving both performance and maintainability. By ensuring that swaps only happen when necessary (`if (i != lastIndex)`), this method reduces the number of memory accesses and writes.
That said, it is important to note that the introduction of `SwapDeckPositions` requires thorough testing to ensure that the function integrates well into the existing game logic and performs as expected.

"You don’t want to manipulate the same list you are searching for in the same function."
(Tweaked)

This category currently contains no pages or media.

Category

Seek CSharp Cleanup

From Shadow Era Wiki