[WIP] Arduboy2 Implementation Overhaul


#1

Background

A few weeks ago I asked about whether people would be interested in a port of one of my Arduboy games, Minesweeper.
A lot of people were, so I started to attempt the port,
but I soon discovered that the implementation of the Arduboy2 library was somewhat outdated and missing a few features.

It was a real shame.
If only I was an experienced C++ programmer who knew a thing or two about the Arduboy2 library…
Oh, wait a minute :P

So I decided to fix the library.
But instead of just fixing the existing one, I decided to go one better…

The Overhaul

So for the last week or two I’ve been reimplementing the Arduboy2 library for Pokitto (on and off) from scratch, without looking at the existing implementation.
And on top of that, I’ve been implementing just slightly more than the minimal amount of avr-libc and Arduino that are necessary to get Arduboy2 working.

It’s taken me some time because I wanted to clean the Arduboy2 library up a bit first, and I’m trying to keep the history as tidy as I can so it can serve as a form of documentation to whoever might have to maintain it after me.

(Though I have made the occaisional mistake - in the worst case I had to scrap a PR because I gave the commit the wrong summary. Nobody’s perfect. :P)

Status

It’s not yet ready but I’m making progress.
There’s still a long list of bugs, but I’m gradually fixing them.

Currently all button functionality works and rendering works.

Unfortunately everything seems to be running slowly.
I don’t know if that’s because Arduboy2 code uses std::uint8_t everywhere and the Pokitto’s processor needs extra instructions to handle those, or if my screen buffer drawing routine is too slow, or if there’s some bug somewhere else.

Implementation Details

(A.k.a the geeky/cool bits)

For the rendering, I’ve done something that I hope is quite clever.

Instead of replacing all the drawing functions with the Pokitto equivalent, I’ve kept all the original Arduboy2 functions intact and added a function that draws an Arduboy2 format bitmap onto the Pokitto’s screen buffer.

Not only does this let me be really lazy,
but hopefully when I’ve finished this will allow for a very special feature to be implemented…

GitHub

I’m going to post a link to the GitHub so people can watch the development if they want, but please don’t raise any issues or PRs while it’s still a work in progress.

There possibly will be a point where I ask for some extra help,
but at the moment it’s all under control and the main issue is just finding the time to work on it.

Licencing

Although it’s just one repo, there’s actually 3 separate libraries, each released under a separate licence.


[Game]CastleBoy (Direct Port)
Pokitto is on the way, what games should I make?
Port Centipede Meta to Pokitto?
[Game]Millipe (Direct port)
#2

Yeah, not many other people would be able to do this, so thanks :slight_smile:

Check the generated assembly then? I’d like to know this too BTW, let me know please :slight_smile:


improvement proposal: s/\t/ /g :stuck_out_tongue:


#3

It’s just one extra instruction, either UXTB or SXTB. It only takes one cycle, so unless it’s in a really tight loop it shouldn’t be a problem.

If only there was some way to run the code and find out what it’s doing most of the time… :-p


#4

But don’t forget the Arduboy uses uint8_t everywhere.
So it’s not just an extra instruction here or there,
it’s lots of extra instructions splattered around the whole thing.

(For what it’s worth though, I now know that it’s my screen drawing loop that’s too slow so I’m going to have to work directly with the screen buffers. I have a plan that I hope will be fast enough.)

That means I have to dig out the objdump executable… Bleh, effort.

I’m due to be sent a JLink at some point so that will help too.

No chance. Tabs forever!


#5

I’ve fixed the speed issue!


I had a few issues due to a problem with Arduboy2 itself (for which I’ve created an issue).

The Problem

Spot the mistake:

void static paintScreen(const uint8_t * image);
void static paintScreen(uint8_t image[], bool clear = false)
The Mistake

Normally if two signatures only differed on a default argument then the compiler would get confused and error,
however in this case the compiler notices the variable given to it (i.e. the Arduboy’s screenbuffer) isn’t const,
so it decides the second one is a better match and the first case is never called.

