Health starts to glitch up


#1

So i’m trying to create a space shooter as a simple beginning, but I’ve ran into a issue.
My health variable starts to glitch up when I update the position of the star particles.
I have no idea why it is glitching up since I am not even touching that variable inside that function.
My speculation is like a memory overflow or something.
The function I’m talking about is called updateStarPosition() and is located in maingame.h

Any help would be appreciated :slight_smile:

(btw, you have to change the file extension from mp4 to zip)


#2

You can check your RAM usage in ProjectABE, an Arduboy emulater. If you’re compiling and getting low RAM warning, then the stack might be getting full. The stack starts at the top of the RAM if I remember. You can post the source code on the forums.


#3

For some reason I can’t use the debugging options when I give projectABE a precompiled HEX, and it also doesn’t want to compile my code since as it says arduboy is not defined while if I compile it on my pc it compiles without issues


(Simon) #4

@Kiwi is right, you really need to have a chunk of RAM as headroom for the stack. The warning is a bit arbitrary and really depends on the code you are running - deep, nested calls require more memory, shallow ones not.

Post your code!


#5

I converted the arrays from integer to byte and reduced the size of the arrays to cut down the memory usage, but even then it still glitches out. As far as I know I’m still under the memory limit (8 * 16 * 3 = 384 bytes, it says it has 1232 bytes of RAM left). Is there something else I’m missing?


#6

I just downloaded the file(copy video location and paste it in another tab got it to download.), I was thinking it was a movie file of the glitch in action. I’m looking at the code now.


(Simon) #7

A very quick look …

int starPositionX[31];
int starPositionY[31];
int starVelocity[31];

These appear to hold values from 0 - 128. If so, you can save a lot of memory by making them unsigned 8-bit ints.

uint8_t starPositionX[31];
uint8_t starPositionY[31];
uint8_t starVelocity[31];

A better approach would be:

struct Star {
  uint8_t x,
  uint8_t y,
  uint8_t velocity  
};

Star stars[31];

Actually, I have just compiled it and do not think RAM is toy problem.


#8

The reason is that the data being entered into RAM over[31] so it was entering data into your game.health and so on. So I gave int starArraySize a fixed number of 30. I don’t know what, ‘sizeof(starPositionX) / sizeof(starPositionX[0]);’ is doing. It’ll enter something into variable[32 and up] without warning, so have to be careful. Anyway, here’s the fixed source,

maingame.h:

int starPositionX[31];
int starPositionY[31];
int starVelocity[31];

bool setupStarArray = true;

int starArraySize = 30;
//sizeof(starPositionX) / sizeof(starPositionX[0]);

void drawHUD() {
  arduboy.fillRect(0, 0, 128, 15, WHITE);
  arduboy.fillRect(1, 1, 126, 13, BLACK);

  arduboy.setCursor(5, 4);
  arduboy.print("HP");
  Serial.println(game.health);
  arduboy.fillRect(18, 4, 25, 7, WHITE);
  arduboy.fillRect(19 + (game.health / 4), 5, 23 - (game.health / 4), 5, BLACK);

  arduboy.setCursor(45, 4);
  arduboy.print(game.score);

  arduboy.drawFastVLine(127, 0 , 15, WHITE);
}

void setRandomStar(int index, bool firstRun) {
  if (firstRun) {
    starPositionX[index] = random(0, 128);
  } else {
    starPositionX[index] = 128;
  }

  starPositionY[index] = random(16, 64);
  starVelocity[index] = random(1, 4);
}

void updateStarPosition() {
  //When I comment this function, the health variable doesn't glitch out
  for (int i = 0; i <= starArraySize; i++) {
    if (starPositionX[i] < 0) {
      setRandomStar(i, false);
    } else {
      starPositionX[i] -= starVelocity[i];
    }
  }
}

void drawStars() {
  for (int i = 0; i <= starArraySize; i++) {
    arduboy.drawPixel(starPositionX[i], starPositionY[i], WHITE);
  }
} 

