Alien Mine - based on Tempest

Alien_Mine.ino.arduino_leonardo.hex (42.3 KB)

GitHub link: GitHub Link

HEX direct download : https://github.com/Tomsk666/arduboy-alien-mine/releases

Keys: LEFT - move anti-clockwise around the top of the mine
RIGHT - move clockwise around the top of the mine
A - Shoot missile into mine
B - Drop a nuke bomb down the mine to kill all aliens! (only available when your power-up progress bar at top of screen is full)
UP - Pause game / un-pause
Currently 5 levels.
My 3rd game since having the Arduboy at Christmas, I’m definitely learning lots & getting my code more organised. Any feedback welcome! Thanks for playing

If you like it, prob best to download from github link above as I’m still improving it.

4 Likes

//Thanks to @Pharap for this code which I found on the Arduboy forum

Can you remember where on the forum that was?
(I’d like to check whether it had a licence or not.)


Also, are you wanting/expecting any feedback?
I can see one or two places where you could make your code more efficient.

Hi Pharap, it was here:

You were helping someone else out with an example. I found it trying to work out how to make a dynamic array in C++ so would have been stuck without it. Hope it’s OK that I used it?
Feedback would be great, I’ve only been going since December, so any tips/help are much appeciated. Thanks

1 Like

Ok, fair enough, I’d forgotten about that one.

It’s fine that you used it, but my example there was a very quickly made example for the sake of demonstration and it could have been much better.

E.g. that int N should be size_t capacity, the (void)s should be (), the brackets should be on separate lines and some of those functions could be constexpr, and most importantly I should have included a licence with it.

I have some better examples somewhere, but every time I try to make a utility library with useful 'collection types (array, list, stack, queue et cetera) I fall into the feature creep trap and end up implementing a large chunk of the C++ standard library…

There’s quite a few things you could do, but I’ll just focus on a few for now.

sounds variable and 'String'

Firstly, instead of making sounds a String,
you’d be better off getting rid of sounds and then doing something like…

arduboy.print(F("   B - Sound: "));
arduboy.print((arduboy.audio.enabled()) ? F("on") : F("off"));

Then you can just have:

// B Button Toggle Sound On/Off
if (arduboy.justPressed(B_BUTTON)) {
  arduboy.audio.toggle();
}

And you’ve saved yourself a lot of processing,
some of which is quite expensive.

String is quite an expensive type, so you should aim to avoid it, or ideally to never have to use it.
It’s expensive because it does something called ‘dynamic allocation’,
which can cause problems on devices with very small amounts of memory like the Arduboy.

'Sprites' and explosions

Secondly, you could combine your explosion images into a single image,
then make use of Spritesframe parameter.
E.g. replace explosion_1 through explosion_5 with

//Explosion
unsigned char const explosions[] PROGMEM
{
  // Width, Height
  8, 8,

  // Frame 0
  0xc3, 0xc3, 0x00, 0x00, 0x00, 0x00, 0xc3, 0xc3,

  // Frame 1
  0xc3, 0xc3, 0xc1, 0x0c, 0x0c, 0x00, 0xc3, 0xc3,

  // Frame 2
  0xc3, 0xc3, 0x4c, 0x0c, 0x30, 0x30, 0xc3, 0xc3,

  // Frame 3
  0xf3, 0xf3, 0xcc, 0xcc, 0x33, 0x33, 0xcf, 0xcf,

  // Frame 4
  0xff, 0xff, 0xfc, 0xfc, 0x33, 0x33, 0xff, 0xff,
};

Then you can modify draw_explosion to be:

void draw_explosion()
{
  sound.tones(explosion_sound);
  const auto x = (Zones[bullet_zone].area[bullet_area].x - 4)
  const auto y = (Zones[bullet_zone].area[bullet_area].y - 4);

  for(uint8_t frame = 0; frame < 5; ++frame)
  {
    Sprites::drawOverwrite(x, y, explosions, 0);
    arduboy.display();
    delay(20);
  }
}
Zones setup