My Solution

Implement the helper function embedded in the second version as:

void drawArduboy2Buffer(std::uint8_t * buffer, bool clear)
{
	if(clear)
		drawArduboy2BufferAndClear(buffer);
	else
		drawArduboy2Buffer(buffer);
}

And later I had problems getting the maths right, but once I’d implemented drawArduboy2Buffer_2bpp, writing drawArduboy2Buffer_4bpp was a doddle.


If anyone’s interested in the progress of this project,
let me know if you want me to post regular or semi-regular updates.

(At the moment @jonne’s getting a semi-regular report of my almost unfiltered thoughts on the project and he’s probably getting sick of them :P)

It’s not really far along enough to post bins of games,
but there’s enough implemented to get LoveRush to the titlescreen.


#6

I like to read about your progress…


#7

I’ve established which part of the code has been causing LoveRush to lock up upon entering the gameplay state and half of why it’s doing it, but the other half won’t be fun to solve because it’s present in both Sprites::drawBitmap and SpritesB::drawBitmap.


So in the meantime, it took a bit of butchery and you can’t save,
but who wants a working .bin of Minesweeper?

Minesweeper.bin (53.5 KB)

Minus the saving…
And it crashes if you try to scroll the credits page because of the same bug in Sprites
And you only actually get one game board becuase of how I hacked together the random seed generator function…
(Give me 3 minutes and I’ll fix that last one. Actually, let me get back to you on that…)


#8

Found the problem with the seed generator.
It seems Pokitto::Core::initRandom(); doesn’t actually have much variance in its value,
so I’ve substituted srandom(Pokitto::Core::getTime()) for now.
Hopefully we can replace it with some pin noise or something later on.

So here’s a non-saving, crashes when you scroll the credits down version, but at least you can play it:

Minesweeper.bin (53.5 KB)


#9

Success! Goodbye Sprites bug!

Game of LoveRush anyone?

LoveRush.bin (56.8 KB)

Or Minesweeper with the credits?

Minesweeper.bin (53.6 KB)

Edit: The .bins are now individually named instead of both being firmware.bin. Can’t rename due to website caching.


This is what I love about programming - encountering an absolutely baffling problem and then having a sudden ‘eureka’ moment when everything clicks and the solution becomes clear.

I haven’t commited the fix yet,
I want to do a little write-up on the problem and its solution first,
and before I do that I’m going to get some lunch.


#10

Gonna try those out immediately with the brand spanking new loader

EDIT: Pharap: both files are named firmware.bin which makes drag and drop to SD a btit cumbersome


#11

Great work @Pharap !

Looks absolutely fabulous, runs like butter on a hot plate.

You’re so getting the top Wizard badge for this. I know behind the scenes how much work has gone into this.

I just wish there was some sort of write-up available … :grinning:

Meanwhile you’ll have to make do with the 2nd level badge, congratulations!

wizard%20journeyman


#12

Sorry, I dragged them over as quick as possible because I was excited to show them off. :P

Yes, finally a wizarding badge!
It may just be silver, but given time I’ll get it to gold.

Yep. Measure twice, cut once.
Prepare everything carefully and everything falls into place.
(Like a certain famer.)

I will do one, but I’m not done yet.

The core mechanics might be there,
but I’ve still got a big list of functions and features to implement.

  • EEPROM reading, writing and updating
  • LED emulation
  • Sprites::drawPlusMask (fortunately I know exactly where to get the code for that :P)
  • Vertical and horizontal screen flipping
  • Screen inversion
  • Various other display commands
  • And my worst enemy… sound…

#13

Just tried both of them. They run indeed very smooth!

But… they look so tiny :slight_smile:


#14

I’m going to ramble on a bit about the Sprite::drawExternalMask bug and the process I went through to solve it.

