Skip to content

Conversation

@SoluMilken
Copy link
Collaborator

@SoluMilken SoluMilken commented Aug 13, 2025

Purpose

Reduce memory usage from DISPLAY_WIDTH x DISPLAY_HEIGHT x uint8_t to DISPLAY_WIDTH x uint8_t.

@SoluMilken SoluMilken changed the title Yien/less memory spaceship use bitmap Aug 18, 2025
@SoluMilken SoluMilken self-assigned this Aug 18, 2025
@SoluMilken SoluMilken force-pushed the Yien/less-memory branch 3 times, most recently from 011860a to 955aad7 Compare August 19, 2025 16:30
@SoluMilken SoluMilken marked this pull request as ready for review August 20, 2025 16:20
@SoluMilken SoluMilken requested a review from john0312 August 20, 2025 17:05
if (_routine_task.IsEnabled()) {
scheduler.DisablePeriodic(&_routine_task);
}
scheduler.DisablePeriodic(&_routine_task);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you removing the check here? You're certain it won't happen? Note that disabling an already disabled periodic task will assert.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me restore it.

namespace spaceship {

enum state_t { INIT, RUN, GAME_OVER, END };

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to static_assert() on the display height.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I use

static constexpr unsigned PLANE_UPPER_BOUND = 1 << (DISPLAY_HEIGHT-2);

instead of using

static_assert(DISPLAY_HEIGHT==8)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason why you need static_assert() is because you used a byte to represent a column, which is 8 pixels currently. Whenever that changes your code will break, and we'll want to warn anyone of that using static_assert().

for (uint8_t i = 0; i < 4; i++) {
if (_my_plane[i] == _enemy_position) {
for (uint8_t i = 0; i < 2; i++) {
if ((_my_plane[i] & _enemy_position[i]) > 0) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually we do != 0 to avoid signedness shenanigans

for (uint8_t i = 0; i < (DISPLAY_WIDTH - 1); i++) {
_enemy_position[i] = _enemy_position[i + 1];
_enemy_position[i + 1] = 0;
if (_enemy_position[i] > 0) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Best to use != 0 for bitfield checks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants