Affecting which ability enemy chooses - help with code refinement

So this code I just wrote works, but it feels like it could be more efficient.

Basically, the game calls the ChooseEnemyAbility method. It sets chosenAbility to null, which in my game results in a standard non-ability attack. And then it does a random roll to see if it will instead use a special ability. If it rolls the right number, it sets the chosenAbility to the new ability, but checks to see if the enemy’s remaining energy isn’t less than the energy cost of the ability. If the enemy’s remaining energy is less than the energy cost of the ability, it just calls the function again, which at the beginning resets the value of chosenAbility.

What this results in, as I’ve watched the Debug.Log is that if the enemy gets really low on energy, it will multiple times choose an ability it cannot afford to use and the log will show like 5-6 error messages “ABILITY COST GREATER THAN ENEMY REMAINING ENERGY”.

I wonder if there is a better way to script this. Like a nested thing or something else. I haven’t been using C# long, or coding long, so I have no clue. But this looks like of messy to me.

Any help would be very appreciated.

public BaseAbility ChooseEnemyAbility(){
chosenAbility = null;
int randomTemp = Random.Range (1,101); //random roll from 1-100
if(randomTemp < 33){ //enemy chooses attack at random
chosenAbility = new AttackAbility(); //enemy uses Attack ability on player
CheckForAbilityCost(chosenAbility);
} else if (randomTemp < 66){
chosenAbility = new SwordSlash(); //enemy uses Sword Slash ability on player
CheckForAbilityCost(chosenAbility);
}
return chosenAbility;
}

public void CheckForAbilityCost(BaseAbility ability){
if(StatCalculations.newEnemy.Energy < ability.AbilityCost){
Debug.Log (“ABILITY COST GREATER THAN ENEMY REMAINING ENERGY”);
ChooseEnemyAbility();
}
}

Without more code or more explanation, it’s difficult to provide many specifics, but here a few points to consider:

  • Rather than choose from all abilities and then determine after-the-fact whether the enemy can afford the chosen ability, you’d probably be better off structuring your abilities and their selection so you’re only selecting from a set of abilities that are currently “affordable”. To do that, your ChooseEnemyAbility() method would need to access your Enemy.Energy property near the top and use that value to select from a set of affordable abilities.
  • If you keep your current structure and methods, I’d at least suggest that you rework the CheckForAbilityCost() method. It should not call ChooseEnemyAbility(). Instead, it should probably just return a bool indicating whether the selected ability can be purchased. Then, in the caller (ChooseEnemyAbility), the return value should be evaluated and if FALSE (ability can’t be afforded), a new ability should be chosen and rechecked.

Jeff

2 Likes

That would something I would be very interested in doing, but I’m not sure of how to write it.

I would want it to (at the moment, for ease) randomly choose between null / SwordSlash / AttackAbility, and then call CheckForAbilityCost and return whether the enemy can legally use the ability (null having no cost), and if it returns false, pick again.

Would there be a way to tell the code to go back up a few lines? Or go back up to a certain point and start from there?

Maybe a while statement?

While the enemy cannot afford the ability, it choose another, and then when it can afford it, just passes the ability on?

That may be the ticket!

I appreciate the help Jeff! I understand with just me posting two methods that it’s very hard to offer help, but you definitely helped!

This ought to be close, though it’s completely untested…

public BaseAbility ChooseEnemyAbility()
{
    bool canAfford = false;
    while (!canAfford)
    {
        int randomTemp = Random.Range (1,101);
        if (randomTemp < 33)
        {
            chosenAbility = new AttackAbility();
        }
        else if (randomTemp < 66)
        {
            chosenAbility = new SwordSlash();       
        }
        else
        {
            chosenAbility = null;
        }
       
        canAfford = CheckForAbilityCost(chosenAbility);
    }
    return chosenAbility;
}

public bool CheckForAbilityCost(BaseAbility ability)
{
    if (ability == null) { return true; } // can always afford base ability
    return StatCalculations.newEnemy.Energy >= ability.AbilityCost;
}
1 Like

This is working!

do{
int randomTemp = Random.Range (1,101); //random roll from 1-100
if(randomTemp < 33){ //enemy chooses attack at random
chosenAbility = new AttackAbility(); //enemy uses Attack ability on player
} else if (randomTemp < 66){
chosenAbility = new SwordSlash(); //enemy uses Sword Slash ability on player
} else {
chosenAbility = null;
}
} while (chosenAbility != null && StatCalculations.newEnemy.Energy < chosenAbility.AbilityCost);

edit: Totally didn’t see your reply Jeff. I’ll read over it.

This is similar to what I came up with and really nicely written. My only concern I mentioned above in caps. If the randomTemp choose null, can null be sent into the CheckForAbilityCost method if the method is expecting a BaseAbility?

You catch it right away inside the method, I’m just wondering if it’d accept it.

Thanks again for helping!

Yeah, that should be fine. Though, as you said, it’s really quite similar your new code…

Jeff

1 Like

Yours is a lot prettier, though :stuck_out_tongue:

happy new years!

Yeah, code tags tend to do that - you should use them… :wink:

Happy New Year to you also!

1 Like