Two sprite-drawing functions for Amara game

Question

Ideally I would have one draw method in the parent class (Player).

But I can't figure out how to do this. I don't know if there is anyway to decouple it totally. I don't mind modifying the sprite sheet so that it can handle this sort of thing.

Any tips are very welcome!!

Both classes inherit from Player.

var skeleton = Player(x, y, hp, name, moveSpeed);
//drawing the fly
var draw = function(ctx) {
if (flyAnimate >= 30){
 flyAnimate = 0;
}
var bugX = canvas.width/2 + fly.getDrawAtX() - localX - 50;
if (fly.getAlive()){
 ctx.fillStyle="#FF0000";
 ctx.fillRect(bugX+30,fly.getDrawAtY()-50,((fly.getHp()/2.2)),6);
} else {
 ctx.fillText("DEAD", bugX + 37, fly.getDrawAtY()-40);
}
if (flyAnimate <= 10){
 ctx.drawImage(flySprite,0,0, 100, 100, bugX,fly.getDrawAtY()-50, 100, 100);
}
else if (flyAnimate <= 20){
 ctx.drawImage(flySprite,100,0, 100, 100, bugX,fly.getDrawAtY()-50, 100, 100);
}
else if (flyAnimate <= 30){
 ctx.drawImage(flySprite,200,0, 100, 100, bugX,fly.getDrawAtY()-50, 100, 100);
}
ctx.drawImage(silverShield, bugX+ 20, fly.getDrawAtY()-3);
if (descendAttack || rightMouseActionHappening){
 if (!rightMouseActionHappening){
 rightMouseActionHappening = true;
 }
 //200 is pretty badass
}
if (descendAttack) {
 ctx.save();
 ctx.translate(bugX+60, fly.getDrawAtY()-40 + 90);
 ctx.rotate(Math.PI);
 ctx.drawImage(silverSword, 0, -10);
 ctx.restore();
} else {
 ctx.drawImage(silverSword, bugX+ 60, fly.getDrawAtY()-40);
}
flyAnimate++;
ctx.fillStyle = "black";
ctx.font = "bold 13px sans-serif";
ctx.fillText(name, bugX + 22, fly.getDrawAtY()-60);
};

// drawing what I call a redhatter

var draw = function(ctx) {
//var drawAtX = skeleton.getX()-50;
if (skeleton.getMoveDirection() === "left"){
 facing_left = true;
} else if (skeleton.getMoveDirection() === "right"){
 facing_left = false;
}
if (facing_left){
 spritesheet_offset_y = 102;
}
else {
 spritesheet_offset_y = 0;
}
var drawAtX = canvas.width/2 + skeleton.getDrawAtX() - localX - 50;
if (skeleton.getAlive()){
 ctx.fillStyle="#FF0000";
 ctx.fillRect(drawAtX+30,skeleton.getY()-50,((skeleton.getHp()/2.2)),6);
 ctx.fillStyle = "black";
} else { /* If it's dead, just write DEAD */
 ctx.fillText("DEAD", drawAtX + 37, skeleton.getY()-40);
}
ctx.fillStyle = "black";
ctx.font = "bold 13px sans-serif";
ctx.fillText(name, drawAtX + 25, skeleton.getY()-60);
/* Decides what sprite to draw*/
if (skeleton.getAnimate() <= 20){
 ctx.drawImage(RedhatterSprite,0,spritesheet_offset_y, 100, 100, drawAtX,skeleton.getY()-50, 100, 100);
}
else if (skeleton.getAnimate() <= 40){
 ctx.drawImage(RedhatterSprite,100,spritesheet_offset_y, 100, 100, drawAtX,skeleton.getY()-50, 100, 100);
}
else if (skeleton.getAnimate() <= 60){
 ctx.drawImage(RedhatterSprite,200,spritesheet_offset_y, 100, 100, drawAtX,skeleton.getY()-50, 100, 100);
} else{
 ctx.drawImage(RedhatterSprite,200,spritesheet_offset_y, 100, 100, drawAtX,skeleton.getY()-50, 100, 100);
}
};

Here is the full source on GitHub.

Answer 1

I stopped reading through the code when I got to this:

if (descendAttack || rightMouseActionHappening){
 if (!rightMouseActionHappening){
 rightMouseActionHappening = true;
 }
 //200 is pretty badass
}

First, that comment means nothing to the code, get rid of it.

Second thing is that the inside operation will never get called when rightMouseActionHappening is true, I am not sure how this is supposed to work, if you try something like

if (true) {
 if (false){
 console.log("true");
 }
}

you won't get an answer from it, so what I mean is that you should write this a little more like this

if (descendAttack && !rightMouseActionHappening){
 rightMouseActionHappening = true;
}

because the inner if isn't going to be called if rightMouseActionHappening is true, so don't test for that.

once we have gone this far, if rightMouseActionHappening is true, reassigning it to true isn't going to hurt it at all, so we might as well drop it off.

if (descendAttack){
 rightMouseActionHappening = true;
}

Now, the next statement is another if checking descendAttack again, so we might just want to merge these together like this

if (descendAttack) {
 rightMouseActionHappening = true;
 ctx.save();
 ctx.translate(bugX+60, fly.getDrawAtY()-40 + 90);
 ctx.rotate(Math.PI);
 ctx.drawImage(silverSword, 0, -10);
 ctx.restore();
} else {
 ctx.drawImage(silverSword, bugX+ 60, fly.getDrawAtY()-40);
}

---

all of your code could use another indentation inside of the functions, so that I know that it is inside of a function.

---

if (skeleton.getMoveDirection() === "left"){
 facing_left = true;
} else if (skeleton.getMoveDirection() === "right"){
 facing_left = false;
}

I would change this into a simple assignment like this

facing_left = skeleton.getMoveDirection() === "left"

and this

if (facing_left){
 spritesheet_offset_y = 102;
} else {
 spritesheet_offset_y = 0;
}

I would change into a ternary assignment statement like this

spritesheet_offset_y = facing_left ? 102 : 0;

• bezzoon: Thank you! This question is ancient and I have rewritten this code. If it makes you feel any better a lot of this has already been sorted out!
• Malachi: You should post what we call a selfie, and show how you sorted everything out. I would love to see the code and read the review.