Array returns undefined when removing elements

Question

Out of boredom I've decided to make a Russian Roulette game out of Javascript. I've managed to do the bullet part which is a random number between 1 and 6 (16,66%). Everytime the random number hits '1', the player will be dead and will be removed from the player list. The players are stored in an array.

Once the player is dead and removed from array, it gives an undefined as result which is strange since my array selector is working fine. How does the code give an undefined as result when a name gets removed from the array?

var players = ["Dmitry", "Sergey", "Nikolai", "Vladimir"];
var totalPlayers = players.length;
var playerSelector = 0;

function showPlayers(){
  alert(players);
  
}

function startGame(){
  var randomNumber = Math.floor(Math.random() * 6) + 1;
  
  if (playerSelector == totalPlayers){
    playerSelector = 0;
  }
  
  if (randomNumber == 1) {
    alert("Player: " + players[playerSelector] + "\nAmmo Slot " + randomNumber + "\nYou are dead..");
    players.splice(playerSelector, 1);
  }
  else {
    alert("Player: " + players[playerSelector] + "\nAmmo Slot " + randomNumber  + "\nYou survived");
  }
  playerSelector++;
}

<button onclick="startGame()">Start Roulette</button>
<button onclick="showPlayers()">Show Players</button>

Answer 1

You probably want to also decrement totalPlayers when a player is killed.

var players = ["Dmitry", "Sergey", "Nikolai", "Vladimir"];
var totalPlayers = players.length;
var playerSelector = 0;

function showPlayers(){
  alert(players);
  
}

function startGame(){
  var randomNumber = Math.floor(Math.random() * 6) + 1;
  
  if (playerSelector == totalPlayers){
    playerSelector = 0;
  }
  
  if (randomNumber == 1) {
    alert("Player: " + players[playerSelector] + "\nAmmo Slot " + randomNumber + "\nYou are dead..");
    players.splice(playerSelector, 1);
    totalPlayers--; //Here
  }
  else {
    alert("Player: " + players[playerSelector] + "\nAmmo Slot " + randomNumber  + "\nYou survived");
  }
  playerSelector++;
}

<button onclick="startGame()">Start Roulette</button>
<button onclick="showPlayers()">Show Players</button>

Answer 2

I would change this If:

if (playerSelector == totalPlayers){
playerSelector = 0;

}

To:

if (playerSelector >= players.length){
playerSelector = 0;

}

Because totalPlayers is Not changing.

Maybe you should also Check, If there are Any players left.

Answer 3

As others have mentioned, totalPlayers never updates, soo you end up referring to indices that are too large for the size of the array, which makes sense. If your array has 3 items, the second index is the last item. But if you remove an item from the array, its length is now only 2, so the first index will now be that last item in the array, and the second index will not exist anymore. I've adjusted your code to account for that, any I've made some other optimizations as well:

var playerSelector = 0, players = ["Dmitry", "Sergey", "Nikolai", "Vladimir"];

function startGame() {
  var randomNumber = rando(1, 6);
  playerSelector = ++playerSelector % players.length;

  if (randomNumber === 1) alert("Player: " + players.splice(playerSelector, 1)[0] + "\nAmmo Slot " + randomNumber + "\nYou are dead..");
  else if (players.length > 0) alert("Player: " + players[playerSelector] + "\nAmmo Slot " + randomNumber + "\nYou survived");
}

<script src="https://randojs.com/1.0.0.js"></script>
<button onclick="startGame()">Start Roulette</button>
<button onclick="alert(players);">Show Players</button>

Explanation:

Note that having ++ before a variable increments its value before anything else in the order of operations. % (modulus) is an operator that gives the remainder after division. So playerSelector = ++playerSelector % players.length; increments playerSelector and knocks it back to zero instead of letting it reach players.length.

splice returns an array of the items it removed, so you can stick that inline in the alert if you want.. I did. If if statements are a single line, you don't need the {}, but again, that's just preference. In the else if statement, I checked to make sure a player still existed before alerting anything to avoid accessing an index of an empty array.

I also used === instead of == because using == with 0's and 1's can be weird, so it's just good practice. If you don't believe me, check this out:

console.log(true == 1, "" == 0);

Finally, I used randojs.com to simplify the randomness and make it more readable. Just make sure to stick this in the head tag of your html document if you want to use randojs:

<script src="https://randojs.com/1.0.0.js"></script>