[WIP] Memory Rhythm

I made a simple memory game for the Pre-Games Jam 5 Warm Up called Memory Rhythm.

The game is similar in style to the classic Simon Says only instead of just repeating the buttons back and forth each button corresponds to a different action and one of 7 notes in an octave. As you progress the speed of each action increases. You get 1 point for each successful step in the sequence as well as bonus points for completing a sequence equal to the number of steps in the sequence. Press either A or B at any time to start a game or restart once lost.
MemoryRhythm.ino.hex (30.4 KB)
Source: https://github.com/tuxinator2009/Arduboy_MemoryRhythm
License: MIT

NOTE: This game was made in one day so it’s fairly basic, but if anyone would like to chip in some artwork I’d be more than happy to oblige. The character doesn’t need single frame animations there can be multiple frames per animation as well and I can modify the code to make it work. Could even use the available space to have some kind of background that scrolls when the player slides left and right as well.

7 Likes

I was able to run your game with ease, but needed to rename the parent folder from Arduboy_MemoryRhythm to MemoryRhythm.

Game

  • The game itself feels smooth in its animation, which is something I am looking forward to understanding how that works.
  • The animation which is simple is pretty solid execution wise. It would have been one thing to just have a stick figure, but having it strife on the road and jump gave it personality.
  • I like the tones used and haven’t worked with the ArduboyTones library yet.

Code Base

I had a long post of questions and things I liked about the code base, but I think instead i’m going to add comments to your code and attempt to explain what is going. If you have a few moments to spare could you please look through my comments and expand on things I missed the mark on?

Can be taken to DMs as to not messy your post.


Very fun game, thanks for contributing!

1 Like

Ah yes this is because github doesn’t support folders/groups to organize repositories into (or I don’t know how) and trying keep things labeled according to what platform they’re for.

Since the game is very simplistic the jump animation just uses a predefined array of y offsets for each frame.

Thanks. I did the animations by simply giving each one 16 steps and different actions do different things in those steps. For example jumping is accomplished by reading a y offset from a lookup table of 16 values. To achieve the slide to the left/right effect I added a little road tile and just offset it by one each step depending on the direction of movement.

Tones are simple enough to work with but songs can take up a sizeable chunk of PROGMEM (4-bytes per note as a note/duration pair). However, for my Shattered Lands project I fit the entire musical ensemble (stripped down version of ArduboyTones and all 7 music tracks) into about 600 bytes (using byte-beat style music).

Actually since the point of this project, and the warm up jam it was made for, is to help others learn by making a very simple, yet amusing, little game I’d say ask away right here. This way questions and answers can be seen by others so that it might help them out as well.

Thanks again. It was rather fun making this simple little thing in all of a day. Perhaps there’s something to it’s simplicity but elegance that might lead to future renditions with better graphics, and maybe even text displayed with each action.

PS: I didn’t put much effort into making nicely optimized code (the lookup tables are in RAM for example) because I wanted to focus on a more coding for beginners. Later, if I start adding more polish to it then I’ll organize the code better and optimize it (even though it won’t effect performance for this game the commit history would give others some useful insights into the optimization process).

EDIT: I think I’ve learned a lot from @Pharap. Not so much on the programming, but more on making nice text walls :P

2 Likes

I made a fork** of your repo and added my comments and questions.

You can traverse my comments and questions by searching // lyellick and jumping to them.

** removed fork in lue of questions posted directly to topic for visibility. Jump To Questions


Feed meeeee!

EDIT:

Text walls are great for new users like me. I’ve stumbled upon a ton of interesting posts with lots of in depth discussion.

2 Likes

Since the code is so small I’ll post it here with your comments for others to see more easily.

Expand for source code
#include <Arduboy2.h>
#include <Sprites.h>
#include <ArduboyTones.h>
#include "graphics.h"

static const uint8_t actionButtons[] = {0, UP_BUTTON, DOWN_BUTTON, LEFT_BUTTON, RIGHT_BUTTON, A_BUTTON, B_BUTTON};
static const uint8_t ACTION_IDLE = 0;
static const uint8_t ACTION_JUMP = 1;
static const uint8_t ACTION_DUCK = 2;
static const uint8_t ACTION_SLIDE_LEFT = 3;
static const uint8_t ACTION_SLIDE_RIGHT = 4;
static const uint8_t ACTION_RAISE_LEFT = 5;
static const uint8_t ACTION_RAISE_RIGHT = 6;
static const uint8_t ACTION_DONE = 7;

