Kong [1.0.3]

I am determined to free up enough memory to use ArduboyPlaytune.

Thanks @MLXXXp … I haven’t had a chance to look at the code and I am not sure I would know what to change. If you can point me to potential change, I will give it a go!

You’ll need an understanding of how to control the pins directly by writing to the port registers. ArduboyPlaytune already directly manipulates the registers (rather than using the Arduino pin functions) but it uses variables rather than constants when doing so. This is where some code could be saved (although I can’t say how much. It may not be very much).

Also, if you’re going to hard code the pins, you can save more code by hard coding the timers as well. (Again, I can’t say how much.)

I would look at the ArduboyTones library as an example of directly controlling specific pins.

If you’re going to do this, I suggest that you copy the ArduboyPlaytune source and make a custom version as a local library that’s part of your sketch, with renamed files and classes. (Be sure to follow the MIT licence it uses.)

Thanks for the feedback, I will probably look at it tonight.

Wrote a sketch that uses ArduboyBeep to read and play the multi-channel patterns in their native format without conversion:

Compiles to the same ~8500 bytes that Playtune did - but this is doing more than just one note for 1/2 second… and storing the patterns in their native format should be a lot more efficient than the Playtune score format?

Also, I edited the pattern to have the duration before every note pair - so this should be even smaller once keeping-the-previous-duration-for-all-notes-until-a-new-duration-is-found gets written in…

2 Likes

Fantastic! Now I have even more incentive to save that memory :slight_smile:

3 Likes

I have added a link to the current ‘Beta’ version.

Current Issues:

  • Occasionally, barrels are launched too close together.
  • There is no theme music (if anyone wants to contribute?)

Recently Fixed:

  • Cable from level 2 appearing in level 1.
  • Made jump a little shorter.
  • I have also added a little ‘Lives Left’ indicator on the scoreboard which scrolls with the game. Not sure if I like it, what do you think?
2 Likes

@Pharap Do you know when constructors and destructors are called implicitly? Kong is spending over 0.5kb just on free and malloc.

It depends on where the variable is allocated.

For statically allocated variables (i.e. globals) the constructor is called once before main and the destructor is called once after main.

For local variables the constructor is called at the point the variable is defined and the destructor is called when the variable falls out of scope and its lifetime ends.

For dynamically allocated variables the constructor is called when new is used and the destructor is called when delete is used.

Why is it using malloc and free in the first place? It probably shouldn’t be.

It doesn’t call free or malloc (that I see), but it does declare destructors on objects:

    virtual ~GameState(void) {};

That’s what that is, right? Could that be pulling them in just as a matter of course? Or are these things entirely unrelated and I’m getting confused?

00001528 00000261 t Images::HighscoreText_Mask
00017874 00000262 T TitleScreenState::update(GameStateMachine<GameContext, GameStateType>&)
00023230 00000262 T Game::loop() [clone .constprop.38]
00001789 00000263 t Images::HighscoreText
00030922 00000274 T free
00030618 00000304 T malloc
00017052 00000344 t ArduboyTonesExt::nextTone()
00016674 00000348 T PlayGameState::activate(GameStateMachine<GameContext, GameStateType>&)
00002052 00000360 t Images::HighscorePanel_Mask
00002412 00000362 t Images::HighscorePanel
00012956 00000426 t Coordinates::Barrel_Splash
00014394 00000448 T HighScoreState::update(GameStateMachine<GameContext, GameStateType>&)
00011265 00000452 t Images::Plate
00003543 00000542 t Images::Gorilla
00013814 00000580 T HighScoreState::render(GameStateMachine<GameContext, GameStateType>&)
00016072 00000590 T PlayGameState::render(GameStateMachine<GameContext, GameStateType>&)
00005611 00000590 t Images::Title_Kong
00000210 00000660 t Coordinates::Barrel
00025032 00000706 T PlayGameState::drawScenery(GameStateMachine<GameContext, GameStateType>&, unsigned char) [clone .constprop.105]
00027224 00000966 t Sprites::drawBitmap(int, int, unsigned char const*, unsigned char const*, unsigned char, unsigned char, unsigned char)
08389252 00001024 B Arduboy2Base::sBuffer
00006262 00001585 T _ZN11CoordinatesL6PlayerE.lto_priv.132
00018460 00002532 T PlayGameState::update(GameStateMachine<GameContext, GameStateType>&)
00008548 00002592 t Images::Crane

Lots of space lost to Images::Crane and those HUGE Coordinates tables.

All the *State classes reference free (via delete):

000044dc <HighScoreState::~HighScoreState()>:
    44dc:	0c 94 f7 37 	jmp	0x6fee	; 0x6fee <operator delete(void*)>


000044e2 <PlayGameState::~PlayGameState()>:
    44e2:	0c 94 f7 37 	jmp	0x6fee	; 0x6fee <operator delete(void*)>


000044e8 <SplashScreenState::~SplashScreenState()>:
    44e8:	0c 94 f7 37 	jmp	0x6fee	; 0x6fee <operator delete(void*)>


000044ee <TitleScreenState::~TitleScreenState()>:
    44ee:	0c 94 f7 37 	jmp	0x6fee	; 0x6fee <operator delete(void*)>

