Shuffling an array and assigning it to different buttons in Unity-CodePudding

I have an array that I am shuffling and then assigning to buttons. Depending on the item in the array, the button will perform a different function. This is currently my code, and it works, but is extremely inelegant.

I am currently shuffling the array first, then assigning the first 2 items of the shuffled array to the button text, and then using if statements to check whether or not the string matches so that it will execute a specific code for that string.

There is definitely a better way to do this, but I can't seem to figure it out.

public TextMeshProUGUI firstChoiceText;
public TextMeshProUGUI secondChoiceText;
public GameObject player;

public string[] upgrades =
{
    "Speed Up",
    "Fire Rate Up",
    "Damage Up",
    "Max Health Up"
};

public void Shuffle(string[] array)
{
    for (int i = 0; i < array.Length; i  )
    {
        string tmp = array[i];
        int rand = Random.Range(0, array.Length);
        array[i] = array[rand];
        array[rand] = tmp;
    }
    firstChoiceText.text = upgrades[0];
    secondChoiceText.text = upgrades[1];
}

// Start is called before the first frame update
void Start()
{
    Shuffle(upgrades);
}

public void FirstChoice()
{
    Debug.Log("first choice clicked");

    if (firstChoiceText.text == "Speed Up")
    {
        player.GetComponent<PlayerController>().playerSpeed  = 1;
    }

    else if (firstChoiceText.text == "Fire Rate Up")
    {
        player.GetComponent<PlayerController>().fireRate -= 0.05f;
    }

    else if (firstChoiceText.text == "Damage Up")
    {
        player.GetComponent<PlayerController>().playerDamage *= 1.1f;
    }

    else if (firstChoiceText.text == "Max Health Up")
    {
        GameManager.maxHealth  = 5;
        player.GetComponent<PlayerController>().Heal(5);
    }

    Time.timeScale = 1;
    gameObject.SetActive(false);
    Shuffle(upgrades);

}
public void SecondChoice()
{
    Debug.Log("second choice clicked");

    if (secondChoiceText.text == "Speed Up")
    {
        player.GetComponent<PlayerController>().playerSpeed  = 1;
    }

    else if (secondChoiceText.text == "Fire Rate Up")
    {
        player.GetComponent<PlayerController>().fireRate -= 0.05f;
    }

    else if (secondChoiceText.text == "Damage Up")
    {
        player.GetComponent<PlayerController>().playerDamage *= 1.1f;
    }

    else if (secondChoiceText.text == "Max Health Up")
    {
        GameManager.maxHealth  = 5;
        player.GetComponent<PlayerController>().Heal(5);
    }

    Time.timeScale = 1;
    gameObject.SetActive(false);
    Shuffle(upgrades);

}

CodePudding user response：

One solution would be to create a Dictionary<string, Action<Player>> where your Action delegate corresponds to your string key and call the method via Action delegate based on the string key of the Dictionary, also moved your shuffle logic into it's own private method to reduce code duplication:

//Dictionary to hold string key/Action delegate pairs
private Dictionary<string, Action<Player>> _actions = new Dictionary<string, Action<string>>
{
    {"Speed Up", (player) => player.GetComponent<PlayerController>().playerSpeed  = 1;},
    {"Fire Rate Up", (player) => player.GetComponent<PlayerController>().fireRate -= 0.05f;}
    {"Damage Up", (player) => player.GetComponent<PlayerController>().playerDamage *= 1.1f;},
    {"Max Health Up", (player) => { GameManager.maxHealth  = 5;
        player.GetComponent<PlayerController>().Heal(5); } }        
};

//You could reduce your First And Second Choice down to using the
//dictionary to call the cooresponding Action delegate:

public void FirstChoice()
{
    Debug.Log("first choice clicked");
    _actions[firstChoiceText.text](player);   
    DoShuffle();   
}

public void SecondChoice()
{
    Debug.Log("second choice clicked");
    _actions[secondChoiceText.text](player); 
    DoShuffle();    
}

//Moved this into a method to reduce repetitive code
private void DoShuffle()
{
    Time.timeScale = 1;
    gameObject.SetActive(false);
    Shuffle(upgrades);
}

CodePudding user response：

You could create a base class for upgrades, and then use inheritance to split up all the logic that is now in the if statements to separate upgrade classes deriving from this base class. In the Unity world you might want to use ScriptableObjects for this.

public abstract class Upgrade : ScriptableOject
{
    public abstract void Apply(PlayerController player);
}

[CreateAssetMenu(menuName = "Upgrades/Speed Up", fileName = "Speed Up")]
public sealed class SpeedUp : Upgrade
{
    public override void Apply(PlayerController player)
    {
        player.playerSpeed  = 1;
    }
}

[CreateAssetMenu(menuName = "Upgrades/Fire Rate Up", fileName = "Fire Rate Up")]
public sealed class FireRateUp : Upgrade
{
    public override void Apply(PlayerController player)
    {
        player.fireRate -= 0.05f;
    }
}

Then you could create one scriptable object asset for each upgrade, and then assign all of them to your script into an Upgrade[] field.

[SerializeField] private TextMeshProUGUI firstChoiceText;
[SerializeField] private TextMeshProUGUI secondChoiceText;
[SerializeField] private PlayerController player;
[SerializeField] private Upgrade[] upgrades;

private Upgrade firstUpgrade;
private Upgrade secondUpgrade;

public void ApplyFirstUpgrade()
{
    Debug.Log("first choice clicked");
    ApplyUpgrade(firstUpgdade);
}    

public void ApplySecondUpgrade()
{
    Debug.Log("second choice clicked");
    ApplyUpgrade(firstUpgdade);
}

private void Awake() => RandomizeUpgrades();

private void RandomizeUpgrades()
{
    firstUpgrade = GetRandomUpgrade();
    secondUpgdade = GetRandomUpgrade(firstChoice);
}

private Upgrade GetRandomUpgrade() => upgrades[Random.Range(0, upgrades.Length)];

private Upgrade GetRandomUpgrade(Upgrade ignore)
{
    if(upgrades.Length < 2)
    {
        Debug.LogError("At least 2 upgrades need to be assigned before calling GetRandomUpgrade.", this);
        return;
    }

    Upgrade resultCandiate = GetRandomUpgrade();
    if(resultCandiate != ignore)
    {
        return result;
    }

    return GetRandomUpgdate(ignore);
}

private void ApplyUpgrade(Upgrade upgrade)
{
    upgrade.Apply();
    Time.timeScale = 1;
    gameObject.SetActive(false);
    RandomizeUpgrades();
}

The benefit with this sort of approach is that you can add more abilities easily without having to make any modifications to existing code, and without ending up with one giant class with hundreds of lines of code.