static const uint8_t STATE_MAINMENU = 0;
static const uint8_t STATE_LEARN = 1;
static const uint8_t STATE_PLAY = 2;
static const uint8_t STATE_DONE = 3;

// tuxinator
//Using Arduboy2 instead of Arduboy2Base for now so I can use arduboy.print for debug info
Arduboy2 arduboy;
// lyellick
// Using direct declaration to save resources
ArduboyTones tones(arduboy.audio.enabled);
Sprites sprites;
// tuxinator
//notes starts at 208hz since below that can't be heard well on the Arduboy's tiny piezo
//notes stops at 7902hz for similar reasons
//I've also removed all the sharps/flats to keep it to normal notes for increased pitch as the player progresses
//5 octaves with 7 notes per octave and 6 actions to perform leaving 1 note for the interim beat.
uint16_t notes[] =
{
	 262, 294, 330, 349, 392, 440, 494, //octave 0
	 523, 587, 659, 698, 784, 880, 988, //octave 1
	1047,1175,1319,1397,1568,1760,1976, //octave 2
	2093,2349,2637,2794,3136,3520,3951, //octave 3
	4186,4699,5274,5588,6272,7040,7902  //octave 4
};
uint8_t jumpOffsets[] =
{
	4, 8, 11, 14, 16, 18, 19, 20,
	20, 19, 18, 16, 14, 11, 8, 4
};

uint32_t startSeed;
uint32_t currentSeed;
uint32_t score = 0;
uint8_t octave = 0;
uint8_t action = ACTION_IDLE;
uint8_t actionDuration = 0;
uint8_t actionRate = 5;
uint8_t increaseRate = 6 - actionRate;
int8_t roadOffset = 0;
uint8_t state = STATE_DONE;
uint8_t numMoves = 3;
uint8_t currentMove = 0;

// lyellick
// Is this basically scrambling the generated seed to make it even more random?
// I have rudimentary understanding of left/right shifting and bitwise operators, 
// but is there a reason you chose prime numbers?
void rnd()
{

	currentSeed ^= currentSeed << 13;
	currentSeed ^= currentSeed >> 7;
	currentSeed ^= currentSeed << 17;
}

void setup()
{
	arduboy.begin();
	arduboy.clear();
	arduboy.setFrameRate(60);
	startSeed = currentSeed = arduboy.generateRandomSeed();
}

