Code Review: Projectile Array

Need another pair of eyes on this. Trying to have 4 projectiles on the screen at once (might go for 3 or 2 later) so I’m using an array of Structs. I’m using the X coord as the “active” flag by setting it to 255 to show it as not active. For some reason it’s never creating a second projectile and always just reusing the first one. So if I shoot another one while the first is on the screen it just jumps back to the character each time. Also even though I’m checking if the X coord is less than negative WIDTH or negative tile width it always resets when the x coord gets to zero so it disappears as soon as it “touches” the left side where it will go all the way off the right side of the screen.
DigginDog.zip (14.5 KB)

Your problem is that you’re using = (the assignment operator) instead of == (the equality operator).

void createNewWave(int x, int y, bool facingLeft)
{
  Wave newWave;
  for (int i = 0; i < 4; i++) {
    newWave = waves[i];
    if (newWave.x = 255) {
      newWave.x = x;
      newWave.y = y;
      newWave.facingLeft = facingLeft;
      waves[i] = newWave;
      break;
    }
  }
}

Note if (newWave.x = 255) { instead of if (newWave.x == 255) {.

To further elaborate, what happens is that newWave.x is assigned the value 255 (thus the value is reassigned every single time), then the value of newWave.x is implicitly cast to bool, which will be true for any value other than 0, thus the code within the if will run every time on the first iteration of the loop, thus creating the problem you’re seeing.

x will never be less than negative width because it will never be negative.
It’s currently a uint8_t, which means it’s an unsigned 8-bit integer, and unsigned integers can’t be negative. They fit a range of 0 to 255 (0 to 28-1).


If you go into File > Preferences, and set the value in the combo box at Compiler warnings: to All, the compiler will start giving you warning messages to tell you when something looks amiss.

Two of these warnings identify the aforementioned issues:

I:\Downloads\DigginDog\DigginDog.ino:318:45: warning: comparison is always false due to limited range of data type [-Wtype-limits]

  if (thisWave.x >= WIDTH || thisWave.x <= -WIDTH) {

And

I:\Downloads\DigginDog\DigginDog.ino:337:19: warning: suggest parentheses around assignment used as truth value [-Wparentheses]

if (newWave.x = 255) {

    ~~~~~~~~~~^~~~~

You also get a few warnings about narrowing conversions (trying to assign values that are too large to fit in the target type), and one about an unhandled enumeration value in a switch.


I know this is more than what you asked for, but I’d also advise replacing your game states, player facing direction and tile types with an enum class as you have done for the player states. Scoped enumerations add better type safety, and you get warnings when you don’t handle a value in a switch, which can help prevent accidental oversights.

You may also be better off using a getSize function like implemented here rather than ARRAY_LEN for the reasons specified here.

2 Likes

This is why code reviews work so well. I can’t believe I missed that = vs == like I’ve found for so many other’s code reviews. Thanks so much for all the tips. I’ll make sure to implement those changes right away.

1 Like