[WIP] Arduboy2 Implementation Overhaul

Hello @Pharap ! what’s the status on this lib? this work is totally worth a simplified util that auto-ports games over…I want to play those sweet games on my new pokitto 8 )

3 Likes

You’re the first person other than @Vampirics to ask about this in quite some time.

In its current state the library can make almost any game playable, but there’s no sound support and a small number of less crucial functions are unsupported.

I’m not sure when I’ll get back to work on this.

Auto porting would be somewhat difficult.

It should theoretically be possible,
but the only way I can think to do it would mean running it through the tool the Arduino IDE uses to convert .ino files, and the code that spits out is always a hideous mess,
which is one of the reasons why so far I’ve just been doing manual conversion.

Any in particular?

1 Like

Jonne gave me a python script to convert the ini files to cpp, I’ll try and convert a game later on to see where the lib is at.
If it’s just about sound and everything else just works, I guess that would be a minor thing to implement as the arduboy only does bleep bloop : )

All of these:
https://arduboy.ried.cl/

There’s also some very specific screen behaviour (flipping the screen horizontally and vertically, possibly inverting the screen, something else that I’m forgetting).

I know nothing about how the sound behaves on Arduboy or Pokitto so I don’t know how easy it would be to replicate on the Pokitto.

The Arduboy’s sound is a bit more complicated than ‘bleep bloop’ though,
have you seen ATMLib at all?

That’s not “in particular”.

At any rate, some of those aren’t open source so they can’t be converted.

I see, so there’s still quite some work before we could have fully functional ports, I might look into the games I like the most and get back here if I attempt a port and need some help, thanks!

An idea for the Arduboy 2 library: Make it possible to specify a color for each bitmap (E.g. having a separate array somewhere, the bitmap address as a key). The bitmaps would be still in 1-bit format. The array would be utilised only when actually drawing a bitmap to the screen. That would be a way to enhance graphics a bit (pun not intended) to look like ZX Spectrum games, one color per bitmap.

2 Likes

It could be done, but there’s no way to do it without either modifying the game or providing some kind of ‘settings’ file because you’d need a way to associate the colours with the bitmaps.

You would either have to store the colour reference in the bitmap or pass it with the draw call,
and both of those approaches would break backwards compatibility.

On top of which, I’d have to completely rewrite the rendering code.
The ported code uses the same 1KB screen buffer that the original library does.
Doing that allows the port to support 3rd party rendering libraries (e.g. ArdBitmap and various font libraries).

Getting rid of that buffer would mean breaking support for those libraries and having to reimplement all the rendering functions (drawLine, drawRect et cetera).


I have already got something involving colour planned (and in fact implemented on my local copy),
I just haven’t got round to getting back to this project because I keep finding new projects.
(And Morrowind is fun.)

Not true. I have implemented this very idea in ports of Gamebuino classic games.

The idea is to have bitfields that are one behind the other. For example, to have 4 colours, you have a buffer that is 128x64/8 for first bit, followed immediately by 128x64/8 buffer for next bit. When rendering the pixel is formed by takinb lsb from buffer 0 and msb from buffer 1

First examples of crabator on Pokitto use this very technique

1 Like

I.e. ‘bit planes’.

To do that without reimplementing the various rendering functions would require changing what is currently a 1024 entry array into a pointer,
as well as introducing a new function to change that pointer.

On top of which masking would have to be done for every bit plane to work properly,
which would eat up a fair chunk of processing time.

Wanna live dangerously ? :sunglasses:

Change it into a dynamic array and bork the pointer in run-time. Then use as normal array!
The poor drawing functions won’t know where the hell they’re writing to.

Remember to save the original address :laughing:

Gratuitous misuse of C++.

I know you love stuff like this.

http://www.fredosaurus.com/notes-cpp/newdelete/50dynamalloc.html

1 Like

You’re joking right?

Smart pointers like std::shared_ptr and std::unique_ptr exist specifically so that people don’t have to resort to using new and delete or risk leaking resources by doing so.

Besides which, there’s absolutely no benefit to using dynamic allocation here anyway.
Dynamic allocation is for when you can’t predict the amount of memory that you’ll need for something.
In this case the amount of memory needed is certain:

  • If the screen were in 4 colour mode then there would be 2 ((128 * 64) / 8) arrays
  • If the screen were in 16 colour mode then there would be 4 ((128 * 64) / 8) arrays.