void loop()
{
	if (!arduboy.nextFrame())
		return;

	// lyellick
	// When the actionDuration is 0, meaning done animiating, it checks the state it should prep for.
	if (actionDuration == 0)
	{
		// lyellick
		// Sets up the moves it is about to show you until the max number of moves have been played. Then it lets the player input them.
		if (state == STATE_LEARN)
		{
			rnd();
			// lyellick
			// Could you expand on this line here? 
			// You have your seed, then you half it efficiently with the right shift, but I get lost on why you modulus 6 and add 1.
			// Seems this algorthm always returns a random actions.
			action = (currentSeed >> 1) % 6 + 1;
			actionDuration = 16;
			// tones.tone(notes[action - 1 + octave * 7] + TONE_HIGH_VOLUME);
			++currentMove;
			if (currentMove == numMoves)
			{
				state = STATE_PLAY;
				currentMove = 0;
				currentSeed = startSeed;
			}
		}
		// lyellick
		// I am not totally sure how you are keeping track of what moves were shown to a player and how it is kept track of when the player inputs moves. 
		else if (state == STATE_PLAY)
		{
			action = 0;
			// lyellick
			// The docs say to do it once per frame and since both your pollButtons() are within a condition.
			// I ran the game removing both pollButtons() and placed it right after the nextFrame() code block and it ran the same.
			// Do you place this here because you want pollButtons() close to where you are calling justPressed()? 
			arduboy.pollButtons();
			
			// lyellick
			// This is looping through the available buttons and checking which one was pressed and setting it to the current action.
			// I like this way better than checking via a switch statement, which I tend to do.
			// Pharap gave a code example similar to this.
			for (uint8_t i = 1; i < 7; ++i)
			{
				if (arduboy.justPressed(actionButtons[i]))
					action = i;
			}
			if (action != 0)
			{
				rnd();
				if (action == (currentSeed >> 1) % 6 + 1)
				{
					// tones.tone(notes[action - 1 + octave * 7] + TONE_HIGH_VOLUME);
					actionDuration = 16;
					++currentMove;
					++score;
					if (currentMove == numMoves)
					{
						currentSeed = startSeed;
						score += numMoves;
						currentMove = 0;
						++numMoves;
						--increaseRate;
						if (increaseRate == 0 && actionRate > 1)
						{
							--actionRate;
							++octave;
							increaseRate = 6 - actionRate;
						}
						state = STATE_LEARN;
					}
				}
				else
				{
					state = STATE_DONE;
					action = ACTION_DONE;
				}
			}
		}
		else if (state == STATE_DONE)
		{
			// if (!tones.playing())
			// 	tones.tone(notes[4] + TONE_HIGH_VOLUME, 100, notes[2] + TONE_HIGH_VOLUME, 100, notes[0] + TONE_HIGH_VOLUME, 100);
			arduboy.pollButtons();
			if (arduboy.justPressed(A_BUTTON|B_BUTTON))
			{
				startSeed = currentSeed = arduboy.generateRandomSeed();
				score = 0;
				octave = 0;
				action = ACTION_IDLE;
				actionDuration = 0;
				actionRate = 5;
				increaseRate = (6 - actionRate) * 2;
				state = STATE_LEARN;
				numMoves = 3;
				currentMove = 0;
			}
		}
	}
	// lyellick
	// This must be the core of the animations on screen prior to printing.
	// I was trying to do something similar to this in my September game, but mostly brute forced duration by counting frames.
	// This will be nice to come back and try to impliment this.
	else if (arduboy.everyXFrames(actionRate))
	{
		--actionDuration;
		if (actionDuration <= 4)
		{
			// tones.noTone();         // tuxinator
			if (action != ACTION_JUMP) //end ducking animation slightly early
				action = ACTION_IDLE;
		}
		if (action == ACTION_SLIDE_LEFT)
			--roadOffset;
		else if (action == ACTION_SLIDE_RIGHT)
			++roadOffset;
	}
	if (roadOffset < 0)
		roadOffset += 8;
	else if (roadOffset >= 8)
		roadOffset -= 8;
	// tuxinator
	//Draw some debug info
	arduboy.setCursor(0, 0);
	// tuxinator
	//Always surround constant strings in F() in order to put them in PROGMEM and save precious RAM
	arduboy.print(F("Score: "));
	arduboy.println(score);
	arduboy.print(F("Speed: "));
	arduboy.println(6 - actionRate);
	// tuxinator
	//Draw the current action
	sprites.drawSelfMasked(112, 0, icons, action);
	// tuxinator
	//Draw the sprite
	if (action == ACTION_JUMP)
		sprites.drawSelfMasked(52, 24 - jumpOffsets[actionDuration], sprite, action);
	else
		sprites.drawSelfMasked(52, 24, sprite, action);
	// tuxinator
	//The screen is 16 tiles wide meaning at most 17 can be visible
	for (uint8_t i = 0; i <= 17; ++i)
		sprites.drawSelfMasked(i * 8 - roadOffset, 56, road, 0);
	arduboy.display(CLEAR_BUFFER);
}
// lyellick
// Is this basically scrambling the generated seed to make it even more random?
// I have rudimentary understanding of left/right shifting and bitwise operators, 
// but is there a reason you chose prime numbers?
void rnd()

For the rnd function it actually is just an Xorshift Random Number Generator algorithm that takes a starting value (the seed) and generates a new number (that new number is also the seed for the next number). Giving it the same seed will generate the same sequence which is why there’s a variable startSeed and currentSeed so that when switching over to the player’s control the sequence is able to easily be restarted from the beginning (and re-seeded when a new game begins).

rnd();
// lyellick
// Could you expand on this line here? 
// You have your seed, then you half it efficiently with the right shift, but I get lost on why you modulus 6 and add 1.
// Seems this algorthm always returns a random actions.
action = (currentSeed >> 1) % 6 + 1;

The call to rnd(); just before the comment is important here as it updates currentSeed to the next value in the pseudo-random sequence used to pick a random action.

The (currentSeed >> 1) part is used because I remember these simpler types of pseudo-random number generators tend to have more apparent randomness in the upper bits then the lower ones. So I shift the bits over by 1 to minimize repetition of actions and get a slightly more “random” output. Try playing around the the amount of shifting done (up to 28 for this formula) and see what the resulting sequences looks like (you can change numMoves = 3 to numMoves = 255 to just watch a long sequence of actions for testing).

The second part % 6 is because there are 6 different actions for the 6 different buttons (UP=Jump, DOWN=Duck, LEFT=Slide Left, RIGHT=Slide Right, A=Raise hand on player’s left, B=Raise hand on player’s right). So a modulus by 6 will give us a value between 0 and 5 inclusive (meaning either a 0, 1, 2, 3, 4, or 5). Since ACTION_IDLE=0 and our first action (ACTION_JUMP) starts at 1 we add a 1 to the result of the modulo to get a value from 1-6 inclusive (ie. 1, 2, 3, 4, 5, or 6) which directly corresponds to the chosen action.

