It took me a while to find the code, the first thing I have to say is... the .h files are meant to be used for declaration, the implementation should be in .cpp files. You're writing everything together. It works, but it's a really standard practice to separate in 2 files.
You're using a lot of globals, this can create bugs more often and makes bug fixing a lot harder. Try initializing what you need only when you need it, and try sending as a parameter only what an object or method should have access to. For example, look at main.cpp, inside the while loop you're using a "link" variable that's magically there, it's not clear where you created that object.
Code like thise one is confusing:
if(state == titleScreen)
{}
else if(state == fileScreen)
{}
else if(state == overworld)
{
Why check for those states if you don't do anything?
Also, that if/else/if chain is really unmaintainable, there's a lot of repeated code in there and maybe it should be designed around the State pattern.
You're using A LOT of magic numbers, you're using exact pixel values like "1274", it's obvious why you don't want to code anymore, I imagine it was a real pain to test and fix bugs that way. I see a lot of code like this one:
//First town
if(owlink.sprite.x+owlink.sprite.width > 1216 &&
owlink.sprite.x+owlink.sprite.width < 1248 &&
owlink.sprite.y+owlink.sprite.height >1216 &&
owlink.sprite.y+owlink.sprite.height <1248)
{
link.linkSprite.x = 96;
link.linkSprite.y = 481;
link.linkSprite.direction = 1;
ChangeState(state, town1);
}
It looks like you're doing collision detection the hardest way possible, it would be a lot easier if you model things with a collision box, with all those position and size values as attributes, and make an iteration over all the boxes. I see you have some kind of collision detection for enemies, you can do the same for doors.
Try something like a Door class with x, y, width and height position, and a "nextState" attribute, so you can rewrite that code to something like:
for (/*iterate over all the doors*/) {
if(owlink.over(door))
{
// I guess all this values are related to each state, so I changed it to a function
//link.linkSprite.x = 96;
//link.linkSprite.y = 481;
//link.linkSprite.direction = 1;
link.setStartingPosition(door.nextState);
ChangeState(state, door.nextState);
break; //Stop iterating if you changed state
}
}
You also do things like this:
else if(state == zeldaPalace)
{
link.doJump();
link.attack();
link.flinchAnim();
}
else if(state == town1)
{
link.doJump();
link.attack();
link.flinchAnim();
}
You know you can use && and || operators when writing a condition, right? You should write that code this way:
else if(state == zeldaPalace || state == town1) // add all the states that repeat this code
{
link.doJump();
link.attack();
link.flinchAnim();
}
Or you can try with a switch instead of chained if/else code:
switch(state) {
// more states with their own block of code
case zeldaPalace:
case town1: // add all the states that repeat this code
{
link.doJump();
link.attack();
link.flinchAnim();
break;
}
}
You repeat a lot of code (which makes it redundant) but also comment redundant things that make everything larger than it should. Things like:
//Draw link
link.drawLink();
Can be totally avoided, that comment makes it look like it's something you should know before continuing, but then you read the code and it says the same.
Also, talking about redundant things a "drawLink" method on the "link" object could be called just "draw"... if you want to draw link you write "link.draw" and it's already clear enough. And inside the Link class some attribute names start with "link"... "linkCamera", "linkSprite", "linkInput"... just name them "camera", "sprite", "input", they belong to link since you defined those attributes in the class already.
Try to keep comments up to date, or make the code self documented. For example:
//Draws link onto the screen, 1 = right 0 = left
void Link::drawLink()
It looks like you had at a time a parameter on that function that could have a value of 1 or 0. You don't have that, but the comment is still there. If you write self documented code you can at one time have this:
void Link::drawLink(FacingDirection dir)
And a FacingDirection enum with "left" and "right" as valid values, and you don't need a comment. If later you decide to remove the parameter, there's nothing else to update.
Talking about Direction enum, you're redefining that enum in different classes, this enum in particular could be defined in globals.h, once.
There's a lot of repeated code everywhere, and the file distribution makes it harder to understand, I'll stop right now, everything I said happens again and again in every file.
Try cleaning things, avoid repeated code and hardcoded pixel values as much as possible, try thinking more about objects and classes and do some research on common design patterns used in games.
EDIT: Looking at the video it's amazing that you did that much, I suggest you keep coding, you can do a lot more in the same time if you have more practice.
Only watched the video, but it seems to be really a finished game..
).