The link is somewhat outdated (it uses NULL instead of nullptr), and is actually incorrect in one place.

It says:

Attempts to use memory location 0, which is the normal default value of NULL, will be blocked by the way most operating systems allocate memory.

Aside from the fact that NULL doesn’t have to be 0, some systems won’t have that restriction.
(And in fact, some systems won’t even have an operating system).

There’s also numerous problems with the example code that make it a bad example…

Rant about the poor quality of the article's example code
  • It creates and assigns a before a is actually needed
  • It’s not properly qualifying std::cin
  • It’s not checking that the input was actually valid
    • For example, -1 is a valid input that creates a ridiculously large array
  • It’s using NULL instead of nullptr
  • It’s using int instead of std::size_t for array sizing and indexing
  • a = NULL most likely isn’t even needed - if a's scope is about to end (as it should be - reusing variables is a bad idea) then assigning it a value is a waste of time
    • std::shared_ptr or std::unique_ptr would deallocate as soon as their scope has ended, which eliminates this problem entirely

And most importantly, the loop isn’t needed in the first place.
If the allocation had been written as int * a = new int[n](); then the compiler would zero-initialise the array - no messy for loops needed.

With all that taken into account, the code should look a bit more like this:

std::intmax_t inputSize;

std::cin >> inputSize;

if(std::cin.bad() || std::cin.fail())
	return;

if((inputSize < 0) || (inputSize > std::numeric_limits<std::size_t>::max()))
	return;
	
std::size_t size = static_cast<std::size_t>(inputSize);

if(size > 0)
{
	int * dynamicArray = new int[size]();
	// ...
	delete [] dynamicArray;
}

(Please note that the returns should actually be throwing exceptions or handling the errors differently,
but I’ve left them as returns for the sake of simplicity.)

Here’s a fully commented version explaining the reasoning behind the changes:

Commented version
// A large integer to store the input size
std::intmax_t inputSize;

// Read in the user's input
std::cin >> inputSize;

// If the input wasn't valid, that's an error
if(std::cin.bad() || std::cin.fail())
	return;

// If the input value was negative, that's an error
if(inputSize < 0)
	return;

// If the input value exceeds a valid size_t, that's an error
if(inputSize > std::numeric_limits<std::size_t>::max())
	return;
	
// Size needed for array
std::size_t size = static_cast<std::size_t>(inputSize);

// Don't bother doing anything if the size is 0
if(size > 0)
{
	// Allocate size 'int's and store the resulting pointer
	int * dynamicArray = new int[size]();

	// No need to set everything to zero, because using new T[size]() already does that for you
	
	// Use the array

	// Deallocate the memory
	delete [] dynamicArray;
	
	// This is no longer actually needed because dynamicArray's scope is about to end,
	// but I'm mentioning it so I have an excuse to say 'use nullptr, not NULL'
	// dynamicArray = nullptr;
}
2 Likes

Oh yes there is. You just didn’t catch it.

If the array is declared as given in the example (agreed its horrible stuff), you can use same drawing function calls that address memory as array and you can increment the pointer to the other bitplane without the function knowing about it.

If it is a static array, the compiler will not allow you to do stuff like that.

EDIT: or wait a sec… probably it will allow, with casting. In which case you are correct

The only diference between a pointer to a statically allocated object and a pointer to a dynamically allocated object is that you have to remember to delete the latter when you’re done with it.

You don’t need any casting because the type doesn’t change.

constexpr std::uint8_t width = 128;
constexpr std::uint8_t height = 64;
constexpr std::uint8_t bitsPerByte = 8;
constexpr std::size_t size = ((width * height) / bitsPerByte);

std::uint8_t plane0[size];
std::uint8_t plane1[size];

std::uint8_t * Arduboy2::sbuffer = plane0;

Then just reassign sbuffer as necessary.
Completely type safe, no casting required.

You could also have the planes be a 2D array if you really want:

constexpr std::uint8_t width = 128;
constexpr std::uint8_t height = 64;
constexpr std::uint8_t bitsPerByte = 8;
constexpr std::size_t size = ((width * height) / bitsPerByte);

std::uint8_t plane[2][size];

std::uint8_t * Arduboy2::sbuffer = &plane[0][0];

But that’s the easy bit. As I said, you then have to inject a load of function calls to change the colour at the right moment, and you have to make sure masking works properly.