// lyellick
// I am not totally sure how you are keeping track of what moves were shown to a player and how it is kept track of when the player inputs moves.

Haha this was the fun little semi-advanced trick making use of how Xorshift Random Number Generators work. I’ll mention again here briefly but when supplying an initial starting value (seed) you will always get the exact same number sequence. So by storing the starting seed for the sequence and running the generator for each step allows the sequence to be started back from the beginning by simply setting currentSeed = startSeed. This way when the player presses a button to perform an action it effectively regenerates the randomly chosen action at that step in the given sequence and sees if it matches the action chosen by the player.

// lyellick
// The docs say to do it once per frame and since both your pollButtons() are within a condition.
// I ran the game removing both pollButtons() and placed it right after the nextFrame() code block and it ran the same.
// Do you place this here because you want pollButtons() close to where you are calling justPressed()? 
arduboy.pollButtons();

The reason I placed the pollButtons here was because I wanted to use the justPressed routine instead of the pressed routine thus forcing the player to let go of the button and press it again for repeated actions (helps with people with slower reaction times from accidentally issuing the same action twice). The way justPressed works is it only fires once between calls to pollButtons(). That is to say if a call to pollButtons() is made and then a button is pressed and held up to the next call to pollButtons() justPressed() will return true for that button but will then be false after subsequent calls to pollButtons() until the button is released and pressed again. Doing it this way allows the player to press and hold the button for the next step in the sequence while the currently chosen action’s animation is played out (only requiring them to wait if repeating the same action more than once).

Normally you would have pollButtons() done every frame though.

// lyellick
// This is looping through the available buttons and checking which one was pressed and setting it to the current action.
// I like this way better than checking via a switch statement, which I tend to do.
// Pharap gave a code example similar to this.
for (uint8_t i = 1; i < 7; ++i)
{
	if (arduboy.justPressed(actionButtons[i]))
		action = i;
}

There is actually an even better method for checking if the correct button was pressed but I didn’t use it since it’s not as easily done (requires creating an Arduboy2EX class to extend the base classes functionality). The trick is to get the previous button state and current button state and generate a justPressed state (each bit corresponding to a button is on if it was just pressed or off if it wasn’t just pressed). Then you could simply check if the value is non-zero (meaning at least one button was pressed) and if it’s equal to the chosen action’s button bit-mask (meaning only that button was just pressed). This can be a little more complex for beginners though so I avoided it intentionally. Not sure which one would take less code though overall.

// lyellick
// This must be the core of the animations on screen prior to printing.
// I was trying to do something similar to this in my September game, but mostly brute forced duration by counting frames.
// This will be nice to come back and try to impliment this.
else if (arduboy.everyXFrames(actionRate))

This part is also interesting because I’m varying the value passed to everyXFrames in order to increase the overall speed as you progress. At speed 1 actionRate = 5 so every 5 frames it steps the animation (thus playing slowly). In the part that checks for a sequence being completed it will track the number of sequences completed since the last speed increase and if it’s equal to the current speed then it’ll increase it by 1 up to a speed of 5 (every 1 frame, 16 frames of animation at 60 frames per second means each animation takes about 0.25 seconds). When the speed increases it also increases the octave for the notes played (thus increasing their pitch as the speed increases).

If you, or anyone else, has anymore questions feel free to ask. Like I said the main goal of this project was to be a very basic (but functional and fun) project for beginners to take a loot at and dissect it’s inner workings. The fact that I stumbled upon something that’s actually been rather fun to play (I find myself pausing what I’m doing and playing it quite frequently now) is kind of cool in it’s own regard.

I really hope this project (and these questions and answers) will help beginners who aren’t really familiar with c++ or even programming in general realize that you can make a nice, fun, simple project with only a small amount of code. Also, there will always be those of us here that are more than willing to help newcomers to the Arduboy, even if they’ve never done c++ before or any programming at all for that matter.

2 Likes

You should create a folder in the Arduboy_MemoryRhythm repository named MemoryRhythm and move MemoryRhythm.ino and graphics.h into it.

3 Likes

To move files on GitHub, just edit the file and add the new path in front of the file name, using / as the separator. The new directory (path) will be created if it doesn’t exist. To move to the parent of a directory, use ../

1 Like

This blew my mind and kinda made everything click. So, if I understood right, rnd() is the key here and thats how it ‘steps’ through actions? So if rnd() is ran once from X seed during learn it running once for play with have the same number since it shares the same seed.

