Избавление от ненужных циклов

Question

В моей игре у меня будет много взаимодействий, в которых мне нужно будет увидеть, есть ли у игрока предмет, а если он делает и что-то еще верно, то выполните действие , Описывается в следующем коде.

private void SetTinderInPit() 
{ 
    MouseState currentMouseState = Mouse.GetState(); 

    if (player.NextToFirePit == true) 
    { 
     foreach (Item item in player.PlayerInventory.Items) 
     { 
      if (item.ItemName == "tinder") 
      { 
       foreach (Item pit in allItemsOnGround) 
       { 
        if (pit.ItemName == "firepit" && 
         pit.ItemRectangle.Contains(MouseWorldPosition) && 
         currentMouseState.LeftButton == ButtonState.Pressed && 
         oldMouseState.LeftButton == ButtonState.Released) 
        { 
         item.ItemName = "empty"; 
         pit.ItemName = "firepitwithtinder"; 
         pit.Texture = Content.Load<Texture2D>("firepitwithtinder"); 
        } 
       } 
      } 
     } 
     oldMouseState = currentMouseState; 
    } 
} 

Как вы можете видеть, что это некрасиво, чтобы смотреть на, и я думаю, что было бы лучше, чтобы это сделать, но я не знаю, как. Поскольку таких методов будет много, мне интересно, что было бы лучшим способом добиться этого?

Answer 1

Похоже, вы могли бы избавиться от (на самом деле скрыть) петлю в целом с помощью некоторой LINQ:

private void SetTinderInPit() 
{ 
    MouseState currentMouseState = Mouse.GetState(); 

    if (player.NextToFirePit) 
    { 
     Item tinder = player.PlayerInventory.Items.FirstOrDefault(i => i.ItemName == "tinder"); 
     if (tinder != null) 
     { 
      Item firepit = allItemsOnGround.FirstOrDefault(i => i.ItemName == "firepit" && i.ItemRectangle.Contains(MouseWorldPosition)); 
      if (firepit != null && 
       currentMouseState.LeftButton == ButtonState.Pressed && 
       oldMouseState.LeftButton == ButtonState.Released) 
      { 
       tinder.ItemName = "empty"; 
       firepit.ItemName = "firepitwithtinder"; 
       firepit.Texture = Content.Load<Texture2D>("firepitwithtinder"); 
      } 
     } 
     oldMouseState = currentMouseState; 
    } 
} 

Это имеет дополнительное преимущество, замыкая цикл, когда элемент найден. Это также позволяет легко проверять вещи, отличные от имени (например, свойство IsFlammable или CanContainFire), поэтому вы можете использовать несколько элементов вместо «tinder» и «firepit».

Если вы на самом деле предназначен для удаления всех костровых чаш и трут, использование:

private void SetTinderInPit() 
{ 
    MouseState currentMouseState = Mouse.GetState(); 

    if (player.NextToFirePit) 
    { 
     foreach (Item tinder in player.PlayerInventory.Items.Where(i => i.ItemName == "tinder") 
     { 
      foreach (Item firepit in allItemsOnGround.Where(i => i.ItemName == "firepit")) 
      { 
       if (firepit.ItemRectangle.Contains(MouseWorldPosition) && 
       currentMouseState.LeftButton == ButtonState.Pressed && 
       oldMouseState.LeftButton == ButtonState.Released) 
       { 
        tinder.ItemName = "empty"; 
        firepit.ItemName = "firepitwithtinder"; 
        firepit.Texture = Content.Load<Texture2D>("firepitwithtinder"); 
       } 
      } 
     } 
     oldMouseState = currentMouseState; 
    } 
} 

Быстрый нюанс; этот код удалит все ружья с первый tinder, оставив другие tinders невредимым. Я мог бы разгадать петли, чтобы удалить все, но эта функция соответствует предоставленной; и, кроме того, я предполагаю, что это не предполагаемое поведение.

Заметьте, что вам не нужно ToList, потому что вы не изменяете сбор во время перечисления. Вы всегда можете изменить пункты в коллекции, доказанное со следующим испытанием:

class IntWrapper 
{ 
    public int value; 

    public IntWrapper(int value) 
    { 
     this.value = value; 
    } 
} 

class Program 
{ 
    static void Main(string[] args) 
    { 
     List<IntWrapper> test = new List<IntWrapper>() { new IntWrapper(1), new IntWrapper(2), new IntWrapper(3), new IntWrapper(4), new IntWrapper(5) }; 

     foreach (IntWrapper i in test.Where(i => i.value == 1)) 
     { 
      i.value = 0; 
     } 

     foreach (IntWrapper i in test) 
     { 
      Console.WriteLine(i.value); 
     } 

     Console.ReadLine(); 
    } 
} 

Answer 2

я бы, вероятно, использовать LINQ больше.

Обратите внимание, что это написано в «Блокноте», а не в визуальной студии, поэтому это скорее псевдо-код.

private void SetTinderInPit() 
{ 
    var currentMouseState = Mouse.GetState(); 
    if (!player.NextToFirePit) return; 

    player.PlayerInventory.Items.Where(item => item.ItemName == "tinder").ToList().ForEach(item => 
    { 
     allItemsOnGround.Where(x => x.ItemName == "firepit" && 
      x.ItemRectangle.Contains(MouseWorldPosition) && 
      currentMouseState.LeftButton == ButtonState.Pressed && 
      oldMouseState.LeftButton == ButtonState.Released) 
       .ToList().ForEach(pit => 
       { 
        item.ItemName = "empty"; 
        pit.ItemName = "firepitwithtinder"; 
        pit.Texture = Content.Load<Texture2D>("firepitwithtinder"); 
       }); 
    }); 
    oldMouseState = currentMouseState; 
} 

Answer 3

Единственное реальное изменение, которое я сделал бы, чтобы существующий код будет перенести проверку состояния мыши на ранней стадии, чтобы избежать проверок это несколько раз в ваших петлях. Кроме того, я хотел бы использовать Linq укоротить условия (путем удаления «если» заявление):

MouseState currentMouseState = Mouse.GetState(); 

// I would get all the conditional checks out of the way up front first 
if (player.NextToFirePit && 
    currentMouseState.LeftButton == ButtonState.Pressed && 
    oldMouseState.LeftButton == ButtonState.Released) 
{ 
    foreach (var tinderItem in player.PlayerInventory.Items 
     .Where(item => item.ItemName == "tinder")) 
    { 
     foreach (var firePit in allItemsOnGround 
      .Where(item => item.ItemName == "firepit" && 
          item.ItemRectangle.Contains(MouseWorldPosition))) 
     { 
      tinderItem.ItemName = "empty"; 
      firePit.ItemName = "firepitwithtinder"; 
      firePit.Texture = Content.Load<Texture2D>("firepitwithtinder"); 
     } 

    } 
} 

oldMouseState = currentMouseState; 

Альтернативной идея, так как вы искали способ избавиться от «уродливого» кода, будет для перемещения некоторых из этих функций в объект игрока.