void mainGame() {  
  
  game.score += 1;
  if (setupStarArray) {
    setupStarArray = false;
    for (int i = 0; i <= starArraySize; i++) {
      setRandomStar(i, true);
    }
  }
  updateStarPosition();
  drawStars();
  drawHUD();
}

(Simon) #9

This will give you the number of elements in the starPositionX array. He divides the length of the array in bytes (which is 31 * 2) by the size of onw of the elements (2 bytes) to derive 31.

I would always make this a constant and use it instead to both declare the array and to iterate through it.


#10

Ahh, then the result was 31. I entered 31 as a test before trying 30, so that was the cause of the glitch.


#11

So I’ve modified my code so it uses a struct for the stars, but now it glitches up even more. It can’t even show the actual game for more than 1/4th of a second before giving me a unknown game state error. I think its going over the gamestate variable now as well :confused:
For some reason the gamestate becomes 1.

This is the code for maingame.h now:

bool setupStarArray = true;

//byte starArraySize = sizeof(starPositionX) / sizeof(starPositionX[0]);

struct Star {
  uint8_t x;
  uint8_t y;
  uint8_t velocity;  
};

Star stars[15];
uint8_t starArraySize = 15;

void drawHUD() {
  arduboy.fillRect(0, 0, 128, 15, WHITE);
  arduboy.fillRect(1, 1, 126, 13, BLACK);

  arduboy.setCursor(5, 4);
  arduboy.print("HP");
  
  arduboy.fillRect(18, 4, 25, 7, WHITE);
  arduboy.fillRect(19 + (game.health / 4), 5, 23 - (game.health / 4), 5, BLACK);

  arduboy.setCursor(45, 4);
  arduboy.print(game.score);

  arduboy.drawFastVLine(127, 0 , 15, WHITE);
}

void setRandomStar(byte index, bool firstRun) {
  if (firstRun) {
    //starPositionX[index] = random(0, 128);
    stars[index].x = random(0, 128);
  } else {
    //starPositionX[index] = 129;
    stars[index].x = 129;
  }

  //starPositionY[index] = random(16, 64);
  stars[index].y = random(16, 64);
  //starVelocity[index] = random(1, 4);
  stars[index].velocity = random(1, 4);
}

void updateStarPosition() {
  for (byte i = 0; i <= starArraySize; i++) {
    /*if (starPositionX[i] <= 0 || starPositionX[i] - starVelocity[i] <= 0) {
      setRandomStar(i, false);
    } else {
      starPositionX[i] -= starVelocity[i];
    }*/
    if (stars[i].x <= 0 || stars[i].x - stars[i].velocity <= 0) {
      setRandomStar(i, false);
    } else {
      stars[i].x -= stars[i].velocity;
    }
  }
}

void drawStars() {
  for (byte i = 0; i <= starArraySize; i++) {
    //arduboy.drawPixel(starPositionX[i], starPositionY[i], WHITE);
    arduboy.drawPixel(stars[i].x, stars[i].y, WHITE);
  }
} 

void mainGame() {
  game.score += 1;
  if (setupStarArray) {
    setupStarArray = false;
    for (byte i = 0; i <= starArraySize; i++) {
      setRandomStar(i, true);
    }
  }
  updateStarPosition();
  drawStars();
  drawHUD();
}

#12

When i first read your post I didn’t completely understand it, but after I read it again I understood it and it worked! thanks! :smiley:


(Stephane Hockenhull) #13

@CrazyVito11