That’s brilliant and is super exciting to me. Can’t wait to implement this!


Can you go into more detail on the animation side of things? How is action affecting drawSelfMasked()'s frames? Maybe I am missing something here.

Simple really. There are 8 possible actions (Idle, Jump, Duck, Slide Left, Slide Right, Raise Left, Raise Right, and Done) and each action corresponds to a single frame of the sprite. So using the archived TeamARG sprite sheet converter I have a 24x32 sprite with 8 frames one for each action. The animation part is handled by actionDuration and is set to 16 and decremented once per step inside the check to everyXFrames giving us 16 steps per animation. Each animation can be setup to do something different, for example if the action is SLIDING_LEFT or SLIDING_RIGHT then the roadOffset variable gets adjusted accordingly, in the case of the JUMP action the sprite is offset by the preset values in the lookup table (one value for each of the 16 steps).

The reason for drawSelfMasked is because my sprites are 1 color (white) with black for transparent. The masks are helpful for drawing sprites with both white and black while also having transparency as the mask will clear away whatever’s behind it (but only the parts masked out) before then drawing the sprite over that.

2 Likes

I see that you’re “abusing” TONE_HIGH_VOLUME and just playing every tone at high volume. It would add a bit of code but you could call
tones.volumeMode(VOLUME_ALWAYS_HIGH);
at the beginning to accomplish this instead of putting TONE_HIGH_VOLUME on every note .

If you find the regular volume too low, that may not be the case for others. You could add a user option to select the volume and use
tones.volumeMode(VOLUME_ALWAYS_NORMAL);
or
tones.volumeMode(VOLUME_ALWAYS_HIGH);
to accomplish this.

1 Like

Yeah it’s hard to tell how it sounds to other users since I have tinnitus so these little piezo speakers are muffled quite a bit. Even with high volume I can barely hear it.

The next update I planned on first adding a menu to set options, and enter a Tutorial mode (allows you to just press buttons to do different poses without any scoring), Demo mode (keeps playing a randomly generated set of sequences), and Normal mode (current mode).

Since I didn’t need the extra space I’m calling the normal arduboy.begin() so the startup functions allowing the user to enable/disable audio work as expected at least.

2 Likes

The issue is with me not understanding bitmaps and how drawing a frame in a bitmap works. I personally need to research and play around with this.

Off to google and the forums!

EDIT:

I actually am really enjoying the tones even through they are loud. Going with a deeper tone for jumping is satisfying for some reason. Plus the game it self is a nice kill a minute game that’s fun.

Though program size isn’t yet an issue here, with the sprite images you’re currently using you might benefit from using Arduboy2Base::drawCompressed() instead of Sprites::drawSelfMasked. I found the images compressed down to about 45% of the original size. For your 8 24x32 sprites you would save about 400 byes.

To keep the code similar to what you have now you could use an array of pointers to the individual sprite arrays. Whether the additional code required to use drawCompressed() would nullify the data savings is something that would have to be tested.

It should be noted that you could have used the Arduino random() function and re-seeded it with randomSeed() as necessary.

One thing I find a bit annoying is that you have to wait for an action to complete before you can press the next button in the sequence. You should be able to enter the sequence as fast as you like.

You could test for a new button using justPressed() while the current action is playing and, if detected, abort the action early and process the new button.

I don’t know if ending the jump sequence early would make things look strange, though. You might need to add a short “quickly end action” sequence when the new button press is detected. This would erase the button icon and return the character to idle in only a few frames. EDIT: maybe you could “quickly end action” by skipping everyXFrames() until the action is complete.

Now, I would like to see your take on a simple game. There is still time left on making a simple game.

True, however I’ve found that this form of RNG is basic enough that if I want to have full control over it’s functionality (sometimes it’s helpful to seed it, generate some numbers, store it’s current state, generate some new numbers, jump back to a previous state). While it’s possible to do that I find it easier to to use this simplistic xorshift RNG.

Yeah, it’s a bit annoying, but it’s also very early on mostly getting it to a releasable state as quickly as possible. Later, when I get a chance, I’ve got ideas for fast forwarding the animation. At the same time I feel the current limitation forces you to pace yourself otherwise it just becomes too much like simon says. I wanted to have the button presses correspond to doing something on screen.

2 Likes

I turn my back for five minutes and suddenly there’s a 16 new comments. :P


You could create some organisations - one for your Arduboy stuff and one for your Pokitto stuff.
Not really what organisations are intended for, but it would probably work.

(I was also going to suggest what @MLXXXp said, but he beat me to it.)

I’d give bonus likes/points for this if it were an option.

