What is wrong with this code?

Started by
13 comments, last by metorsummoner7 12 years, 10 months ago
My first suggestion is to not use Dev-C++, please read this article for why http://www.jasonbadams.net/20081218/why-you-shouldnt-use-dev-c/

It would also be helpful to post the error/warning logs so we can see what errors you are getting
Advertisement
I added you (King) on MSN, fire it up if you want help with anything.

By the way, I did the same if 1 then ... if 2 then ... etc. thing once when I was new and using Visual Basic. I didn't realize you could reuse file handles >_<
Some things to consider:

  • There are lots of problems with the current code. This is because you've done too much without getting stuff working. I recommend you start smaller and build the game up in pieces. Comment out most of the code, and get a basic game loop running. Then get one paddle working, then both. Put the ball in. Make the ball react to the paddles. Add the scoring system.

  • Things like images are generally loaded at startup and freed at shutdown. Freeing them throughout the game is probably a bad idea, it might lead to crashes if it turns out you do need those images later on. At the very least set the pointers to NULL so you can detect that they are unloaded.

  • Don't use SDL_Delay() for long periods of time. Instead use a timer value such as SDL_GetTicks() and transition when the ticks have passed the threshold. This is important because your application should consume events otherwise the OS will think it is hung, plus if the user tries to exit the program while you are Delaying it will not respond.

  • Use named constants or enumerations instead of magic numbers. For example: gameState == SINGLE_PLAYER is much more meaningful than gameState == 1.

  • The two Button classes can actually be merged. They are almost identical, extract the differences into member data, set via the constructor. The two Paddle classes can also be merged, but this is a slightly more complex task because you'll need to separate the "controller" (e.g. the player or the AI) from the Paddle logic. It is a good thing to separate them though.

  • Don't return "false" or "true" inside main(). Remember that this is allowed because bool is technically an integral type. The convention for main() is to return 0 for "success" and a non-zero value for "failure". Also consider adding some form of logging so that if the game fails to start due to missing resources you can see this quickly from the log rather than having to fire up the debugger.

  • With a little bit of effort, you can make all your global variables into locals. Using globals like that is just about manageable in small games, but it simply doesn't scale. Some tricks you can do to help this is to invert the logic. For example, instead of updating the score in the middle of the Ball collision code, have the Ball return a value (e.g. enum CollisionResult { NO_COLLISION, LEFT_WALL, RIGHT_WALL }) and let main() update the variable.
Pfff there are a lot of things wrong or bad in your code. I don't have time to look through all the lines, but except for the 'o' and the uninitialized variables I also noted your are using rand() without setting a seed first. rand() generates a random number based on a 'seed' using some kind of algorithm. The seed should be set before using rand() and usually something like the time in milliseconds since the start of the program is used. If you don't set a seed you are getting the same number over and over again.

This has nothing to do with not compiling, but is just something I noticed reading your code.


[size="1"]Next time it might be a good idea to compile your game a little more often. That way you can spot errors more easily and it is easier to debug.
Right now I would suggest commenting away lots of code, make whole functions blank by putting /* and */ in front and at the end of the code.
Remove as much code until your code finaly compiles and than slowly start re-adding the code. (edit: seems someone already posted my great idea...frustrating :) )
If you don't set a seed you are getting the same number over and over again.


I take it you meant "numberS" over and over which isn't such a bad thing and can be vital for debugging. I'd even go as far as suggesting to always set a constant seed (or none at all, which according to the srand documention is the same as srand(1) ) for debugging builds.
f@dzhttp://festini.device-zero.de
You might want to consider breaking your code down into a few more cpp and header files. You might be better off using Visual Studio Express, I think that would be a better IDE for you. Organize your code and build your project often while you code. Work with one thing at a time and make sure it works and complies before moving on to the next. thing.

This topic is closed to new replies.

Advertisement