Here are some peaces of advice.
Use references instead of pointers when passing objects to functions.
Use const references when possible.
Use the STL containers like std::array or std::vector instead of C-Style arrays.
Use const on variables when possible.
Organize your code so that Update code and Drawing code are separate.
Use const on functions that aren't changing the state of the object the function belongs to ( Draw functions and any Get function are a prime example )
Use enums instead of bools to represent different states like what state the game is in:
Code: Select all
enum class GameState{ Menu, Play, GameOver };
enum class TroopState{ Idle, Walking, Attacking, Dead };
I think you'll find it helps keep code a little more organized and easier to read through that way.
Use the classes like Troop to handle it's own update code moving it out of the Game class. The Game classes responsibilities are game flow, delegate the responsibilities that belong to the other classes to them.
If some action affects the flow of the game such as the player dying, then that would go in Game, if it only affects the objects, then it goes in a member function of that class.
You don't have to pass in variables belonging to a class to work on them. Your Troop::Draw function for instance can just use it's own instance of the variables instead of having to pass all of them. When you do need to pass troops variables, decide if it would be better/easier to just pass in the entire object by reference or const reference.
Code: Select all
// You have this:
troop[i].Draw(troop[i].x, troop[i].y, troop[i].type, troop[i].selected, troop[i].alive, troop[i].TroopSizeX, troop[i].TroopSizeY, gfx);
// When all you need is this:
troop[i].Draw( gfx );
Consider creating functions for code you find yourself writing over and over. For instance, checking if some point is inside a bounding rectangle seems to be rewritten a few times. You could create a Rect class that stores the left,top,right,bottom of the bounding area and create a member function that checks of a point is inside that area.
instead of :
if( x > left && x < right && y > top && y < bottom ) everywhere
you'd write:
if( GetRect().Contains( x, y ) ) // admittedly everywhere instead
Even make a Overlaps function that takes in another const Rect& object which tests if two bounding rectangles overlap.