I don’t see where malloc is called anywhere, but maybe it comes along for the ride with free? Not sure why compiler couldn’t strip it though.

I wouldn’t say lost as the coordinates tables are central to the game. Attempting the movements other ways would result in significant code or other reference data. But yes they are HUGE.

The crane images could well be cut down a bit. I have used frames (hence the images are all the same size) and a generic scenery rendering routine (which handles the vertical scrolling as well) which would need to be substituted for code.

Removing the “destructor” mentions from both classes removes free/malloc from the compiled output.

Does the order of const uint8_t PROGMEM Scenery[] = { matter?

Generally no but there are some things, like the cables for the hard level, that I want rendered after other items. These are probably the only things I can think of so they would need to be at the bottom.

Why?

And frees up 660 bytes.

virtual destructors have nothing to do with malloc or free.

virtual destructors are what enable the destructor of a child object to be called when a pointer to said object has been upcast to the object’s parent (or one of the object’s parents).

E.g.

class Base { virtual ~Base() { std::count << "Base" << '\n'; } };
class Child { ~Child() { std::count << "Child" << '\n'; } };

// Don't try this at home, it calls the destructor twice
void function()
{
  Child child;
  // Implict upcast:
  Base * base = &child;

  // Manually call the destructor,
  // which would call both the base destructor
  // and the child destructor
  base->~Base();
  
  // child.~Child(); is implicitly called here,
  // which is a bad thing
}

I’m resisting the urge to post a Trump meme.

Ok, I’ll rephrase my earlier question:
Why is it using new and delete in the first place? It probably shouldn’t be.

For reference:
new is almost always implemented by calling malloc and delete is almost always implemented by calling free.
They’re specifically defined to make that possible.

It doesn’t have to happen that way, the standard explicitly classes ‘the heap’ and ‘the free store’ as separate entities, but I’ve yet to see an implementation where they are actually different.

That sounds like a compiler bug to me.

I can only think of one possible reason why they might be using new and delete.
It’s possible that they’re using it to somehow allocate a buffer to copy the vtable to.
But that seems absolutely ridiculous, and is frankly concerning.
It would explain a lot though.

Make sure the destructors are actually safe to remove.
If they’re all empty then destruction is trivial and it should be fine to remove them and in fact never even call them (because the compiler won’t either - empty/trivial destructors are a nop).

‘Matter’ how? Syntactically?


@Dreamer3 & @filmote
You know you can edit your old comments instead of posting new ones in quick succession, right?

1 Like

Yes.  

1 Like

That can make it HARDER to follow a fast moving thread though. :frowning:

Generally no but there are some things, like the cables for the hard level, that I want rendered after other items.

Quite obvious you should GROUP them then instead of repeating the type with every item:

// bad
    9, 67, static_cast<uint8_t>(Components::Girder),
    19, 66, static_cast<uint8_t>(Components::Girder),
    29, 65, static_cast<uint8_t>(Components::Girder),
    39, 64, static_cast<uint8_t>(Components::Girder),
    49, 63, static_cast<uint8_t>(Components::Girder),
    59, 62, static_cast<uint8_t>(Components::Girder),
    69, 61, static_cast<uint8_t>(Components::Girder),

    9, 47, static_cast<uint8_t>(Components::Ladder),

    21, 47, static_cast<uint8_t>(Components::Girder_OverHead),
    33, 47, static_cast<uint8_t>(Components::Girder_OverHead),
    45, 47, static_cast<uint8_t>(Components::Girder_OverHead),
    57, 47, static_cast<uint8_t>(Components::Girder_OverHead),
    69, 47, static_cast<uint8_t>(Components::Girder_OverHead),
//better
    8, static_cast<uint8_t>(Components::Girder), # 8 of these
    9, 67, 
    19, 66,
    29, 65, 
    39, 64, 
    49, 63, 
    59, 62, 
    69, 61, 

    1,  static_cast<uint8_t>(Components::Ladder),
    9, 47,

    5, static_cast<uint8_t>(Components::Girder_OverHead),,
    21, 47, 
    33, 47, 
    45, 47, 
    57, 47, 
    69, 47, 

You can even keep the ordering aspect since you’re free to declare extra groups - ie, not every ladder has to be in the same group, etc.

I wouldn’t say lost

I didn’t mean “wasted” exactly, I should have said spent. I merely meant that if you want to attack space you start with the HUGE offenders, not the functions taking up 6 bytes… so when you see something like a 1.5kb data table you start to wonder if there is a better way to encode it to use far less space.

And frees up 660 bytes.

Welcome. :slight_smile: How many bytes to go before I get a mention in contributors.txt? :slight_smile:

I don’t want to nit pick, but the scenery table is 228 bytes long. Your suggestion would remove 76 bytes but would add 20 (number of Component elements) * 3 (some delimiter, the quantity and component type) = 60 bytes back in. I would also have to add extra logic in the rendering which I am sure will consume the saved 16 bytes!

But your point is absolutely take - big chunks of data lend themselves to optimisation.

Consider it done.

At present, I am not sure what I am going to do with these space savings. The sound effects that @uxe found could be included but they didn’t sound as good as the arcade machine on the Arduboy’s tinny little speaker.