6
\$\begingroup\$

I have here a snippet of code from a simple video game AI enemy. The basic idea behind the enemy is that he can be in one of a few states. By default he is in a patrolling state, where he moves around at a slow speed to random points within a circle. While in this state, he is rolling every X seconds to switch to one of his more interesting states. Currently, he may either transition into a state where he spins rapidly while patrolling as stated above, but at a much faster move speed. Or he will can instead go into a stationary defensive spin. Here is the current method which handles this state change:

IEnumerator RollForStateChange()
{
    while (true)
    {
        yield return new WaitForSeconds(RollTickTime); 

        if (CanRollForStateChange)
        {
            float spinAttackRoll = Random.Range(0, 100);

            if (spinAttackRoll <= SpinAttackChancePerTick)
            {
                StartCoroutine(SpinAttackTimer());
                continue;
            }

            float spinDefenseRoll = Random.Range(0, 100);

            if(spinDefenseRoll <= SpinDefenceChancePerTick)
            {
                StartCoroutine(DefenseSpinTimer());
                continue;
            }
        }
    }
}

While this sort of dice rolling method works just fine for now, I can't help but feel that it is more likely to choose the spin attack, since that roll goes first. I can also see this becoming more inefficient if I wanted to add more states for the enemy to transition into. However, I am struggling to come up with a more elegant way to decide which state the enemy should go into.

\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! I hope you get some good answers! \$\endgroup\$ – Phrancis Jul 8 '15 at 0:33
5
\$\begingroup\$

You are right that this roll is biased. Instead of using a fixed range and less-than for your rolls, you should just roll once and use specific ranges to choose which action is taken. Something like:

int spinAttackChancePerTickMin = 0;
int spinAttackChancePerTickMax = 50;

int spinDefenceChancePerTickMin = 50;
int spinDefenceChancePerTickMax = 100;

int roll = Random.Range(0, 100);

if (roll >= spinAttackChancePerTickMin && 
    roll < spinAttackChancePerTickMax)
{
    StartCoroutine(SpinAttackTimer());
    continue;
}

StartCoroutine(DefenseSpinTimer());
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Please do not use else and else if any time you interrupt flow control using break, continue, return. By doing so, we can reduce the amount of code that needs to be tracked in the flow path and potentially reduce the amount indentations. \$\endgroup\$ – Snowhawk Jul 8 '15 at 1:42
  • \$\begingroup\$ @Snowhawk04 So just make that an if? \$\endgroup\$ – user34073 Jul 8 '15 at 2:23
  • 1
    \$\begingroup\$ In the situation where you are breaking the flow, just make them ifs. In this situation, since we would only be transitioning into 2 possible states, you could remove the 2nd conditional and just run the defense coroutine (drop the final continue as well since the loop would be finishing anyway). \$\endgroup\$ – Snowhawk Jul 8 '15 at 2:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.