Why aren't you using the lists we already created

So earlier in the lecture, we created 3 lists:

  • allbattlers
  • enemyBattlers
  • playerBattlers
  • So when we create the 2 new functions to get random members, why don’t you just use the lists we have already created.

    We already created an enemy list, so instead of walking through allBattlers, just add each item from the enemyBattlers list - since we already did the checking.

    Same goes with the party member list. We already have a list for that, so just add them to the temp list since we already added them.

    for (int i = 0; i < enemyBattlers.Count; i++)
    {
        enemyMembers.Add(i);
    }
    for (int i = 0; i < playerBattlers.Count; i++)
    {
        partymembers.Add(i);
    }

    The index in playerBattlers and enemyBattlers are not necessarily the same in allBattlers. allBattlers could have (Enemy, Enemy, Player, Enemy, Player). If you pick indices from enemyBattlers, your list would be (0, 1, 2) instead of (0, 1, 3), and picking indices from playerBattlers would be (0, 1) instead of (2, 4). The random selection will return an index from either enemyBattlers or playerBattlers for use in allBattlers which will be the wrong index.

    I have not been following the course yet - just looked at this lecture quickly - so I’m not sure why the two separate lists were made. Also not sure if they’re being kept up to date. If an enemy dies, does the enemyBattlers get updated? Same for playerBattlers. If so, then I see no reason why you can’t do what you did and then use the index to pick from the correct list, i.e. if you picked a random enemy from the (0, 1, 2) index list, use the index on enemyBattlers instead of allBattlers. Again, same for playerBattlers; if you picked an index from the (0, 1) list, use that index on the playerBattlers list instead of the allBattlers one.

    If that is indeed the case, there’s also no need to do a for loop. You can just pick a random number from 0 to count

    private int PickRandomEnemy()
    {
        return Random.Range(0, enemyBattlers.Count);
    }
    private int PickRandomPlayer()
    {
        return Random.Range(0, playerBattlers.Count);
    }
    

    I would personally also name the functions PickRandomEnemyIndex() and PickRandomPlayerIndex() because we’re not picking enemies or players, we’re picking their indices. But that’s just me

    You are correct that each list does contain different entities. The lists are being kept up to date. They were created so that we would have a separate list of all the enemies and all the party members and then a list with all of them.

    It appears though as he has migrated to just using only the AllBattlers list. I see why he used the allbattlers, because he is using the all battlers in the Attack routine - so there would be a mismatch in the indices. But a simple code change to use the enemy / player battlers in the attack routine, since we are updating the lists just seems like a better option

    It seemed odd to create more lists. Not tested it yet, but I went with a simple:

    return _allBattlers.IndexOf(_partyBattlers[Random.Range(0, _partyBattlers.Count)]);
    

    (I actually split this up over a few lines for readability, but this is what it boils down to)

    Privacy & Terms