This is a code review on group 15’s project “Geneva Lost”. In this report I am going to look into how their player class is implemented in their game and the types of connections it has with other classes. Simply put; what makes the player do stuff.
Their player needs a few things to make it work:
It inherits from a base class called GameObject.
A Entity class which is derived from GameObject.
It also needs a DrawManager which renders the players texture onto the screen.
A SpriteManager is needed to create a sprite from a sprite sheet.
And a CollisionHandler which in turn checks the collision between objects.
So the first thing I noticed in their project is that they are not using a player class, they are instead implementing everything that the player needs to function as it is suppose too in the GameState. So the problem now is that if they were thinking about changing anything that has to do with the player they have look through the entire code in GameState instead of only having to change things in a player class. This also makes a very long list of code if they implement a lot of different entities the same way because now, everything the player does is written in the GameState.
In the current state of this project they create a player from a sprite, they move the player in the update function and they draw the player in the draw function all in the GameState. This means that in the update function they have approximately 100 lines of code to move the player. If they were to add an enemy that another player could control in the same way, they would have 200 lines and so on. In my opinion the way they should have done this is by having a player class where they define how the player moves and so on because now the GameState is not just for the player, it is also where they create the background, declare the enemies health, declare the projectiles damage and how it moves.
They do have a enemy class however but no player class so I guess that makes it a bit easier, but still, a player class is needed in my opinion.
Let’s say that a bug appears and causes the player to move in a weird way or something along those lines. It would be a lot easier to go through a player class that handles all that and look for the error. Instead they have to look through code that may not have anything to do with the player just because it is in the same place.
Even though it may not be troubling now to change the players properties but imagine you had a project four times as big and that the player has a lot of new things it can do. That would make bug fixing a nightmare and if one thing changes a lot of other thing may change as well.
To make this easier I suggest you make a player class, like you made an enemy class and do everything that has to-do with the player there. Then you can add what it needs there instead of adding it in the GameState which is considered high or tight coupling, since if something goes wrong in the players code the game would most likely not compile.
A couple of hours before deadline, I decided to check in with their project and see how and what they were doing and I noticed something. They have now a player class! The player class however doesn’t do very much right now, but at least they have one and that is great! The player class in its current state has an Update function that handles the movement and attaches a camera to the player so it can follow the one that is playing and we can see what going on. They also move the player in this Update function, they tell it that certain buttons will move the player in a certain way. I like that they started this at least, its a solid ground to build upon. However, the GameStates Update function also tells the player how it moves, by binding the same keys to the same movement. I don’t know if this was done on purpose, I doubt it was though because it’s reduntant code. But as I said it seems like they know what they’re doing and that they are on the right track. The code is very easy to read and understand but if I want to find something specific, like what happens to the player if I pick up this power up, it is going to be slightly more difficult. And I have to say that I really like the way you have connected your other classes like the GameObject and Entity. The game at its current state looks and plays awesome, can’t wait to see the finished version, keep it up!
