Sign in to follow this  
ccuccherini

Win loop errors

Recommended Posts

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;
}
Edited by ccuccherini

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites

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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites

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.

Share this post


Link to post
Share on other sites
  • 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?

Share this post


Link to post
Share on other sites

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!

Share this post


Link to post
Share on other sites

 

  • 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!

Share this post


Link to post
Share on other sites

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.

Share this post


Link to post
Share on other sites

 

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.

Edited by ccuccherini

Share this post


Link to post
Share on other sites

 

enum class Winner {Nobody, PlayerOne, PlayerTwo};
Winner winner = Winner::Nobody;
As soon as I get a good understanding of classes I will be switching over to what you suggest.

 


Heheh, sorry for the confusion. This is an enum, not a class.  :) 

The C++ design committee have an annoying tendency to reuse the same keywords ('class', 'static', 'struct', 'auto', etc...) for multiple different unrelated features.  :rolleyes:

In this case, saying 'enum class MyEnum { };' has absolutely nothing to do with regular "classes" like:

class MyClass //Completely unrelated
{
    public:
    void DoSomething();
};

 
Basically, enums work like this:

enum ENUM_NAME { ONE, OR, MORE, CONSTANTS  };

 
For example:

//Each name within the brackets basically becomes a constant variable (there's a few differences though).
enum VehicleType { Horse, Car, Truck, Train, GiantEagle, Airplane};
 
VehicleType myType = Truck;
int myTypeAsInt = Car;

 
The problem with that 'regular' kind of enum is that (A) the names of the enum constants can accidentally conflict with other variable types (unless you protect them in a namespace or within a class or function's scope), and also (B) the enums automatically convert into integers if assigned to an integer, which is good in some cases (when used as what's called "bitwise flags" - you'll learn about those later), but can occasionally be a source of bugs.
 
By saying 'enum class' instead of just 'enum', you create what's called "strongly-typed" enums - enums that don't automatically convert to ints. As an added bonus, they are automatically in their own namespace.

enum class VehicleType { Horse, Car, Truck, Train, GiantEagle, Airplane};
 
VehicleType myType = VehicleType::Truck;
//int myTypeAsInt = VehicleType::Car; <--- Won't compile

Share this post


Link to post
Share on other sites
SotL - Thank you for clearing that up. That's almost as bad as the English language where you can say "boot" and have it mean several different things that have nothing to do with one another.

I had seen enum used before in a tutorial I had watched before and it wasn't really explained what it was used for past the fact that that particular program would be using it to store directions (up, down, right, left, etc) so your explication is very helpful and now it makes sense why it was used in that tutorial.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this