Thirdly, something I’ll mention but I won’t demonstrate just yet (hopefully I’ll demonstrate later or tomorrow if I can find some time).
In your setUpZones function, instead of having a long list of assignments,
what you ought to be able to do is to store the default arrangement of Zones in progmem and then use memcpy_P to copy that into your zones array.
Doing a full copy from progmem into RAM should work out cheaper than having lots of individual assignments.
(That’s not always true, but it’s usually true.)

Scoped enums

Fourthly, you should prefer to use an enum class (‘scoped enum’) instead of an enum (‘plain enum’) because enum classes are ‘typesafe’ and won’t cause name clashes.
(You can read more about that here,
but don’t follow their example of using ‘all caps’ for enumerators.)
In your case this would mean doing something like:

enum class GameState
{
  Title,
  Gameplay,
  Lose,
  Pause,
};

GameState gameState;

And then in your state switch your cases would be more like case GameState::Title:, case GameState::Gameplay et cetera.

The problem with delay

Finally, ideally you should avoid making your game rely on delay because doing so gives the game an unsteady framerate, blocks input updating and can cause other problems,
but designing a game without delay can be difficult and changing your game to avoid delay would take quite a lot of effort,
so I won’t discuss that now, instead I’ll say that for your next game you should try to avoid using delay to see if you can manage it.

If you can’t then don’t worry too much,
but avoiding delay is what the vast majority of games do because of how a typical game loop works.

There’s a nice article about game loops here,
and a companion article about having an update function here.
It’s mainly aimed at general computer game development,
so some things either don’t apply to Arduboy or have different nuances,
but most of it is applicable so it’s worth reading if you have time.

I can think of a few more things,
or at least some other places you could apply this advice to,
but I won’t discuss any more suggestions at the moment because I don’t want to bombard you with too much information.
That should give you more than enough to contemplate for the moment.

Feel free to ask questions if you have any.

Thanks @Pharap that really helps me. It’s all a learning curve, but I’m really enjoying it.

I did wonder how I could get rid of the String for the sound, so I will implement that, I did find though that when I used just the toggle method it would turn off but not come back on which is why I added the extra line to turn audio.on. But I will revist this, as I knew my approach wasn’t great!
arduboy.audio.toggle();
}
if (sounds==“on”)
arduboy.audio.on();

Your help with the explosions is brilliant!! I couldn’t work out how to make a sprite sheet & convert it, but of course, I can just paste each image into the array, don’t know why I didn’t think of that! That makes the code much neater, thank you.

The Zones setup is not great, my nephew was saying I could do it with maths as they are all triangles, but that’s not my best subject, so I just worked out each co-ordinate manually - very painful!

I’ll change the enums as you suggested. That would be good practice for me to get into, I’m trying to have a template for my new games so I can get started quickly. The original one I had came from your intro tutorial, and I’m now building this up.

You mentioned about using constexpr for your List Collection, I did read a little about constexpr but still don’t really understand it, so I’ll have another look into it, as I see it is used a lot by people in their arduboy games.

Thank you for your help, I really appreciate it, and will return to trying to improve the code tomorrow.

1 Like

It ought to, toggle literally calls on and off:

(audio_enabled is a bool variable, returned by enabled.)

Note that if you want to save the change to the Arduboy’s EEPROM settings then you also have to call saveOnOff.

Which tool are you using to convert sprites?
There ought to be one or two that can do it.

Failing that you can (as you say) combine several sprites manually.
It is possible to create an actual sprite sheet image and then you only have to edit the sprite dimensions,
but you have to make sure all the sprites in the sheet have a height that is a multiple of 8 (e.g. 8, 16, 32),
and that the sprite sheet is vertical, not horizontal.

That’s true, though triangles aren’t always easy to work with.

There’s absolutely nothing wrong with that solution as long as the values are correct and you only need a fixed number of values.
It can even work out cheaper than making the CPU do the calculations (especially if you’re avoiding floats and trigonometry).

Though I can perhaps think of a slightly better way to implement it than what you currently have.
I.e. using a progmem array rather than a switch statement,
and using sprite frames instead of separate sprites.