(I haven’t checked properly, but I’m pretty sure you can’t mask just one bitplane or it won’t display properly. You’d have to mask all the bitplanes.)


And of course there are cases where changing sbuffer to a pointer will break code.

For example if someone is trying to infer the size of the array, as is demonstrated here:

(The README.md file is pretty educational if I do say so myself.)

1 Like

Hey @Pharap … I finally forked this and am trying to make it work.

How do you compile this (your AB2 lib port) for Pokitto?

Edit: tried twice, couldn’t make it work. stdlib.h/cstdlib missing declarations & math.h round etc conflicts

I really could not figure out the right way to build

Edit2: I know the fault is not in your code. But the avr-libc makes the build process convoluted. I really need your recipe

What is your setup like?
What are you trying to compile with,
and what sort of error messages are you getting?

It’s been quite some time since I touched this,
but I don’t remember having any issues building it.
I was using PlatformIO and I just shoved everything in the lib folder and it all just worked.

It ought to just be a matter of making sure you build avr-libc, then Arduino, then Arduboy2, then the user’s program.
(Though I can imagine the PokittoLib’s reliance on having a user-provided My_settings.h might complicate matters.)

Its possible my local copy had some changes that the GitHub repo doesn’t that made it easier, but again it’s been quite some time so I don’t quite remember.

This is a bloody nightmare to get to compile.

I know it is not your fault but it just is (a nightmare). The avr-libc clashes with the gcc files.

Frankly I don’t understand how PlatformIO was able to compile it.

EDIT, now I am stuck with io.h

… got past that one by copy-pasting the missing structs

… avr_libc compiling now, some progress!

… stuck in math.h, definition of extern _cdecl round (double) gives “expected unqualified id”

1 Like

Could any of the differences be explained by the fact it would have been compiling with PokittoIO instead of PokittoLib?

I can’t really help without knowing more about your environment.
E.g.
Are you including any C files outside of Pokitto lib?
Are you using angle bracket includes or quote includes?
Are you using FemtoIDE, EmBitz, something else?
What order is everything being compiled in?

GCC’s math.h? Where is it being included?


This may or may not be useful:
https://docs.platformio.org/en/latest/librarymanager/ldf.html#ldf

My platformio.ini was just:

; PlatformIO Project Configuration File
;
;   Build options: build flags, source filter
;   Upload options: custom upload port, speed and extra flags
;   Library options: dependencies, extra library storages
;   Advanced options: extra scripting
;
; Please visit documentation for the other options and examples
; http://docs.platformio.org/page/projectconf.html

[env:pokitto]
platform = nxplpc
framework = mbed
board = lpc11u68

build_flags =
  -D MBED_FAULT_HANDLER_DISABLED
  -D POKITTO_PIO_BUILD
  -I src
  -std=c++11

; Needed for Arduboy2 to compile properly
lib_compat_mode = off

extra_scripts =
  pre:pokitto_update_ld.py
  post:pokitto_copy_bin.py
  ;post:pokitto_post.py
1 Like

I’ve been using my local version of this project to refine the library system on the compile script I’m working on, and I’ve also come across some of the problems you’ve encountered.

I’m not done yet, but so far some of the problems I’ve encountered:

  • Cannot find <avr/pgmspace.h>
    • Solution: only use -I <path>/avr-libc, adding -I <path>/avr-libc/avr seems to upset GCC for some reason
  • Unexpected unqualified-id before double
    • A conflict between the round macro defined in Arduino.h and the round function that <cmath> tries to declare
    • Note 1: Yet more examples of why macros are evil
    • Note 2: An example of why <cmath> shouldn’t be implemented by including <math.h> and then trying to using ::<symbol>; everything into namespace std
  • mbed_wait_api.h: No such file or directory
    • Interesting factoid - this is actually what the file appears to be called in PlatformIO’s mbed framework, but the version bundled with the PokittoLib is just called wait_api.h. Aside from some extra doxygen at the top of mbed_wait_api.h, the two files are identical, which suggests that either mbed changed the filename at some point, or this file was misnamed when it was added to the PokittoLib. Considering that a large number of the mbed framework files have an mbed_ prefix and PokittoLib also has mbed_assert.h, mbed_debug.h, mbed_error.h and mbed_interface.h, I’m inclined to assume it’s the latter - this file was renamed when it was originally added to the PokittoLib.
    • Not really evidence either way, but the current version on mbed-os is called mbed_wait_api.h.
2 Likes