Skip to content

Conversation

@Riksu9000
Copy link
Contributor

Made the interface slightly larger.
Removed magic row and col counts. The size can be increased to 5x5 for example just by incrementing a value.
Fixed many warnings.
Reduced for loop count. Each either save a few bytes or no change.
Reduced library dependency.

In updateGridDisplay() only a for loop was removed and the indentation changed. The diff looks more complicated.

InfiniSim_2022-07-02_063814

@Riksu9000 Riksu9000 added this to the 1.11.0 milestone Jul 2, 2022
Copy link
Member

@FintasticMan FintasticMan left a comment

Choose a reason for hiding this comment

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

Wouldn't it be better to use std::array instead of raw arrays? It should be possible to just replace the raw arrays with std::array because nCols, nRows and nCells are constexpr.

@JF002
Copy link
Collaborator

JF002 commented Jul 4, 2022

I had the same thought, but I'm not sure it'll help to make the code more readable. For example TwosTile grid[nRows][nCols]; would become std::array<std::array<TwosTile, nRow>, nCol>. Plus we are using this array as a simple table, we don't really need methods from std containers.

TwosTile grid[4][4];
static constexpr int nCols = 4;
static constexpr int nRows = 4;
static constexpr int nCells = nCols * nRows;
Copy link
Member

Choose a reason for hiding this comment

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

It seems to me that a lot of the variables here are full (unsigned) ints, but they could just be (u)int8_ts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've noticed that sometimes using smaller integers results in a larger binary size. These specifically don't make a difference, but if tryMerge() and tryMove() functions used uint8_t, the binary size would increase. This is why I'm not always sure which is best to use.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm interesting... Maybe with smaller integers the compiler isn't able to do as many optimisations? Anyway, if it increases the binary size, I don't see much benifit to using smaller integers.

@Riksu9000 Riksu9000 merged commit 9b92861 into InfiniTimeOrg:develop Jul 6, 2022
@Riksu9000 Riksu9000 deleted the twos-cleanup branch July 6, 2022 08:29
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.

5 participants