using "while" in the setup

Hi !

Please forgive my noob skills... I'm starting a project (game) and the first important thing to do is to tell the arduino board how many players will be involved in the game.
I set the inclosed jpg file up and loaded the enclosed sketch.
It's supposed to wait during setup until someone press button 1, 2, 3 or 4, then set the variable "playersNumber" with the corresponding value and then loop and lit LED 1, 2, 3 or 4 regarding the number that was chosen during setup.
It works well... 1 time on 3, sometimes 1 time on 5. Other times I reset the board and click on a button, nothing happens and the program stays stuck in setup.
I clearly do something wrong here... After hours on the web looking for informations about how to use "while", I give up.

If someone has a minute to look at it and lead me to a solution, I'll more than thankful !

Thanks in advance

int playersNumber;

void setup() {
  Serial.begin(9600);
  pinMode(2, INPUT_PULLUP);
  pinMode(3, INPUT_PULLUP);
  pinMode(4, INPUT_PULLUP);
  pinMode(5, INPUT_PULLUP);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(10, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(13, OUTPUT);
  playersNumber = 0;

  while (digitalRead(2) ==HIGH && digitalRead(3) ==HIGH && digitalRead(4) ==HIGH && digitalRead(5) ==HIGH) {
    
    if (digitalRead(2) ==LOW && digitalRead(3) ==HIGH && digitalRead(4) ==HIGH && digitalRead(5) ==HIGH) {
      playersNumber = 1;
      digitalWrite(8, HIGH);
    }
    if (digitalRead(2) ==HIGH && digitalRead(3) ==LOW && digitalRead(4) ==HIGH && digitalRead(5) ==HIGH) {
      playersNumber = 2;
      digitalWrite(9, HIGH);
    }
    if (digitalRead(2) ==HIGH && digitalRead(3) ==HIGH && digitalRead(4) ==LOW && digitalRead(5) ==HIGH) {
      playersNumber = 3;
      digitalWrite(10, HIGH);
    }
    if (digitalRead(2) ==HIGH && digitalRead(3) ==HIGH && digitalRead(4) ==HIGH && digitalRead(5) ==LOW) {
      playersNumber = 4;
      digitalWrite(11, HIGH);
    }
   
  }
 
}

void loop() {
 
  if (playersNumber ==1) {
    digitalWrite(8, HIGH);
    Serial.print(playersNumber);
  }

   if (playersNumber ==2) {
    digitalWrite(9, HIGH);
    Serial.print(playersNumber);
  }
   if (playersNumber ==3) {
    digitalWrite(10, HIGH);
    Serial.print(playersNumber);
  }
   if (playersNumber ==4) {
    digitalWrite(11, HIGH);
    Serial.print(playersNumber);
  }
  else {
    digitalWrite(8, LOW);
    digitalWrite(9, LOW);
    digitalWrite(10, LOW);
    digitalWrite(11, LOW);
  }
  
}

Please attach your code and images properly.

Always show us your ‘current’ complete sketch.
Use CTRL T to format the sketch.
Please use code tags.
Use the </> icon in the posting menu.

[code] Paste sketch here. [/code]

Show us a good schematic of your circuit.
Show us a good image of your wiring.
Give links to components.
Posting images:
https://forum.arduino.cc/index.php?topic=519037.0

Wouldn't it be great if all those pins had nice descriptive names?

@Loyc, first code - excused.
Please avoid Fritzy drawings - they're half a step away from useless when sharing or debugging a project.

Size reduced for faster loading

Your while loop seems to say "while none of the buttons is pressed, if one of them is pressed then do something"

If I am right then does that make sense ?

Also look into pin[] arrays and switch( case ) statements.
Your code will be a LOT shorter - especially as you add more players.

It may seem to make sense to use setup() for this kind of thing but that is incorrect. setup() should be used to set up the environment, connect to the displays, show a "hello" message and that kind of thing.

The main loop() should be used to ask the user a question, start and stop a game. What if you want a new game with a different number of players? It's difficult to go back to setup() at that point.

It also spreads your user-interface button code around the place. If you are forced to change one button to a different pin or some other trivial change, suddenly it's more difficult to find, change and test all of those different parts of the code.

Your while loop seems to say "while none of the buttons is pressed, if one of them is pressed then do something"
If I am right then does that make sense ?

The while() loop is very fast, and to be inside the loop no buttons can be pressed. That leaves a very small window of time to catch a button press. You do not test for multiple buttons pressed, so there is no need to have all the && conditions in your tests.

I would use a global boolean variable numberSelected, initialized as false, and let it control the while loop. As MorganS recommends, this can be placed in loop() instead of setup(), and anytime you want to re-enter the player selection set numberSelected back to false.

while (numberSelected == false) 
  {
    
    if (digitalRead(2) ==LOW)
    {
      numberSelected = true;
      playersNumber = 1;
      digitalWrite(8, HIGH);
    }
    if (digitalRead(3) ==LOW ) 
    {   
      numberSelected = true;
      playersNumber = 2;
      digitalWrite(9, HIGH);
    }
    if (digitalRead(4) ==LOW) 
    {
      numberSelected = true;
      playersNumber = 3;
      digitalWrite(10, HIGH);
    }
    if (digitalRead(5) ==LOW) 
    {
      numberSelected = true;
      playersNumber = 4;
      digitalWrite(11, HIGH);
    }
   
  }

Guys, Thanks a lot for your help. There is a lot of good advices here that I will follow to find the solution.
I'll dig into it as soon as I can and come back here for an update.
Thank you all again !

As suggested by Cattledog, using a global boolean variable with the while question in the loop instead of setup seems to be a perfect solution to my problem. It works perfectly.
The end of the code is a bit different to allow fourth button switching off all LED and tell loop to ask for player Numbers again (it's just for the test code).

Now can go on with my project thanks to Cattledog and to all of you're suggestions.
Thank you all for you help solving my problem.

int playersNumber;
int askForPlayers;

void setup() {
  Serial.begin(9600);
  pinMode(2, INPUT_PULLUP);
  pinMode(3, INPUT_PULLUP);
  pinMode(4, INPUT_PULLUP);
  pinMode(5, INPUT_PULLUP);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(10, OUTPUT);
  pinMode(11, OUTPUT);
  playersNumber = 0;
  askForPlayers == false;

}

void loop() {
  
  while (askForPlayers == false) 
  {
    if (digitalRead(2) ==LOW)
    {
      askForPlayers = true;
      playersNumber = 1;
      digitalWrite(8, HIGH);
    }
    if (digitalRead(3) ==LOW ) 
    {   
      askForPlayers = true;
      playersNumber = 2;
      digitalWrite(9, HIGH);
    }
    if (digitalRead(4) ==LOW) 
    {
      askForPlayers = true;
      playersNumber = 3;
      digitalWrite(10, HIGH);
    }
  }

  if (digitalRead(5) ==LOW) 
    {
      askForPlayers = false;
      digitalWrite(11, HIGH);
      delay (1000);
      digitalWrite(8, LOW);
      digitalWrite(9, LOW);
      digitalWrite(10, LOW);
      digitalWrite(11, LOW);
     
    }
  
}

There is NOTHING inherently wrong with your approach to well defined task. (single button push ...)
That is one of the advantages of software - there are many ways to accomplish a task.
Saying that you should not do that is setup is defeating the flexibility of software solutions.

Checking buttons in Setup and then passing the results to loop is actually much better - you control the "wait" and NOT the "loop". But it is indeed a single shot at buttons.

There is only one thing you could change , but it is superficial and not really necessary.

The "while" monitors all of the buttons and when ANY of them is active the if's no longer need to read all of therm again.

Just checking for if(!buttonx) will suffice.

I cannot prove it, but the compiler probably "optimize" the if's and no longer checks all inactive buttons anyway.

Your problem is somewhere else and adding global variables or switch IS NOT a solution - just another way to skin a cat.

Good luck - I would look for loose connections, but tha's OF me.