A rule of thumb: if you have a switch where the cases are more or less the same thing, but using slightly different values,
it’s often cheaper to stuff the values into an array and index the array.
Especially if every case just consists of calling the same function(s).
And on the Arduboy, putting that array in progmem and reading the information out of progmem saves RAM.

Sometimes the compiler can actually beat you to the punch,
but sometimes it simply can’t, and either way you usually end up with simpler source code.

Which intro tutorial? (I don’t remember writing one.)

Hopefully that link I provided will explain why they’re enum class is better anyway.
Type safety is a big deal, perhaps not as much in games,
but in some programs the right type magic can make all the difference.

constexpr is basically a way of telling the compiler “you should be able to figure this out at compile time, and if you can then please try to do so”.

constexpr variables are the best way to create named constants (which are the ideal way to combat ‘magic numbers’ - unnamed numeric constants that to an outsider appear to have been plucked from thin air).
They’re also guaranteed to be evaluated at compile time.

constexpr functions are functions that can be calculated at compile time, and thus can be used to initialise constexpr variables.
However, they’re not guaranteed to be evaluated at compile time.
In some cases they actually can’t be evaluated at compile time (e.g. if you pass a non-constexpr variable as an argument).

As an example, you could have:

constexpr int square(int value)
{
  return (value * value);
}

constexpr int someImportantConstant = (square(5) + square(8));

And you’d be guaranteed that the value of someImportantConstant was evaluated at compile time,
so the resulting machine code would be as if you’d just written constexpr int someImportantConstant = 89;,
but you don’t loose the important context of how the value is calculated.

I used this sort of thing in Minesweeper for calculating layout values. (E.g. here.)

However, in most cases on something like a collection class,
making the functions constexpr won’t actually be useful because it’s rare to have a constexpr collection, but it’s better to be safe than sorry.

Thanks @Pharap
I’ve implemented those changes, uploaded to GitHub and it’s also had the benefit of reducing the memory usage :smiley:
The audio is working correctly, I think before I was setting it in two places (Setup & Title) which was causing the issue, so your approach is much cleaner & saved quite a number of lines of code.
Your explanation of constexpr makes sense, I’ll have a play around with them.
Next thing I want to do is put the high score into EEPROM, so I’ll start investigating that.

2 Likes

That tends to be the case when simplifying code.

Getting rid of String is probably a big part of that because String has a lot of large functions (managing dynamic memory is a complex task).

But there’s also the benefit of improved readability.

For example, previously your gamestate definition was all squashed into a single line:

enum gamestates {title,gameplay,lose,pause} gamestate;

Whereas now it’s easier to see what the different values (‘enumerators’) are and there’s a clear separation between where the GameState enum is defined and where the gameState variable is defined:

enum class GameState : uint8_t
{
  Title,
  Gameplay,
  Lose,
  Pause,
};
GameState gameState;

In fairness it’s not really my approach, it’s how the audio library is designed to be used so really the credit goes to the library designer.

Unfortunately there isn’t a tutorial explaining that’s how you’re supposed to use it.

One thing I will mention - all the audio functions are actually static, so you can actually use Arduboy2Audio:: instead of arduboy.audio., much in the same way you can use Sprites:: instead of having a sprites object and doing sprites..
Hardly anyone ever does though (for reasons I cannot fathom).

I’ve seen your current attempt.
You have a slight bug because EEPROM.update only updates a single byte.
You’d need to call EEPROM.put instead.

However, you could simplify your code somewhat.
Something like this should be enough for what you’re currently doing:

//High score EEPROM save Methods
constexpr uint16_t high_score_address = 256;

void save_high_score()
{
  EEPROM.put(high_score_address, high_score);
}

void load_high_score()
{
  EEPROM.get(high_score_address, high_score);
}

If you want to save more information, that’s fairly simple.
If you want to store a high score table and update the high scores properly,
that’s more complicated because you have to compare scores, but it’s still doable.
If you want I can show you how to do either of those,
or you could have a go yourself first as a bit of a challenge.
(After all, half the fun of programming is the problem solving aspect.)