I want to print this out and frame it so I can point to it if I ever get any complaints. :P

I expect I’ve probably told you this before, but just to raise awareness of it, using % with a non-power-of-two will incur modulo bias.

There’s actually two ways of doing it:

Do I take it that this means you didn’t know how PRNGs work before this?

If so I’m genuinely surprised.

Now I’m tempted to run it with the volume on to see what it’s like since I have an almost opposite problem.

It’s quite simple really…
Here’s an example adapted from Easter Panic:

constexpr uint8_t smallRabbitWidth = 8;
constexpr uint8_t smallRabbitHeight = 8;

constexpr uint8_t smallRabbit[] PROGMEM
{
	// Dimensions
	smallRabbitWidth, smallRabbitHeight,
	
	// Frame 0 (North)
	0x00, 0x78, 0x7E, 0x78, 0x78, 0x7E, 0x78, 0x00,
	
	// Frame 1 (East)
	0x00, 0x00, 0x78, 0x7E, 0x48, 0x78, 0x00, 0x00,
	
	// Frame 2 (South)
	0x00, 0x78, 0x4E, 0x78, 0x78, 0x4E, 0x78, 0x00,
	
	// Frame 3 (West)
	0x00, 0x00, 0x78, 0x48, 0x7E, 0x78, 0x00, 0x00,
};

The Sprites image format is an array of uint8_t that consists of a width, a height, and then a sequence of frames with each frame being a single bitmap.

The sprite drawing functions figure out how large each frame should be using the width and height, and from that they can access the correct frame by offsetting the start point by that many bytes.