I’m going to put it in a details block because I don’t know how many people are actually interested in the ramblings of a madman, and frankly it’s probably a bit boring, but maybe someone will find it useful or interesting.

I guess Arduboy2 porting isn’t as shiny as 3D rendering and emulation…

(Warning: these are really long.)

The Long And Tedious Process

My first port of call was to narrow down the cause of the bug.
I don’t have a hardware debugger yet, so I had to do things the old fashioned way - inserting print statements into the code to print information onto the screen.

This can be very tedious.
First I narrowed it down to gameplay(),
then I narrowed it down to drawObjects()
then I narrowed it down to drawObject(object)
and from there I narrowed it down to Sprites::drawExternalMask.

I tried substituting Spritess:drawOverwrite and SpritesB::drawExternalMask… same result.
I then thought “ah, maybe I broke it during clean up”, so I substituted the original Arduboy2 Sprites code (minus the assembly).
Same problem, which means that for some reason code that works perfectly well on the Arduboy doesn’t work perfectly well on the Pokitto.

Next I tried commenting out drawObject to see if everything else worked.
I discovered drawBackground() had the same issue, coming from the exact same functions.
So I commented out drawBackground() and suddenly the game worked - all the logic was there and working perfectly without that rendering code.

I realised that other parts of the code were still using Sprites::drawExternalMask just fine, so I decided to log the parameters being fed into the drawing function in drawObject (since that was the first breaking point).

At first I thought it was specific sprites causing the problem so I disabled specific object types.
I’d got to the third disabled object when I suddenly spotted a pattern.
Every time it froze, the y input was negative.
So I inserted an if(y > -1) above the drawing call, and sure enough everything started working.

So at this point I’d established that:

  • The code works fine on Arduboy
  • There’s something causing it to behave differently on the Pokitto
    • That something is related to negative y values

So I moved into the drawing functions and started logging everywhere to find the breaking point, as standard.

I manage to pinpoint the break to one exact line:

uint8_t data = Arduboy2Base::sBuffer[ofs + WIDTH];

And I discovered that actually ofs was surprisingly high (in about the 65000 range).
I know from experience that’s usually a tell-tale sign of negative values being converted to unsigned values.
So I trace the code to ofs's defnition.

uint16_t ofs = ((sRow * WIDTH) + x + xOffset);

I find that sRow is defined:

const int8_t yOffset = (y % 8);
const int8_t tempSRow = (y / 8);
int8_t sRow = ((y < 0) && (yOffset > 0)) ? (tempSRow - 1) : tempSRow;

So evidently this is somehow related.

I start analysing the uint16_t ofs = ((sRow * WIDTH) + x + xOffset); expression and logging its components.
I find that sRow = -1, WIDTH = 128, x = 63 and xOffset = 0, which means (sRow * WIDTH) = -128 and x + xOffset = 63, so obviously -128 + 63 = -65 and of course -65 converted to a uint16_t is 65471.

So then I come back down to uint8_t data = Arduboy2Base::sBuffer[ofs + WIDTH];.
I think “ok, so 65471 + 128 = 65599, and obviously -65 + 128 = 63, which cancels out the -128 from earlier (which was from sRow * WIDTH while sRow = -1)…”
“But…” I ask myself, “this works on Arduboy, so what would cause it to not be working now?”

And then it hit me.
I remembered a basic law of C++ and suddenly everything made sense…

Note

(There was a point where I went down the wrong path and started looking into endianness, but after establishing both devices are little endian I scrapped that idea. A small 15-20 minute red herring.)

The Solution To The Problem

So eventually I had isolated the problem and had a moment of inspiration.

I realised two important bits of information and put 2 and 2 together and arrived at a wonderful 4.

The first thing I realised was the solution to the thing that was bugging me - why would it work on Pokitto and not Arduboy?
The answer has to be something that’s different between the Arduboy and Pokitto, and sure enough it is.

