Tile Collisions?

I’ll definitely be taking up your offer on that.

I’m thinking to continue with this tutorial it would make sense to figure out collision with only certain tiles, like the rock one, and stop movement if the player rectangle is colliding with it.

The Arduboy library has the collide function which can compare two Rectangles. So I’m thinking it would make sense to designate the black rectangle in the middle of the screen as a Rect and somehow create every tile as a Rect as it’s drawn to the screen, so essentially I can check every time the player Rect moves over the tiles and stop increasing `mapx` or `mapy` if the collision happens with a certain tile based on its #define value. Am I even close to the right track?

You really only need to check tiles that you are about to move into - if it is a solid tile, you simply prevent that movement.

1 Like

I see, kind of.

Inside the tile rendering loop I’m thinking something like `if (arduboy.collide({PLAYER_X_OFFSET, PLAYER_Y_OFFSET, PLAYER_SIZE, PLAYER_SIZE}, {???, ???, TILE_SIZE, TILE_SIZE})) {...}`. Finding the `x` and `y` of a certain type of tile is the real question. I know they all have their own `#define` number and place in the `world` array, but aside from that I’m not sure.

In addition to what @filmote said, you don’t want to mix your rendering code with your collision code.

For tile-based games you don’t actually need full rectangle collision,
you can cheat somewhat thanks to the fact that all tiles have the same dimensions and are directly next to each other.

Checking whether a point is inside a tile is easy:

``````// Note: there's a much better way to represent tiles than 'int'
bool isSolid(int tileIndex)
{
// return true if the tile is solid, return false otherwise
}

bool isPointInTile(int x, int y)
{
return isSolid(world[y / tileHeight][x / tileWidth])
}
``````

So you can easily check the tiles a player is about to move into and determine whether or not they’re solid.
If they are solid then you either prevent the movement entirely or snap the player to the first position that isn’t colliding with the tile(s).

1 Like

I started this and maybe am on the right track. It seems to be working for some of the solid tiles (stones and trees) and just bypassing others. I’m thinking it’s being caused by the x and y coordinates passed to the `isPointInTile` function being off, so it isn’t detecting the correct tiles.

I added this to the functions:

``````bool isSolid(int tileIndex) {
switch (tileIndex) {
case GRASS:
return false;
break;

case WATER:
return false;
break;

case TREES:
return true;
break;

case STONE:
return true;
break;
}
}

bool isPointInTile(int x, int y) {
return isSolid(world[0 - y / TILE_SIZE][0 - x / TILE_SIZE]);
}
``````

I added the `0 -` to the array indexes in that function so the numbers wouldn’t be negative, but is that incorrect?