However, eventually you’ll encounter the issue of how to know whether the information in EEPROM is actually a high score from your game or random data from someone else’s game.

There's at least two solutions that I know of to that problem
  • Write a multi-byte ‘game id’ to your chosen save address to indicate that you were the last game to write there.
    • Simple, but still quite fragile - it’s very easy for another game to overwrite the saves and leave the ‘game id’ intact
  • Use a hash function to hash your game’s data and store the hash code
    • Will detect almost any kind of corruption to your game’s data, but quite an advanced solution (personally I think the theory is actually more difficult than the implementation)

I don’t want to suddenly throw new suggestions at you,
but I’ll mention a few things just because I’ve spotted them and they should be fairly quick and easy to deal with…

MIDDLE

Firstly I’d suggest changing MIDDLE to middle.
Conventionally ‘all caps’ is reserved for macros (#defines) because it’s important to not have macro names clashing with actual names within the code.
(Macros aren’t actually part of the code as such - they don’t go to the compiler, they’re handled by the preprocessor.)

If middle never changes, you might also want to mark it constexpr.

State update functions and a smaller loop function

My next suggestion would be to make your loop function smaller by moving all the individual states to separate functions,
so you end up with something like:

switch(gameState)
{
  case GameState::Title:
    updateTitleState();
    break;
  case GameState::Gameplay:
    updateGameplayState();
    break;
  case GameState::Lose:
    updateLoseState();
    break;
  case GameState::Pause:
    updatePauseState();
    break;
}

Don’t worry about memory usage - there shouldn’t be any extra cost because the compiler should be smart enough to realise that it can ‘inline’ the functions because they’re only used once.

The benefit of doing it this way is that instead of dealing with one really large function (which can cause issues, not just for people reading or maintaining the code, but for the compiler as well - the compiler struggles to optimise functions once they get past a certain size because it start struggling to keep track of all the context) you can focus on each state individually because each state is a separate function.

(When you’re more comfortable with classes you could even have one class per state, which is what people tend to do.)

Using releases for `.hex` files

My final suggestion for now - instead of uploading the .hex files as part of the code, I’d suggest using GitHub’s releases feature instead.

Managing releases in a repository - GitHub Docs

It makes it easier for people who only want to download the .hex file because they don’t have to download the code with it, it means you don’t end up having an out of date .hex file mixed in with the code and it means you don’t end up with a load of ‘update .hex file’ commits interleaved with the important commits (the ones about code changes).

It also makes it possible to provide a .hex compiled for the old Arduboy devkit if you want, though not many people have those anymore.

Thanks Pharap,
When testing the game earlier I realised that the EEPROM.update was doing something strange where I’d seemingly get random values stored, I changed it to a .put and that has solved it, but can save some more lines doing what you’ve suggested, so thank you for that. I couldn’t understand what was happening there, but now that makes sense. I was going to PM you and ask for help with that as couldn’t understand what was going wrong!

Yep, MIDDLE is always the same, so I’ll change it to a constexpr (that’s why I put it in CAPS, as used to doing that in C# to indicate a Const). Gives me a chance to try out constexpr as well.

I haven’t used GitHub releases, so that is a good pointer, as I kept going back and doing another commit just for the hex file. So will check that out.

I guess with the Arduboy2Audio if that’s static, I just invoke the methods directly then? like Arduboy2Audio::on instead of arduboy.audio.on ?? is this more conservative on RAM? Just unsure of what this means. Can I play the sounds using those methods as well like this?

1 Like

The values probably weren’t random, they were probably either just the low byte of your 2 byte score value.

(If you don’t already know how bytes and binary work I can throw you some more reading material upon request.)

Fortunately I’ve saved you the trouble in this case,
but if you do encounter something like that again feel free to PM me.

You can also just ask openly on the forum.
I’m not the only one capable of answering programming questions,
I just have a habit of getting there first because I enjoy doing it. :P

Personally I think asking in the open is slightly better for programming problems/questions because it means the information is then available for other users who might stumble upon the conversation when doing a search, which in turn means that the solution to your problem might one day be the solution to someone else’s problem.

It’s entirely up to you though - I appreciate that some people prefer one-to-one discussions for various reasons (e.g. more freedom to go off on a tangent and ask irrelevant questions or to discuss one’s personal life, worries about appearing naive in public).

Odd.

Usually C# convention is to treat consts the same as regular variables - Pascal case for public members, camel case for locals and private members.
Using all caps for constants is actually a Java convention.

It’s not unheard of in C#, I think there’s a few odd style guides that recommend it, but personally I’d say avoid doing it for two reasons:

  • It’s not what the C# standard library does
  • Trying to encode that kind of information in a variable name is generally a bad idea, especially if one day your constant is no longer constant or vice versa

You still have to upload the .hex file of course,
but keeping it separate from the source code makes life easier I find.

Yep, that’s pretty much how it works.

The :: is the ‘scope resolution operator’,
and the . is the ‘member access operator’.
The former is applied to a namespace or type name to resolve a name within the namespace/type’s scope,
the latter is applied to an object to refer to a member of that object.
They’re conceptually similar, but used for different things and have different nuances.

Interestingly no, it makes no difference.

static functions don’t actually need an object, so when you write (for example) arduboy.audio.toggle(), the compiler actually only uses the arduboy.audio. part to identify which toggle function you’re referring to, it doesn’t actually use the arduboy or arduboy.audio objects in any way.

The real implication is this: your code doesn’t need to be able to ‘see’ the arduboy object to be able to toggle the audio on and off.

When you have your code separated into lots of small files (as, ideally, you should do),
acessing global variables can become more difficult (which isn’t really a bad thing),
so being able to just use a static function can come in handy.

It depends which library you’re using for playing sound.
Arduboy2Audio iself doesn’t actually produce any sound,
it only enables and disables the speaker.
I expect actual sound libraries (with the exception of Arduboy Beep) don’t use static methods,
but I couldn’t say for definite without checking.

The Const naming I may have got from Java then. I’m an automated software tester & have to use tools like Selenium, but in Java, C#, Ruby & JavaScript, so get very confused sometimes!! But coding as a tester is very different from dev, we only touch the surface of the languages really.

I’ve added the changes & will commit to github and use the realeases page - much neater way, thanks.

The other thing I was going to change was the way you go through levels. At the mo’ I have:

if (score==300){
            level=2;
            alien_speed_level = 60;
            arduboy.setFrameRate(65);
            level_up();
          }
          if (score==500){
            level=3;
            alien_speed_level = 50;
            arduboy.setFrameRate(65);
            level_up();
          }
          if (score==800){
            level=4;
            alien_speed_level = 40;
            arduboy.setFrameRate(70);
            level_up();
          }
          if (score==1200){
            level=5;
            alien_speed_level = 30;
            arduboy.setFrameRate(80);
            level_up();
          }

But though that using an array (although more memory expensive) was tidier & lets me easily add/change values, so have done this:

int levels[]{
            //score, level, alienSpeed, frameRate//
            300, 2, 60, 65, //level 2
            500, 3, 50, 65, //level 3
            800, 4, 40, 70, //level 4
            1200,5, 30, 80 //level 5
          };

for (int i=0;i<=16;i+=4){
            if (score==levels[i]){
              level = levels[i+1];
              alien_speed_level = levels[i+2];
              arduboy.setFrameRate(levels[i+3]);
              level_up();
            }
          }

What do you think?

1 Like

It might also just be that whichever company you work for has a set code style and that code style is influenced by Java.
Particularly if they mainly hire Java programmers or their style guide was developed by Java programmers.

Languages tend to have very different styles and characters,
but in a corporate environment it’s probably not uncommon to have one style guide for everything so workers have less to learn or less to think about.

To be honest we could really do with a tutorial discussing this kind of thing,
but writing tutorials is boring and a lot of effort.

I expected there was a way to tidy this up,
but I didn’t try to suggest anything at the time because that would have required reading through and understanding all the if statements and their implications.

This is indeed a big improvement and is a testement to the fact your skills are improving.

There is still quite a bit of room for improvement though.


The details blocks should save people from scrollbar hell.

Introducing structs

Firstly, I’d like to point something out:

//score, level, alienSpeed, frameRate//

The fact you’ve documented what the ‘columns’ mean.
I see people do this quite a lot here, particularly beginners.
This is almost always a big hint that actually what you really want/need is a struct.

In this case, you could do something like this:

struct LevelInfo
{
	int score;
	int level;
	int alien_speed;
	int frame_rate;
};

// This is now constexpr because the data never changes
constexpr LevelInfo levels[]
{
	{ 300, 2, 60, 65, },
	{ 500, 3, 50, 65, },
	{ 800, 4, 40, 70, },
	{ 1200, 5, 80, 80, },
};

// Note how much cleaner this is now
for (size_t index = 0; index < 4; ++index)
{
	if (score == levels[index].score)
	{
		level = levels[index].level;
		alien_speed_level = levels[index].alien_speed;
		arduboy.setFrameRate(levels[index].frame_rate);
		level_up();
	}
}

This is more or less a literal translation of what you were doing,
but using an actual struct instead of loosely related integers.

More suitable integers

Next up, I’d like to point out that currently the struct is using int for all its fields.
As it happens, int is a poor choice for some of these because of two things:

  • int is signed, but none of the data being stored will ever be negative
  • int is 16 bits wide on the Arduboy*, but some of the data being stored could fit in 8 bits

* The size of int varies depending on the target platform.
It’s definition is ‘a signed integer of at least 16 bits’.

Regarding 32-bit int...

On x86 and x64 (i.e. most desktop and laptop computers) it tends to be 32 bits wide,
which has lead some people to erroneously expect it to always be 32 bits.

To add more confusion, on 64-bit systems int tends to still be 32 bits wide,
whilst pointers are naturally 64 bits wide,
so if you ever catch someone trying to cast a pointer to an int please scream at them for me and tell them to use uintptr_t instead.

C#, Java and other languages decided that their int shall always be exactly 32-bits,
which has also contributed to the confusion.

So let’s select some more suitable types:

  • score has to be larger than 8 bits because even the lowest value (300) is larger than the maximum value of an 8-bit unsigned integer (255), so uint16_t (an unsigned 16-bit integer) is the most appropriate type.
  • level, alien_speed and frame_rate are all less than 255, so they can happily fit inside a uint8_t (an unsigned 8-bit integer)

That gives us:

struct LevelInfo
{
	uint16_t score;
	uint8_t level;
	uint8_t alien_speed;
	uint8_t frame_rate;
};

Which should of course save yet more progmem, not just because the struct is now smaller,
but because the Arduboy’s CPU is an 8-bit CPU so it needs fewer instructions to manipulate 8-bit data than it does to manipulate 16-bit data.

I’d like to take this opportunity to point that you’re using int8_t in many places where uint8_t would be more suitable.
When selecting integer types always ask yourself two questions:

  • What is the expected maximum value of my data?
  • Will my data ever be negative?

This is less of an issue on a desktop computer where memory is plentiful,
but on the Arduboy picking the smallest datatype possible is very important.


Now, following those things I was about to demonstrate how to put your array in progmem so you don’t waste any RAM,
but as I was doing that it suddenly occurred to me that looping through all this data is inefficient and unnecessary.
(If you’ve read to the end before reading the details blocks, go back and read those, they’re still important.)

Consider that:

  • You know that you start on level 1
  • You know that level 1 will be followed by level 2
  • You know that level 2 will be followed by level 3
  • You know that level 3 will be followed by level 4

And so on, and so forth.

But currently the code is looking at all the levels,
as if there’s some chance you might somehow go down a level by loosing points.
(I’m assuming that’s not possible, if I’m wrong then please correct me.)

Considering the afforementioned facts,
surely it makes more sense to only consider the next level in the sequence?

Naturally I’ve thought of a solution to this problem, but rather than just giving you my answer,
I thought I’d give you a chance to ponder the problem and come up with your own solution.

Here's a few hints that might help you
  • Don’t scrap the array, it’s still useful
  • Don’t be afraid to alter the struct if you think something might be superfluous
  • Remember that arrays are 0-indexed
  • Remember that 1 - 1 is 0 and that 0 + 1 is 1

(If you don’t want to have a go yourself or you just want me to reveal my answer for whatever reason then feel free to ask.)


Lastly I’d like to mention that changing the frame rate mid-game is generally a bad idea and you should avoid doing that in future games.
There are better ways to achieve variable speeds.

Hey @Pharap,
Brilliant! Thank you - using a Struct makes it so much more readable - love it!
So I’ve done that, and my solution for the only checking last level onwards is this:

for (size_t index = (level-1)*1; index < 5; ++index)
          {
            if (score == levels[index].score)
            {
              level = levels[index].level;
              alien_speed_level = levels[index].alien_speed;
              arduboy.setFrameRate(levels[index].frame_rate);
              level_up();
            }
          }

So using *(level-1)1, but you may know a better way.

On another Q, I understand using the costexpr for functions - makes sense, in C# I’ve used the => before so things only get executed when called. BUT, I can’t figure it for using just for constants. I see you used:

constexpr LevelInfo levels[]

BUT, when I change the middle Point var to constexpr:

constexpr Point middle(65,32); // middle of the mine shaft

I get an error:
Alien_Mine:26:29: error: call to non-constexpr function ‘Point::Point(int16_t, int16_t)’

So struggling with this, can you help me get my head around this?

Many thanks
Tom

Not exactly what I was expecting.

Before I make any suggestions though, I need to ask a question:
is it actually possible to go up more than one level at a time,
or is it impossible to earn enough points to increase multiple levels in a single update/frame?

If the answer is “no, it’s not possible” then my suggestion is “get rid of the for loop”.

However, I will point out that * 1 is entirely redundant.
Multiplying anything by 1 gives back the exact same thing.
The compiler should actually realise this and choose to ignore your * 1.

(Without going too far into the realm of crazy abstract maths:
1 is actually the ‘identity element’ for multiplication and division,
just as 0 is the ‘identity element’ for addition and subtraction.)

I think I know what you’re talking about,
though it’s one of the newer features I’ve never used.
(Unlike C++ where I try to be up to date with the latest standards, I don’t know much about the C# features added after C# 4.)

If you’re thinking of:

public string Name => First + " " + Last;

That’s just syntactic sugar for writing a get-only property.

It’s comparable to const in the sense that attempting to write to a get-only property will give you a compiler error, but functionally it’s very different.

For example, using the Name example from above,
attempting to use the property Name as a value will give you different results if the values of First or Last change because a property actually consists of one or two functions (a ‘getter’ and a ‘setter’).
That means that if you write something like "Hello " + Name the compiler actually translates it to something like "Hello " + __get_Name()

In C++ on the other hand, if you assign to a const variable you’re actually doing a one-time-only copy, copying a value into that variable, and then that value never changes.
C++ doesn’t have a concept of properties because it was invented before getters and setters became popular,
so people tend to just write getter functions directly.

Ah…

I know exactly what’s causing this, and it’s mostly my fault.
Basically Point has a special kind of function called a constructor that’s used to create the point,
and because the constructor isn’t marked constexpr the Point can’t be constepxr

I’ve made a PR to rectify this misjudgement,
and @MLXXXp has very swiftly reviewed and merged the fix,
but it won’t be available via the Arduino library manager until the latest version is released,
which might not be for a while.

If you want to use it now you can download the Arduboy2 source code and manually replace your local copy of Arduboy2 with the lastest source code,
but anyone else trying to compile your code from source would need to be made aware of that.

For now you’ll have to make do with const.

Yep - I was being dense!

No need for a loop (and multiplying x 1 is really dumb!!)

All I need is:

if (score == levels[level-1].score)
            {
              alien_speed_level = levels[level-1].alien_speed;
              arduboy.setFrameRate(levels[level-1].frame_rate);
              level = levels[level-1].level;
              level_up();
            }

And no loop - sorted!
Thanks Pharap

1 Like

That’s more like it.

I’ll now present my solution, which involves a number of extra changes.

// Globals
uint8_t level_index = 0;
uint16_t score = 0;
uint8_t alien_speed_level = 80;

// The level is no longer part of the struct
// because the level number can be inferred from the array index
struct LevelInfo
{
	uint16_t target_score;
	uint8_t alien_speed;
	uint8_t frame_rate;
};

// Level 0 (1) is now included to permit 'set_level(0)',
// as explained later
constexpr LevelInfo levels[]
{
	{ 0, 80, 60 },
	{ 300, 60, 65, },
	{ 500, 50, 65, },
	{ 800, 40, 70, },
	{ 1200, 80, 80, },
};

// There's a better way to calculate this.
// See: https://github.com/Pharap/SizeDemo
constexpr size_t level_count = (sizeof(levels) / sizeof(levels[0]));
constexpr uint8_t max_level = (level_count - 1);

// Introducing this function allows one to 'set_level(0)' when resetting the game
void set_level(uint8_t level_index)
{
	// This is a 'const reference'
	const LevelInfo & next_level = levels[level_index];
	
	alien_speed_level = next_level.alien_speed;
	arduboy.setFrameRate(next_level.frame_rate);
}

void attempt_level_up()
{
	// Avoid trying level up when it's not possible to level up further
	if(level_index >= max_level)
		return;

	// The potential bug of reading past the end of the array
	// is avoided thanks to the earlier check
	const LevelInfo & next_level = levels[level_index + 1];
	
	// If the score is sufficient to level up
	if(score >= nextLevel.target_score)
	{
		// Increase the level, set the level parameters and level up
		++level_index;
		set_level(level_index);
		level_up();
	}
}

I’d like to point out that my solution changes ‘level 1’ to be represented internally as ‘level 0’,
so you’d also have to change the functions that print the level so that they add a 1.

I don’t think there’s any other caveats, but I haven’t actually done any testing,
so it’s entirely possible that I’ve made a mistake somewhere.

Feel free to ask questions if there’s anything you don’t quite comprehend,
and don’t be afraid to question my decisions.


This version still doesn’t store the array in progmem.
I’ll discuss that some time tommorrow (or whenever I next have chance).

Unless there’s something more pressing you’d like help/advice with,
or you’re not that bothered about storing the level data in progmem.

Hi @Pharap, Sorry, been a bit busy last few days.
Nice solution, I understood most of it, apart from the bit below with the ampersand:

That’s new to me.
It would be interesting to see how you store things like an array in progmem, but you may have to keep any examples very simple for me :wink:

All the best

1 Like

The & denotes a reference.

Writing:

LevelInfo next_level = levels[level_index];

Would mean that next_level stores a (mutable) copy of levels[level_index].

Writing:

const LevelInfo & next_level = levels[level_index];

Means that next_level is a (read-only) reference to levels[level_index],
henceforth using next_level behaves as if you were writing levels[level_index] instead.

The typical example for demonstrating objects vs references is:

void set5Value(int value)
{
	// value is a copy of the argument,
	// so this won't affect the original object
	value = 5;
}

void set5Reference(int & value)
{
	// value is a reference to the argument,
	// so this will affect the original object
	value = 5;
}

void test()
{
	int a = 10;
	// a is 10
	set5Value(a);
	// a is still 10
	set5Reference(a);
	// a is now 5
}

const references are mainly used:

  • To avoid copying a large object
  • When having a reference to an object would make the code more readable

In this case (next_level) both of those apply to an extent.

Non-const references are mainly used:

  • For ‘out’ parameters, an alternative to return values
    • This should be done sparingly, out parameters vary in efficiency and can be more surprising than return values
  • For returning a reference, which is sometimes desirable

I’ll cover that when I next have chance and when you’re happy with the references idea.