So in this case each frame is (8 * 8) / 8 ((width * height) / bits_per_uint8_t), which is 8 bytes.
Then the start of any specified frame is smallRabbit[2 + (8 * frameIndex).

To simulate frames with arduboy.drawBitmap or Arduboy2Base::drawCompressed you’d typically use an array, although theoretically you could actually use a Sprites-formatted image with the right code.

E.g.

inline void drawBitmap(Arduboy2 & arduboy, int16_t x, int16_t y, const uint8_t * image, uint8_t frameIndex)
{
	const size_t width = pgm_read_byte(&image[0]);
	const size_t height = pgm_read_byte(&image[1]);
	const size_t frameSize = ((width * height) / 8);	
	const uint8_t * frame = &image[2 + (frameSize * frameIndex)];
	
	arduboy.drawBitmap(x, y, frame, width, height, WHITE);
}

Though that would make the game harder to port accurately.
random doesn’t guarantee what PRNG engine is used,
whereas using a specific PRNG engine does provide that guarantee,
thus allowing the game to play exactly the same sequences even when ported.

Some Arduino environments don’t even provide random (as I found out when publishing my fixed points library).

Xorshift is probably cheaper anyway.

3 Likes

Normally I try to avoid double-posting, but I think in this case it makes sense for this to be separate…


If we’re critiqueing the code, my main complaints/suggestions would be:

  • You should use constexpr instead of static const
    • Con.5: Use constexpr for values that can be computed at compile time
    • static (in the context of global variables) actually means “only make this visible to this translation unit” and hence is completely pointless here.
      • Presumably you’ve been told the partial myth that static const saves memory or is more efficient. That’s a C-ism and there’s only limited truth to it. In C static const int x = 0; and const int x = 0; have different meanings, but in C++ that isn’t the case (for a global variable at least).
    • constexpr is better than const because it guarantees that the variable can be computed at compile time
  • You shouldn’t use all caps (typically called “macro case” in the C++ world) for anything other than macros
  • Ideally you ought to be using an enum class instead of semantically unrelated variables
    • Enum.2: Use enumerations to represent sets of related named constants
    • enum classes are type safe and provide a semantic relationship between several alternative values.
    • If you use an enumeration in a switch and forget to include a case label for any value, you’ll get a warning, which can help prevent mistakes
    • E.g. An enumeration for action would look like this:
    enum class Action : uint8_t
    {
    	Idle,
    	Jump,
    	Duck,
    	SlideLeft,
    	SlideRight,
    	RaiseLeft,
    	RaiseRight,
    	Done,
    };
    
    Action action = Action::Idle;
    
  • state comparisons should ideally be in a switch rather than a long list of ifs because switch is more readable and more importantly is a better way to communicate your intent.
    • switch effectively says “I want to do several different things depending on the exact value of this specific variable”, whereas if is much more broad and doesn’t communicate the intent as well.

Part of the reason I’m picking on these things is because I want beginners to learn good habits early on.

Less important but still worth mentioning:
  • You don’t actually need a sprites object, you can just do Sprites::drawSelfMasked.
    • The advantage of the latter is that getting access to global variables is more awkward than getting access to static functions. The former requires an extern declaration somewhere, the latter only requires #include <Arduboy2.h> or #include <Sprites.h>
  • Splitting loop up into more functions would make it a bit easier to digest.
    • Contrary to what you might expect, this actually reduces code size. The compiler has an easier time digesting lots of smaller/medium-sized functions than it does one large function. Think of it like this: it’s easier to eat slices/mouthfuls of cake than it is to eat a whole cake in one go.
  • It would be nice if your arrays are also marked constexpr to indicate that they’re never modified.
  • Personally I (among others) consider chained assignment to be bad form.
  • A few more blank lines would help the readability.
    • (In particular I was almost caught out by the if after else if (arduboy.everyXFrames(actionRate)), for a moment I thought it was another else if in the chain.)
  • rnd() probably ought to return the calculated seed.
    • E.g. uint32_t value = rnd(); if (action == (value >> 1) % 6 + 1) ...
    • It actually works out cheaper because local variables are typically cheaper than global variables because they can be kept in registers and the compiler can prove that they haven’t been changed. The compiler can’t prove that a global variable hasn’t been changed since it last read the value. This is especially true on AVR.
  • rnd() isn’t really a very descriptive name. Contractions make life harder. I would presume that’s especially true for beginners.
    • To put it another way, contractions are how C ended up with function names like strxfrm which C++ inherited. Without checking the documentation I haven’t the first clue what that function does and I’m really glad I’ve never had cause to use it. Theres a long list of similar offenders.
  • numMoves is also a relatively cryptic name. Contractions aside, ‘number’ in what sense? Quantity, capacity, count, limit?
    • For comparison, names like actionRate and currentMove are good names because they’re descriptive and precise.
  • Really rnd ought to be implemented as a class to make it more useful, but I can understand not doing that to try to make it easier for beginners to digest.

Here’s a version of your code incorporating a number of these suggestions.
(It’s 50 bytes smaller as a result.)

Code
#include <Arduboy2.h>
#include <ArduboyTones.h>
#include "graphics.h"

enum class Action : uint8_t
{
	Idle,
	Jump,
	Duck,
	SlideLeft,
	SlideRight,
	RaiseLeft,
	RaiseRight,
	Done,
};

enum class State : uint8_t
{
	MainMenu,
	Learn,
	Play,
	Done,
};

constexpr uint8_t actionButtons[7]
{
	0,
	UP_BUTTON,
	DOWN_BUTTON,
	LEFT_BUTTON,
	RIGHT_BUTTON,
	A_BUTTON,
	B_BUTTON,
};

//Using Arduboy2 instead of Arduboy2Base for now so I can use arduboy.print for debug info
Arduboy2 arduboy;
ArduboyTones tones(arduboy.audio.enabled);
Sprites sprites;

//notes starts at 208hz since below that can't be heard well on the Arduboy's tiny piezo
//notes stops at 7902hz for similar reasons
//I've also removed all the sharps/flats to keep it to normal notes for increased pitch as the player progresses
//5 octaves with 7 notes per octave and 6 actions to perform leaving 1 note for the interim beat.
constexpr uint16_t notes[]
{
	 262, 294, 330, 349, 392, 440, 494, //octave 0
	 523, 587, 659, 698, 784, 880, 988, //octave 1
	1047,1175,1319,1397,1568,1760,1976, //octave 2
	2093,2349,2637,2794,3136,3520,3951, //octave 3
	4186,4699,5274,5588,6272,7040,7902  //octave 4
};

constexpr uint8_t jumpOffsets[]
{
	4, 8, 11, 14, 16, 18, 19, 20,
	20, 19, 18, 16, 14, 11, 8, 4
};

uint32_t score = 0;
uint8_t octave = 0;

Action action = Action::Idle;
uint8_t actionDuration = 0;
uint8_t actionRate = 5;
uint8_t increaseRate = (6 - actionRate);

int8_t roadOffset = 0;
State state = State::Done;
uint8_t numMoves = 3;
uint8_t currentMove = 0;

uint32_t startSeed = 0;
uint32_t currentSeed = 0;

uint32_t generateValue()
{
	currentSeed ^= currentSeed << 13;
	currentSeed ^= currentSeed >> 7;
	currentSeed ^= currentSeed << 17;
	return currentSeed;
}

Action generateAction()
{
	return static_cast<Action>((generateValue() >> 1) % 6 + 1);
}

void setup()
{
	arduboy.begin();
	arduboy.clear();
	arduboy.setFrameRate(60);
	
	startSeed = arduboy.generateRandomSeed();
	currentSeed = startSeed;
}

void loop()
{
	if (!arduboy.nextFrame())
		return;
		
	update();
	draw();

	arduboy.display(CLEAR_BUFFER);
}

void update()
{
	if (actionDuration == 0)
	{
		switch(state)
		{
			case State::MainMenu:
				updateMainMenuState();
				break;
		
			case State::Learn:
				updateLearnState();
				break;
				
			case State::Play:
				updatePlayState();
				break;
				
			case State::Done:
				updateDoneState();
				break;
		}
	}
	else if (arduboy.everyXFrames(actionRate))
	{
		--actionDuration;
		if (actionDuration <= 4)
		{
			tones.noTone();
			if (action != Action::Jump) //end ducking animation slightly early
				action = Action::Idle;
		}
		
		if (action == Action::SlideLeft)
			--roadOffset;
		else if (action == Action::SlideRight)
			++roadOffset;
	}

	if (roadOffset < 0)
		roadOffset += 8;
	else if (roadOffset >= 8)
		roadOffset -= 8;
}

void draw()
{
	drawDebugInfo();

	//Draw the current action
	sprites.drawSelfMasked(112, 0, icons, static_cast<uint8_t>(action));

	//Draw the sprite
	if (action == Action::Jump)
		sprites.drawSelfMasked(52, 24 - jumpOffsets[actionDuration], sprite, static_cast<uint8_t>(action));
	else
		sprites.drawSelfMasked(52, 24, sprite, static_cast<uint8_t>(action));

	//The screen is 16 tiles wide meaning at most 17 can be visible
	for (uint8_t i = 0; i <= 17; ++i)
		sprites.drawSelfMasked(i * 8 - roadOffset, 56, road, 0);
}

void updateMainMenuState()
{
	// Currently unimplemented
}

void updateLearnState()
{
	action = generateAction();
	actionDuration = 16;
	
	tones.tone(notes[static_cast<size_t>(action) - 1 + octave * 7] + TONE_HIGH_VOLUME);
	++currentMove;
	
	if (currentMove == numMoves)
	{
		state = State::Play;
		currentMove = 0;
		currentSeed = startSeed;
	}
}

void updatePlayState()
{
	arduboy.pollButtons();
	action = Action::Idle;
	
	for (uint8_t i = 1; i < 7; ++i)
	{
		if (arduboy.justPressed(actionButtons[i]))
			action = static_cast<Action>(i);
	}
	
	if (action != Action::Idle)
	{
		if (action == generateAction())
		{
			tones.tone(notes[static_cast<size_t>(action) - 1 + octave * 7] + TONE_HIGH_VOLUME);
			actionDuration = 16;
			
			++currentMove;
			++score;
			
			if (currentMove == numMoves)
			{
				currentSeed = startSeed;
				score += numMoves;
				currentMove = 0;
				
				++numMoves;
				--increaseRate;
				
				if (increaseRate == 0 && actionRate > 1)
				{
					--actionRate;
					++octave;
					increaseRate = 6 - actionRate;
				}
				
				state = State::Learn;
			}
		}
		else
		{
			state = State::Done;
			action = Action::Done;
		}
	}
}

void updateDoneState()
{
	if (!tones.playing())
		tones.tone(notes[4] + TONE_HIGH_VOLUME, 100, notes[2] + TONE_HIGH_VOLUME, 100, notes[0] + TONE_HIGH_VOLUME, 100);
		
	arduboy.pollButtons();
	
	if (arduboy.justPressed(A_BUTTON | B_BUTTON))
	{
		startSeed = currentSeed = arduboy.generateRandomSeed();
		score = 0;
		octave = 0;
		action = Action::Idle;
		actionDuration = 0;
		actionRate = 5;
		increaseRate = (6 - actionRate) * 2;
		state = State::Learn;
		numMoves = 3;
		currentMove = 0;
	}
}

void drawDebugInfo()
{
	//Draw some debug info
	arduboy.setCursor(0, 0);

	//Always surround constant strings in F() in order to put them in PROGMEM and save precious RAM
	arduboy.print(F("Score: "));
	arduboy.println(score);
	arduboy.print(F("Speed: "));
	arduboy.println(6 - actionRate);
}
2 Likes

I’ve submitted a pull request on GitHub to do this.

But the game starts with a call to generateRandomSeed() in setup(), so there’s no intent to ever have exactly the same sequence. It may be beneficial during development but not after release.

You didn’t put markdown code tags around your code. I added them.