The proper coding pattern is:
for (int i = 0; i < starArraySize; ++i) {
not <=

and starArraySize should be your array size, not (array size - 1)

There’s also a convenient macro you want to define:
#define arraylength(x) (sizeof((x))/sizeof((x)[0]))

That lets you do

for (int i = 0; i < arraylength(starPositionX); ++i) {
   // do stuff with starPositionX[i] ...
}

So you don’t depend on knowing the array size and another variable like starArraySize that could be accidentally changed.

This way you don’t need starArraySize at all.


(Pharap) #14

The simplest solution is just to switch <= for <,
though some of the other suggestions are useful for improving the code in general.

Technically velocity is supposed to involve a directional component.
What you’re actually using is speed (a scalar) rather than velocity (a vector).

https://www.mathsisfun.com/measure/speed-velocity.html

The code’s pretty good overall.
My main suggestions for now would be to change your loops to use < and size_t and introduce a star struct.


The syntax is wrong, but it seems @CrazyVito11 realised that.
Might want to correct it for future readers.

There’s actually a better way (which was added to the standard library in C++17),
but most people don’t understand the better way because it involves templates.

It has the same usage syntax.

The advantage is that it will error if you pass it a pointer and that it’s not a macro.
Macros are evil, templates are good.

Technically you shouldn’t use int for indexing, you should use size_t, but otherwise yes,
using < and preincrement are the C++ way.

(On the Arduboy sometimes uint8_t uses less progmem though, so sometimes uint8_t is acceptable.)

(It’s worth nothing that there are certain cases where using a signed type to represent size can result in serious security bugs.)


(Stephane Hockenhull) #15

Iterative baby steps :slight_smile:

The very first thing is to fix the off-by-one-prone habit.

We can get into templates, and metaprogramming much later :smile:

I’d say the next learning step is to start using for-each loops & references whenever possible and completely avoid the counter.

It can be much faster and compact on AVR (passing the pointer/reference rather than having the function recalculate the array+index*size)

void setRandomStar(Star &star, bool firstRun) {
  if (firstRun) {
    star.x = random(0, 128);
  } else {
    star.x = 129;
  }

  star.y = random(16, 64);
  star.velocity = random(1, 4);
}

void updateStarPosition() {
  for (auto &star : stars) {
    if (star.x <= 0 || star.x - star.velocity <= 0) {
      setRandomStar(star, false);
    } else {
      star.x -= star.velocity;
    }
  }
}

void drawStars() {
  for (const auto &star : stars) {
    arduboy.drawPixel(star.x, star.y, WHITE);
  }
}

@CrazyVito11 This form of for-loop above goes over the entire array without having to explicitly calculate the length and use an ‘i’ variable. The compiler knows how big your array is and will do the range automatically.

auto tells the compiler to figure out the type automatically from what is on the other side:

int a = 3;
float b = 1.5f;
auto c = a * b; // c will be a float variable because (float * int) results in a float value

The & means make a reference (not a copy). A reference lets you change what it refers to rather than make a copy.

for(int i=0; i < starArraySize; ++i){
    Star &star = stars[i];

    star.x = 2;
    //  now star[i].x is 2
}

And that is the same as

for(Star &star : stars){
    star.x = 2;
}

or

for(auto &star : stars){
    star.x = 2; 
}
// now all the stars[].x are set to 2

If you pass a reference to a function the function can change the variable passed.

void setRandomStar(Star &star, bool firstRun){ 

If you don’t want the function to change the object (struct/variable) and want to make sure you don’t accidentally change it but still want to pass a reference you use const here’s an example:

inline void drawStar(const Star &star){ 
    arduboy.drawPixel(star.x, star.y, WHITE);
}

void drawStars() {
  for (const auto &star : stars) {
    drawStar(star);
  }
}

bool twinkle = false;
void drawTwinkleStars() {
  twinkle = !twinkle;
  if(twinkle){
    drawStars();
  } else {
    for (const auto &star : stars) {
      if(star.velocity > 2){
        drawStar(star);
      }
    }
  }
}

It’s a bit of a silly example but using const makes sure you don’t accidentally change star in the function. The compiler will give you an error if you try.

Say you meant to do arduboy.drawPixel(-star.x, star.y, WHITE); but instead typed arduboy.drawPixel(--star.x, star.y, WHITE); the compiler will tell you that you can’t decrement star.x inside drawStar because star is const.


(Pharap) #16

You don’t need to fully understand templates to be able to use them.

And as far as ‘metaprogramming’ goes, capturing the extent of the last dimension of an array through a template parameter is pretty simple.

Technically they’re called ‘range-based for loops’ in C++.
The ‘foreach’ terminology used in other languages isn’t used in C++, except for std::for_each in <algorithm>.

I tend to avoid them on Arduboy because I’ve found that they use more memory than a regular for loop in some circumstances.