Win loop errors

Started by
10 comments, last by ccuccherini 7 years, 8 months ago

Hey guys! I've started working on some coding aspects of my Pong game and started off with setting up the game to determine when to end the game when only looking at the number of points each player has and then use that information to determine which player won.

Right now I have a while loop set up to check to see how many points each player has and if at the end of the loop one of the two players has 10 points (this is also the number of points required to win) the program exits the loop and tells you which player won. I have followed this up with an if/else check to determine the winner.

I've run the check a few different times with various conditions and when I have it set in a way that would make player 1 (player 1's score is stored as p1score) it works just as it should and displays "Player 1 wins!" at the end of the program. However, as soon as I change the conditions to make player 2 win (player 2's score is stored as p2score) it doesn't do what it needs to and still outputs "Player 1 wins!" even though it clearly shouldn't.

That being said, I've been able to narrow the bug down to the if/else statements but I can't for the life of me figure out what is wrong. The only way I have been able to get it to display "Player 2 wins!" when I have it set up to where they should is if I comment out everything from the first if to the else that is outside of the while loop. I'm almost certain whatever is wrong is something that is so plainly easy to spot it is painful but I just can't see it...

I've included the code with a set to where player 2 should win. I'm sure there is some cleaner way to write this but at the moment I am more focused on why my if/else statements aren't working they way they should.

Any help or insight would be greatly appreciated.


//Pong Win loop

#include <iostream>
using namespace std;

int main() {
	// your code goes here
	
	int p1score = 0;
	int p2score = 0;
	bool win = false;
	
	while (win = false){
	    
	    p1score++;
	    p2score = p2score + 2;
	    
	    if (p1score = 10){
	        win = true;
	    }
	    else if (p2score = 10){
	        win = true;
	    }
	    else{
	        win = false;
	    }
	}
	
	if(p1score = 10){
	    std::cout<<"Player 1 wins!"<<endl;
	}
	else if (p2score = 10){
	    std::cout<<"Player 2 wins!"<<endl;
	}
	
	return 0;
}
Advertisement

First thing to jump out at me: you probably want an == instead of an = in your while loop condition.

First thing to jump out at me: you probably want an == instead of an = in your while loop condition.


I had completely missed that I had not added the second '='. Thank you fit pointing that out. I'll be sure to fix that in the morning and I'll post how that affects the outcome.
The == also is required in the if conditions.

Otherwise the first if check always evaluates to true. Remember, in C/C++ any value other than zero is promoted to true if queried as boolean.

Fruny: Ftagn! Ia! Ia! std::time_put_byname! Mglui naflftagn std::codecvt eY'ha-nthlei!,char,mbstate_t>

The == also is required in the if conditions. Otherwise the first if check always evaluates to true. Remember, in C/C++ any value other than zero is promoted to true if queried as boolean.
QFE

To save yourself a lot of head ache, turn on C/C++ compiler warnings. Modern compilers tend to give useful warnings about doubtful constructs, and assigning a value to a variable in a condition is no doubt one such doubtful construct. Such a warning would have pointed out the problem for you at first compile.

  • Rather than determining the winner twice (once in the loop and once after), why not use an int instead of a bool? The int could be called "winnerID" and a value of zero would indicate that nobody has won yet.
  • p2score increments by 2 every round. You can express that as p2score += 2.
  • What would happen if the scores started at 1 instead of zero?
void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

SiCrane and Endurion - Thank you for pointing my missing '=' in the while loop and the if/else statements. That ended up being what the issue was and my initial problem was solved with those few corrections.

Alberth - I had no idea that we could set up our compliers to check for these sorts of issue. Thank you for bringing it up. I'll definitely look into the settings and get them set up to save myself time and energy when debugging. Do you know if this works for IDEs as well or is it something that ends up getting set up in them when we change the settings in the compiler?

Khatharr - I've started looking into your suggests and in fixing the '=' to '==' went ahead and changed the increments for p2score to p2score += 2. I also tested the program to see what would happen if both players started off with 1 point each and found where my current code would cause an issue with showing who actually won and have since updated it to determine who has the higher score before saying who won. As for your first point, I'll try that out too and see how that works for me. I think another possible solution would be to change the if/else statement in my while loop to one where it is worded if(p1score >= 10 || p2score >= 10) win = true. But that is also something I need to play around with to see which one works best and keeps the code as clean as possible while also keeping it easy to ready and understand.

I thank all of you wonderful people for your suggestions and insight in helping me solve my issue!

  • Rather than determining the winner twice (once in the loop and once after), why not use an int instead of a bool? The int could be called "winnerID" and a value of zero would indicate that nobody has won yet.
  • p2score increments by 2 every round. You can express that as p2score += 2.
  • What would happen if the scores started at 1 instead of zero?

Your idea of using an int instead of a bool worked wonderfully and I was able to cut out a several lines of code that ended up being unnecessary. Thank you so much for the suggestion!

Your idea of using an int instead of a bool worked wonderfully and I was able to cut out a several lines of code that ended up being unnecessary. Thank you so much for the suggestion!

It'd be better to use an enum for that:


enum class Winner {Nobody, PlayerOne, PlayerTwo};
Winner winner = Winner::Nobody;

The reason is because you are assigning hidden meaning to each value, and it'd be good to be explicit about what those meanings are, especially because you use those same values (with the same meaning) in more than one location in your code.

Your idea of using an int instead of a bool worked wonderfully and I was able to cut out a several lines of code that ended up being unnecessary. Thank you so much for the suggestion!

It'd be better to use an enum for that:


enum class Winner {Nobody, PlayerOne, PlayerTwo};
Winner winner = Winner::Nobody;

The reason is because you are assigning hidden meaning to each value, and it'd be good to be explicit about what those meanings are, especially because you use those same values (with the same meaning) in more than one location in your code.

That makes a lot of sense. That should also make it easier to reference those values later on correct?

As soon as I get a good understanding of classes I will be switching over to what you suggest.

This topic is closed to new replies.

Advertisement