And in the `playerInput` function I’m checking if it’s returning false because if it’s returning true, the tile is solid and the movement should stop (just the right button being pressed as an example:

``````if (PLAYER_X_OFFSET + PLAYER_SIZE < mapx + TILE_SIZE * WORLD_WIDTH && isPointInTile(mapx, mapy) == false) {
mapx -= 1;
}
``````

I’ve tried adding all sorts of things to the arguments in the `isPointInTile` function, like the player size and player offset, in an attempt to correct the coordinates and make sure the player is detecting the right tiles as it moves, but weird things continue happening.

I’ve migrated this now because it’s no longer really ‘on topic’ for the thread it was on.

Firstly, `0 - x` is equivalent to `-x`, so you could have just written `-(x / TILE_SIZE)`.

Secondly, either option will cause positive numbers to be negative,
which possibly isn’t what you want to be doing.

You’re better off writing `!isPointInTile(mapx, mapy)`.
`!` is the ‘not’ operator, it inverts a `bool`.
I.e. `!true` evaluates to `false` and `!false` evaluates to `true`.

Anyway, now the style stuff is out of the way, onto solving the problem.

Part of the problem is that `mapx` and `mapy` are actually the negative offset of the map rather than the player position.
(Personally I always tend to store the player’s position instead.)

The other part of the problem is that `PLAYER_X_OFFSET` isn’t in ‘world coordinates’, it’s in ‘screen coordinates’, so you’re mixing coordinate spaces.

Instead you want to be looking at something like…

``````int playerX = -mapx;
int playerY = -mapy;

int playerLeft = (playerX);
if ((playerLeft > 0) && !isPointInTile(playerLeft, playerY))
{
mapx += 1;
}

int playerRight = (playerX + PLAYER_SIZE);
if ((playerRight < (WORLD_WIDTH * tileSize)) && !isPointInTile(playerRight, playerY))
{
mapx -= 1;
}
``````

Completely untested, but theoretically it should work.
Be aware that this might break for diagonal movement,
and it will break if `1` is increased to something larger.
(When I say ‘break’ I mean the player won’t be able to move right up against the solid tile, they’ll stop a few pixels short of the solid tile, which will probably look quite odd.)

In hindsight `isPointInTile` is a terrible name, I must have been distracted when I wrote that.
`isTileSolid` would probably be a better name (or `isTileSolidAt`).

1 Like

Dang it, I usually pay attention to at least using the not operator instead of `== false`. My bad.

It’s still doing some weird stuff, like sometimes stopping movement a tile or two before the solid one or stopping halfway through a tile, but I want to keep messing around and figure it out. Thanks for putting me on the right track of following the player position instead of the map, I was getting hung up on that but wanted to stick as close as I could with the tutorial.

1 Like

Don’t worry, you’ll break the habit with enough practice.

Personally I’m against teaching the `== false` approach in the first place.
It’s a lot harder to break the habits you learn early on than it is to learn good habits in the first place.

One good way to break the habit is to look at how illogical it is.
In a normal conversation you might say something like “if the shop has eggs, buy 6 of them”, you don’t say “if the shop has eggs is true, buy 6 of them”, so when writing code you should say `if (shop.has(Item::Eggs)) { shop.buy(Item::Eggs, 6)); }`, not `if (shop.has(Item::Eggs) == true) { shop.buy(Item::Eggs, 6)); }`.

In an ideal world, you should be able to read code as if it were English (or some other natural language).
That’s also one of the reasons why picking good variable names is important.

Without being able to see the code I can only guess at the problem.

If in doubt, print the results of any calculations.
You’ll probably spot which number is wrong and then be able to investigate further.

Don’t worry about sticking to the tutorial too much.

The tutorial isn’t actually a particularly good example because it simplifies quite a lot of things in an attempt to be more accessible and/or do less explaining, and does a handful of things that are just bad practice for C++.

For example it uses `if (x == true)` and `if (x == false)` instead of `if (x)` and `if(!x)`.
The former will earn you nothing but frowns from experienced programmers.
Pick any random piece of code off GitHub and there’s probably a 99% chance it’ll be using `if (x)` and `if (!x)`.

It uses macros (`#define`) instead of proper constants (e.g. `constexpr int worldWidth = 20;`.

It uses macros (`#define`) instead of enum classes.
E.g. `#define GAME_TITLE 0` et cetera instead of:

``````enum class GameState
{
Title,
Play,
GameOver,
Highscores
};
``````

Also `#define GRASS 0` et cetera instead of:

``````enum class TileType
{
Grass,
Water,
Tree,
Stone,
};
``````

And it doesn’t wrap any of its strings in `F`, which means they waste RAM instead of being stored completely in progmem.

Also the `clear` and `display` in `setup` are redundant because `loop` clears the screen anyway and `begin` already calls `display` at the end.

And lastly, something not quite as serious, it uses `int` for indexing instead of `size_t`.
That won’t break on the Arduboy, but could be a problem on some systems.
(It can even cause security bugs.)

Awesome, thanks for the C++ best practices info. After I fix up this collision thing I plan to go through and correct all that stuff, for practice.

Here’s the progress I made on this. I thought last night after I added the commented out lines in each button press if statement that I had it figured out, since it was accounting for all corners of the player rectangle. Then I tried today after being more than 5% awake and realized that it just causes the player to get stuck all over the place since it’s checking all corners all the time.

``````void playerInput() {
int playerX = -mapx + PLAYER_X_OFFSET;
int playerY = -mapy + PLAYER_Y_OFFSET;

int playerLeft = (playerX);
int playerRight = (playerX + PLAYER_SIZE);
int playerTop = (playerY);
int playerBottom = (playerY + PLAYER_SIZE);

if (arduboy.pressed(UP_BUTTON)) {
if (
playerTop > 0
&& !isPointInTile(playerLeft, playerTop)
// && !isPointInTile(playerRight, playerTop)
) {
mapy += 1;
}
}
if (arduboy.pressed(DOWN_BUTTON)) {
if (
(playerBottom < (WORLD_HEIGHT * TILE_SIZE))
&& !isPointInTile(playerLeft, playerBottom)
// && !isPointInTile(playerRight, playerBottom)
) {
mapy -= 1;
}
}
if (arduboy.pressed(LEFT_BUTTON)) {
if (
playerLeft > 0
&& !isPointInTile(playerLeft, playerTop)
// && !isPointInTile(playerLeft, playerBottom)
) {
mapx += 1;
}
}
if (arduboy.pressed(RIGHT_BUTTON)) {
if (
(playerRight < (WORLD_WIDTH * TILE_SIZE))
&& !isPointInTile(playerRight, playerTop)
// && !isPointInTile(playerRight, playerBottom)
) {
mapx -= 1;
}
}

arduboy.fillRect(0, 0, 48, 8, BLACK);
arduboy.setCursor(0, 0);

// PLayer coordinates
arduboy.print(playerX);
arduboy.print(',');
arduboy.print(playerTop);
arduboy.print('\n');

arduboy.print(world[playerBottom / 16][playerRight / 16]);

}
``````

It sort of works, but if a certain corner of the player collides with the solid tile by one pixel off it goes through it. I was printing out the tile index for each corner of the player to test it out, but now I’m thinking it’s something slightly off with the calculation of where the player rectangle is on the screen.

1 Like

The bit about `if` is actually more general rather than C++ specific.

And the thing about `F` only applies to Arduino, and specifically is only important on AVR chips.
It’s needed because the machine code instruction for reading progmem is a different instruction to the one that reads RAM (other architectures like ARM don’t tend to have this distinction - their memory reading instruction will read from ROM or RAM).
I could go into the mechanics of what `F` actually does if you want to know more, but it’s a somewhat intermediate technique and you don’t really need to know the details to be able to use it properly.

If you want to know more about C++-specific good practice then isocpp’s C++ Core Guidelines is probably the closest thing there is to a ‘standard’ document about them.

I have a lot more programming and C++ resources hosted on my Resource Collection repo if you want a look.
It’s where I dump links to webpages that I find particularly interesting or useful.

What is `PLAYER_X_OFFSET` exactly?
Are you sure you aren’t mixing screen coordinates and world coordinates?

I’m fairly sure that `playerX` should just be `-mapx`.

I think these commented out lines are on the right track,
but what you’re missing is that they’re only checking collisions against the player’s current position.
What you want to be checking is the position that the player is trying to move to,
which admittedly I left out in the example I gave earlier.

So what you should be doing is something like:

``````if (arduboy.pressed(UP_BUTTON) && (playerTop > 0))
{
int newPlayerTop = (playerTop - 1);
if (!isPointInTile(playerLeft, newPlayerTop) && !isPointInTile(playerRight, newPlayerTop))
{
mapy += 1;
}
}
``````

(Completely untested, as my code often is.)

By the way, do you have a GitHub account, and if not have you considered getting one?

Hosting your full code on GitHub would make it a lot easier to keep track of the changes and for other people to help out with it.

As far as I can understand it `PLAYER_X_OFFSET` (and `PLAYER_Y_OFFSET`) are the player’s coordinates on the screen. They’re constants because basically the player rectangle just sits in the middle of the screen all the time.

I do have a GitHub, here’s the code for this: https://github.com/ajsaucier/DinoSmasher-tutorial/blob/master/DinoSmasher.ino

I’ve just been sloppily using the Arduboy emulator for all this so the alignments and tabs look all crazy when I pasted it in the file on GitHub. Like I said, I’m planning to go back in and restructure all this to follow some better C++ and Arduino specific stuff you gave me.

Then yes, your code is mixing world coordinates and screen coordinates when it should only be dealing in world coordinates.

Screen coordinates (probably) only make sense during rendering.

Ah, that makes matters a bit easier.

I won’t be able to look into it much now though, it’s quite late.

A small tip: if a `case` of a `switch` statement is just a `return` then you don’t need a `break` after it because the `return` implicitly breaks out of the `switch`.
The compiler might actually tell you about this if you’ve got warnings set to ‘all’, since technically the `break`s are ‘dead code’ that can’t be reaches.
(There’s also another technique that could be used here that in some cases is smaller than a `switch`, but I won’t get into that now.)

Oh, and you don’t need to use `setCursor(0, 0)` when you’re calling `clear()` in `loop()`.
`clear()` already calls `setCursor(0, 0)` (or at least does the equivalent).

Note...

I recommend avoiding chaining assignments like that Arduboy2 code does.

I’ve always considered it bad because I follow a personal “one assignment per line” rule because I believe doing so improves code clarity, but I just looked it up to be sure and apparently I’m not the only one who thinks it’s bad.

Also the actual behaviour can be surprising because not all languages implement it in the same way.
The example linked to shows that C# behaves differently to C++.

Part of that’s because the code I post almost always uses tabs (and always uses Allman style braces), so anything copied from one of my comments will have tabs.

The Arduino IDE on the other hand uses two spaces instead of proper tabs (though you can change that by editing a settings file hidden in `%APPDATA%`).

I tend to only use the IDE for the occasional bit of compiling, most of the time I write my Arduboy code either in VSCode or Notepad++.

Getting the collisions sorted is probably the more important thing at the moment to be fair.
After that it’s up to you whether you stop to do a bit of redecorating or add more features.
(Though I’ll admit, I’m in favour of redecorating, if for no reason other than getting rid of the macros.)

Here’s a random fact I just learned. The following code is valid C++, but not valid C:

``````int a = 0;
int b = 20;

(a = 10) = b;
``````

Another interesting incompatibility to add to my ever-growing collection. `:P`

Interesting. Apparently I’ve been using the BSD KNF method without really thinking about what its technical name is.

Please don’t rush around for this stuff, I already appreciate how much time you take with helping.

I’m really only doing this to get my feet wet and maybe figure some stuff out that I’ll need for my own game at some point (probably very far) down the line. I know tile collision will be a super important thing for basically any game type so I’m trying to nail it down now before I move onto anything else.

Not exactly BSD KNF because BSD KNF uses 8 spaces for indentation (which is really unusual), but apart from that detail

Out of the K&R variants, Stroustrup is probably the most popular form used in C++ purely because it’s what Stroustrup uses for all the examples in “The C++ Programming Language”.
(In case you missed it, Bjarne Stroustrup created C++.)

It’s no problem.
You’re not the first person I’ve helped, you won’t be the last,
and I haven’t actually spent that much time.

Besides which, when I help people I almost always end up learning something myself, or at least refining some of my explanations.

Not quite any game, but most games that want some kind of world to move around in will use a tile map purely because it’s a good way of storing a level in a compact way.

I looked at the latest copy of your code with the working collisions and I suddenly realised why the `+ PLAYER_X_OFFSET` part is needed.

Without it, `-mapx` and `-mapy` actually correspond to the top left of the screen when rendering rather than the top left of the player rectangle.

I was thinking `PLAYER_X_OFFSET` was the player rectangle’s offset from the centre of the screen,
but it’s actually the player’s offset from the top left corner of the visible portion of the map.
(Or, depending on how you think about things, it’s the top left of the implicit ‘camera’. Usually when I write tile-based games I make the camera an actual object so it’s an explicit concept.)

One last thing.
When I ran the compiler with your code I got the following warning:

``````I:\Documents\Github\Downloads\DinoSmasher\DinoSmasher.ino: In function 'bool isSolid(int)':

}

^
``````

Basically your `isSolid` function doesn’t have a `return` if `tileIndex` isn’t a valid tile.

In normal C++ this would actually be illegal and your code wouldn’t compile, but Arduino uses this stupid `-fpermissive` flag when it compiles, which means that a lot of things that aren’t valid C++ are allowed.

The way to fix it is to either have a `default` case in the `switch` that `return`s a default value, or to put a `return` at the bottom of the function which `return`s a default value.

E.g. something like this:

``````bool isSolid(int tileIndex)
{
switch (tileIndex)
{
case GRASS:
return false;

case WATER:
return false;

case TREES:
return true;

case STONE:
return true;

// Tiles are non-solid by default
default:
return false;
}
}
``````

While I’m at it, I’ll point out that you can also assign multiple `case`s to do the same thing:

``````bool isSolid(int tileIndex)
{
switch (tileIndex)
{
case GRASS:
case WATER:
return false;

case TREES:
case STONE:
return true;

// Tiles are non-solid by default
default:
return false;
}
}
``````

And technically you could just list the solid tiles:

``````bool isSolid(int tileIndex)
{
switch (tileIndex)
{
case TREES:
case STONE:
return true;

// Tiles are non-solid by default
default:
return false;
}
}
``````

But it depends which approach you think is more readable.
Personally I think listing both the solid and non-solid tiles is more useful,
because it definitively states whether a tile is non-solid rather than requiring you to infer that a tile is non-solid by its absence from the list.

Super interesting stuff about assigning multiple switch cases. I had no idea that could be done!

I updated the code on GitHub to have collision working (I had to add or subtract 1 from the player position variables, after testing it and finding things were somehow off by 1 pixel).

I implemented most of the improvements you mentioned in a previous post except for these:

I implemented the function pointer that was shown in issue 3 of the Arduboy magazine to save some space there:

``````const FunctionPointer PROGMEM mainGameLoop[] = {
gameTitle,
gamePlay,
gameOver,
gameHigh
};
``````

Then called this pointer in the `loop()` function to run the right function. I’m still not entirely sure how this works, especially the `pgm_read_word` part, but I’m sure you’ve had to explain this millions of times already.

The interesting thing about using that function is that the macros are still being used as well in the magazine’s lesson. When you recommended using proper constants and enum classes instead of those macros, you mean to totally do away with the `#define` parts and replace them with classes?

Sorry if that question makes no sense.

1 Like

Hrm… it makes sense that you’d need to subtract 1 from `PLAYER_SIZE` when calculating `playerRight` and `playerBottom`,
but needing to add `1` to `playerX` and `playerY` to get `playerLeft` and `playerRight` doesn’t make sense.

Something seems off there.
That implies that either `mapx` and `mapy` or `PLAYER_X_OFFSET` and `PLAYER_Y_OFFSET` are off by one for some reason.

You have to be a bit careful with that technique.

Firstly because it doesn’t always save space in the first place.

More importantly, you have to make sure that the index is valid otherwise you go outside the bounds of the array and end up reading data that isn’t actually a function pointer as if it were a function pointer.
If you try to call an invalid function pointer then almost anything could happen.

On a desktop you’d be able to use `std::array` which has a function that checks the index you’re trying to access is valid an throws an exception if it isn’t.
But the Arduboy doesn’t have access to an implementation of the C++ standard library, just a version of the C standard library.
And even if it did have C++ standard library implementation, exceptions are disabled (as they are on most embedded systems because they need quite a bit of ROM/progmem).

Technically it should actually be `pgm_read_ptr`.

You’d be best off using something like this:

``````// Modern C++11 type alias
using FunctionPointer = void(*)();

// Array of game state function pointers
constexpr size_t gameStateCount = 4;
const FunctionPointer gameStates[gameStateCount] PROGMEM
{
gameTitle,
gamePlay,
gameOver,
gameHigh,
};

// API for calling a game state function safely
void callGameState(size_t index)
{
// Prevent attempts to read outside the array
if(index >= gameStateCount)
return;

// Read the pointer out of the array

// Convert it to the proper type
auto function = static_cast<FunctionPointer>(pointer);

// Call the function
function();
}
``````

If you want to be extra safe, you could replace the `return` with something that prints an error message to the screen and stops execution (e.g. by using `while(true);`).

Hopefully the way I’ve broken the code up a bit will make it easier to make sense of what the code from the magazine was doing.

Also, be aware that `void *` has nothing to do with `void (*)()`,
despite looking vaguely similar.

`void *` is the ‘opaque pointer’ type that can be used to point to anything.
`void (*)()` is a pointer to a function that takes no arguments and returns no values.

I probably have explained it half a dozen times before,
but the number of times I explain something doesn’t really bother me,
particularly if it’s to a different person each time.

I’d gladly explain how it all works in full,
but I warn you that the explanation could be quite long depending how much you already know about pointers.

If you don’t know what pointers are and how they work I’ll have to explain those first - they’re fundamental to understanding how the rest works.

The influence of C...

Macros are still used by a lot of people because of the influence of C and people’s lack of familiarity with C++11.

When C++ was first created it was just an attempt to add OOP features to C, but over time it grew and diverged from C.
As a result of this, a lot of C++'s more fundamental features are inherited from C.

An unfortunate side effect of this is that people tend to think that anyone who can program in C can also write C++ and vice-versa, and similarly they think that something that’s considered good practice in C is also good practice in C++.

As a result, things that are used a lot in C often creep their way into C++ programs despite C++ having a much better option in its repertoire things 90% of the time.

One of these things is macros.
For some reason there’s a lot of people out there who love to use macros for things despite better options being available.
I suspect it’s because a lot of early C libraries set a bad precedent and people have only learned what’s good and what’s bad over time.

Even in C it’s generally considered bad to use macros for constants.
In C, `static const` variables are generally preferred,
though there are some cases where they can’t be used (see this for more info).
A lot of those caveats don’t apply to C++ though - C++ fixed a number of C’s shortcomings.

Don’t just take my word for it though,
here’s a bunch of relevant StackExchange links: 1, 2, 3, 4.

Basically there’s a long list of reasons why macros are bad and the kind of bugs they can cause.
I won’t go into all that unless you particularly want to know though,
because I don’t want to dump too much info on you in one go.

Unfortunately some people won’t listen to the experts, the best practices or the horror stories.
Some change their minds when they finally get bitten or hear a compelling enough story, but others will carry on regardless.

I mean to completely do away with the `#define`s, or as much as you possibly can.

Classes aren’t really a replacement for macros, but `constexpr` variables are.
As of C++11, `constexpr` is the proper way to make a constant in C++.
E.g. `constexpr int tileWidth = 16;`

There’s also ‘function macros’ which you may have not encountered yet.
90% of the time those can be replaced by template functions.

(Unfortunately 60% of the time people won’t replace them with template functions because they have an irrational fear of templates or can’t be bothered to understand templates. `:P`)

And lastly, according to the C++ Core Guidelines you should (and I quote):

Scream when you see a macro that isn’t just used for source control

So I kick up a fuss about it whenever I can. `:P`

I remember doing this in one of my games and found that it actually used more memory than a simple `switch` statement.

Why wouldn’t you have an enum of the valid states - this would prevent ‘out of bounds’ issues implicitly and make it more readable.

1 Like

This is probably the case I was thinking of earlier.
Always good to know I’m not imagining things.
My memory isn’t always completely reliable.
(Like that time I forgot where I got a hash function from,
and somehow convinced myself that I’d invented it. `:P`)

Technically it wouldn’t be completely impossible,
but it would indeed make it a lot harder to end up with an invalid state.

I almost suggested it before, but I don’t want to pile too many things on @raspberrybrain at once.

I’d have to explain what `enum class`es are, what `: uint8_t` means and why they’re being `static_cast`ed,
as well as maybe when they’re useful, why they’re better than macros and why they’re better than plain enums.

To you an I all this stuff is second nature, but to the uninitiated it’s a fair bit of reading and understanding.

Possibly … although it is interesting that @raspberrybrain is tackling function pointers (albeit from an article in the mag) but not some of the more ‘fundamentals’. @raspberrybrain … don’t take that the wrong way - we all learn in our own way and I think its great you are dissecting this code and taking the time to really understand it!

1 Like

True, but that’s just as much the fault of the magazine.

The same article that teaches the function pointer trick (Vol 3, Page 14-18) also teaches macros instead of constants and enums and doesn’t make any attempt to explain what a pointer is (beyond calling it a ‘reference’) or emphasise that it’s actually quite an intermediate/advanced technique.

But in fairness the article does actually say that using an array of function pointers increases the size of the code:

You probably noticed your code is a bit bigger than when you had the switch case. This is one of the things you’ll have to consider yourself, but just so you know. There is [sic] a lot of other places to regain those few lost bytes.

Amusingly the end of the article is:

Next time, we’ll talk about how we can pass variabels into a function and how we can receive a variable back from a function.

Call me crazy, but I think this tutorial series had its priorities a bit muddled.

(I’m probably being a bit harsh, but still…)