The answer is int.
On the Arduboy an int is 16 bits wide.
On the Pokitto an int is 32 bits` wide.

But if ofs is a uint16_t, then where is this int?
And then I remembered one of the fundamental rules of C++.
Something that happens all the time right under everyone’s noses but it often goes unnoticed.

The answer is ‘integer promotion’.
From cppreference’s section on Numeric Promotions:

unsigned char or unsigned short can be converted to int if it can hold its entire value range, and unsigned int otherwise;
arithmetic operators do not accept types smaller than int as arguments
And on the Pokitto, uint16_t is a type alias for unsigned short, hence ofs + WIDTH (i.e. uint16_t + <integer literal>) causes ofs to be promoted to int.

On the Arduboy, with or without promotion the arithmetic is 16-bit arithmetic, thus overflow occurs, the overflow bits are discarded and the addition correctly cancels out the earlier addition of a negative.

On the Pokitto, the promotion causes 32-bit arithmetic to be used, so instead of the overflow bit being discarded, the overflow bit is kept and the result is larger than the size of the display buffer, thus resulting in a buffer overrun.
It is that buffer overrun that caused the Pokitto to halt.

With this realisation I instantly form a solution to mimic the original behaviour:

uint8_t data = Arduboy2Base::sBuffer[(ofs + WIDTH) & 0xFFFF];

Mask off the higher bits that are dropped in the Arduboy’s original calculations.

To think that the addition of one single mask operation can be the difference between perfectly running code and a processor halting bug.

So what can we learn from this…

The Moral Of This Story

There are many things to take away from this.

Type safety is important. Always know the possible ranges of types.
int is at least 16 bits. It could be 16 bits, it could be 32 bits, it could be 64 bits.
If you want to write portable code, keep that in mind.

Be aware of integer promotion.
It can sneak up on you if you aren’t careful.
Sometimes people think their code means one thing,
but integer promotion changes its meaning and result.

Be careful of mixing signed and unsigned values.
Try not to mix them, but if you must mix them, make sure to double check the behaviour.

Always try to make type conversion explicit.
Hiding type conversion makes code harder to undestand.
If conversions aren’t explicit then sometimes you have to really look to understand what’s going on.

And lastly and most importantly:
Avoid integer overflow like the plague.
If your solution to a problem relies on overflow or underflow then you are probably tackling the problem incorrectly.
Don’t wait for overflow to happen - prevent it!


#15

Firstly, compared to the actual Arduboy screen they’re not that much smaller.
An Arduboy screen is about ~2.9x1.9cm.
The Arduboy emulation on Pokitto results in a ~2.3x1.2cm image (so about 6mm either side).

Secondly, they can’t exactly fit the whole screen, unless someone’s prepare to make a scaling algorithm that gives them a width of 1.5 pixels (by mixing black and white to get grey).

Thirdly, I have a magic trick up my sleve for later.
I don’t want to ruin the suprise because it’s really going to be the cherry on top.


At most I’ve told 3-4 people, 2 of whom are actually partly involved in the development.

Jonne (listens to my ramblings and answers the odd question),
Vampirics (told him for a specific reason)
SpaceBruce (I know him in person so I often tell him what I’m working on),
and Filmote (I might not have told Filmote, I can’t remember).

Absolutely nobody else knows.


#16

I’ve now properly commited the fix:

And marked this point in history with another release,
which includes the Minesweeper.bin.


#17

I couldn’t get the insignificant screen manipulation functions that barely 1% of Arduboy games use working today because of the effort involved in adapting my current code to incorporate the requred changes…

So instead of some boring old screen functions, I guess I’ll have to implement EEPROM so you people can save your highscores and stuff…


So… fully functional LoveRush and Minesweeper anyone? :P

LoveRush.bin (56.9 KB)

Minesweeper.bin (53.7 KB)


#18

Just a minute, pardner!

I use a lot of time to come up with a 100% safe EEPROM handler and it seems you intend to write directly to the EEPROM.

Why? Have I failed to explain how it works and/or what are the benefits?

giphy

EDIT: this is all toungue-in-cheek ofcourse :yum:

EDIT2: I will, ofcourse, help make the safe implementation. But it is essential we don’t go back down to that level where settings get wiped out or something because everyone writes wherever they please in the EEPROM

EDIT3:

It should work something like this.

  • The Arduboy program reserves a block of EEPROM using the Pokitto::Cookie mechanism.
  • That block is filled with an array of bytes (for example 256 bytes)
  • EEPROM write bytes and read bytes are actually wrappers that manipulate individual bytes in that array
  • everything happens inside a Pokitto::Cookie, allocation tables do not get broken and there is no possibility of overwriting EEPROM saves from other programs, including other Arduboy programs that use EEPROM

p.s.

The Arduboy2 port is a fantastic contribution. Sorry for the hard comment on the EEPROM write/read bits, afterall its only a part of the problem. But I am dead serious. We will never get the EEPROM situation under control if there is no system to prevent accidental overlaps between programs. AFAIK this is a recurring problem on the Arduboy


#19

I haven’t forgotten about the ‘cookie’ system. All in good time.
Before I go about implementing the ‘cookie’ system I need to inspect the ‘cookie’ API and decide the best way to apply it.

There’s some other bits and pieces I want to get working first.
Despite my little sidetrack with the timers earlier today
(which was primarily for testing the water of how accurate I can get the screen),
I am intending to focus on implementing the minimum amount of features required to get games running first, and then refine them afterwards.


As far as I’m aware, very few people worry about it.
Most people just accept it and remember to back up their saves.

There is actually a way to communicate with the Arduboy’s bootloader over serial to get a copy of the EEPROM for backup purposes but I’ve never tried it myself.

@Mr.Blinky wrote a quick and easy Python script for it, included among his Arduboy utilities:

There’s a video of it in action here.

I haven’t used it myself because I haven’t needed it.


Also, if you have a dig around my Minesweeper game you’ll find this little gem:

Fully checksummed to detect data corruption,
and both forward and backwards compatible
(provided that new versions of a game only ever add new save fields and don’t remove old ones).

(I was intending to try to get people to adopt it for Arduboy,
but my to do heap is ever-growing.)


And in case it looks like I have no idea what I’m doing,
I assure everyone that I have a plan.

I have a ‘to do’ list (categorised rather than ordered):

To Do

Sprites.cpp

  • Fix drawPlusMask

ArduboyCore

  • Implement setCPUSpeed8MHz or mark it as intentionally empty
  • Implement boolOLED or mark it as intentionally empty
  • Implement idle or mark it as intentionally empty
  • Implement bootPowerSaving or mark it as intentionally empty
  • Implement boot or mark it as intentionally empty
  • Implement displayOff *
  • Implement displayOn *
  • Implement paint8Pixels
  • Implement sendLCDCommand or mark it as intentionally empty
  • Implement invert *
  • Implement allPixelsOn *
  • Implement flipVertical *
  • Implement flipHorizontal *
  • Implement setRGBled variants
  • Implement freeRGBled
  • Implement digitalWriteRGB variants
  • Implement exitToBootloader
  • Implement mainNoUSB
  • Implement TXLED1
  • Implement TXLED0

wiring.cpp

  • Implement init or mark it as intentionally empty

stdlib.cpp

  • Implement itoa
  • Implement utoa
  • Implement ltoa
  • Implement ultoa
  • Implement dtostrf
  • Implement dtostre

ArduboyAudio

  • Implement everything

ArduboyBeep

  • Implement everything

And outside of that I have various notes on paper.
(I prefer paper for notes because I’m always misplacing my .txt files and struggling to think of names for them. A piece of paper doesn’t need a name, just some physical space to occupy.)


#20

Just an idea. How about emulating Arduboy EEPROM by saving and loading to SD card?