Difference between revisions of "Category:Seek CSharp Cleanup"

Latest revision as of 21:20, 9 September 2024

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.

@@ Line 12: / Line 12: @@
 <br>
 B1. '''Presentation of the current code and function''': [[Image:bored.gif|link=]]<br>
-Here is the current implementation of the 'StartTargetingForSeek()' function in the '''Seek''' process.<br>
+Here is the current implementation of the `StartTargetingForSeek()` function in the '''Seek''' process.<br>
-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 unnecessarily.
+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.
 {| class="wikitable"
@@ Line 96: / Line 96: @@
 <br>
 D1. '''Optimized version of the code (with added functionality)''': [[Image:warning.gif|link=]]<br>
-With this, based on Alan's insightful suggestion, we introduce the utility function `SwapDeckPositions`, which swaps the positions of two cards (`SwapDeckPositions(i, lastIndex)`).<br>
+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.
-This approach eliminates completely the need to remove and re-add cards ('GameModel.RemoveDeck(card)' and 'GameModel.AddDeck(card)'), reducing the number of operations and improving the overall efficiency of the function.
 {| class="wikitable"
 |-
@@ Line 117: / Line 116: @@
      {
          ShadowEraCard deckCard = GameModel.GetDeck(i);
-         if (abilityTargets.Contains(deckCard))
+         if (abilityTargets.Contains(deckCard) && i != lastIndex) // Ensure swap is necessary
          {
              SwapDeckPositions(i, lastIndex); // Swap the matching card with the last card
@@ Line 140: / Line 139: @@
 </syntaxhighlight>
 |}
-D2. '''Justification of the optimized code''':
+D2. '''Justification of the optimized code''':<br>
-The `SwapDeckPositions` utility function eliminates unnecessary operations, improving both performance and maintainability. This method, although requiring the addition of a new function, provides a cleaner, more efficient way to handle the '''Seek''' process, making it especially valuable as the deck size increases.
+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.<br>
+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. <br><br>
+<blockquote>"You don’t want to manipulate the same list you are searching for in the same function."<br>
+(Tweaked)</blockquote>
 <br>
 <center><img src="https://www.shadowera.com/landing/img/blog-separator-2.png" ></center>
 <br>
-E. '''Conclusion''':<br>
-While the cleaned-up version is simple and improves readability, the optimized version with `SwapDeckPositions` takes the code to the next level. By reducing unnecessary operations, the new version will surely provide better performance and is more maintainable in the long term.<br>
-However, this approach involves adding new functionality, which may require more thorough testing to ensure it integrates smoothly with the rest of the game. It’s the more refined solution, but one that comes with its own considerations, such as the need for additional validation.<br>
-Moreover, if developers ever decide to increase the maximum number of cards in a deck during rated games, for example from 80 to 120, the swap-based approach will significantly improve performance. As the deck size grows, avoiding unnecessary loops and operations will reduce the overall processing time, making the optimized version even more valuable.<br>

Category

Difference between revisions of "Seek CSharp Cleanup"

From Shadow Era Wiki

Latest revision as of 21:20, 9 September 2024