Game of Life Rework #120

Brandon502 · 2024-02-26T02:30:11Z

Removed randomness. Improved infinite pattern detection. Added color mutation slider, wrap option, and overlay option.

No longer uses ColorCount struct. Removed randomness. Improved infinite pattern recognition. Adds color mutation slider and wrap option.

Added option to overlay alive cells. Required more memory usage.

Brandon502 · 2024-02-26T02:38:09Z

I am unable to test the All Colors option. I tried to keep that option available, but can not confirm if it works correctly. Tested the new infinite pattern detection a ton. The crc method isn't perfect, as in it may take an extra loop or two to detect, but I have yet to see it fail.

The overlay option requires more memory to achieve. The first commit doesn't have overlay and uses roughly the same amount of memory as the original.

ewoudwijma · 2024-02-26T08:26:15Z

Hi Brandon, thx for this PR! Looks good from a first look at the code.
@softhack007 , can you also take a look?
Regards,
Ewoud

softhack007

Hi, here is a first set of review comments from my side. I'll take a deeper look during the week, so expect more 😉

@ewoudwijma I think we need to spend some thoughts on the memory footprint of this effect:
1/ its now using two LEDs arrays, one only needed for overlay mode
2/ I'm a bit concerned about alignment of allocated data - sizeof(CRGB) == 3, so everything behind the LEDs array will not be word-aligned any more.

Not sure however how to make the code more robust.

@Brandon502 about point 2, would it be possible to move "repeatDetection" to be the first item thats allocated by SEGENV.allocateData? Or could you also use an array of uint8_t ?

My summary: I think we need to spend some more thoughts on the memory allocation for this effect. I hope the new code can be made more robust, still readable and possibly a bit more memory-efficient. And yes for sure these objectives are in conflict 😄

softhack007 · 2024-02-26T09:53:59Z

wled00/FX.cpp

  if (!strip.isMatrix) return mode_static(); // not a 2D set-up

  const uint16_t cols = SEGMENT.virtualWidth();
  const uint16_t rows = SEGMENT.virtualHeight();
  const uint16_t dataSize = sizeof(CRGB) * SEGMENT.length();  // using width*height prevents reallocation if mirroring is enabled
-  const uint16_t crcBufferLen = 2; //(SEGMENT.width() + SEGMENT.height())*71/100; // roughly sqrt(2)/2 for better repetition detection (Ewowi)
+  const uint16_t repeatDetectionLen = 4; // {crc % 16 gen, crc % 4*r*w gen, prevAlive, changeCount}


I would prefer that you define a struct for repeatDetection. Its very confusing to only have the array indices.

Edit: to be discussed - if repeatDetection becomes a struct, it should be the first part of allocated data, otherwise you'll get alignment problems depending on the number of LEDs. Maybe its better to define macros for array indices, so you can use repeatDetection[GOL_PRE_ALIVE] instead of repeatDetection[2]

Hey @ewoudwijma & @softhack007. I replied earlier, but I don't believe I submitted correctly. Sorry if this is a duplicate message. repeatDetection can be structured however you prefer. I was just trying to keep the code's structure similar. repeatDetection should have been resetting at 4max(rc) generations not 4rc. 4rc is overkill and 'should' detect every possible infinite pattern at this scale. I will fix and retest, tracking prevAlive may not be necessary.

Regarding memory. I'm working on a version that will use 1 bit of memory per cell vs 3 bytes currently (6 bytes for overlay version). Dominant color won't work perfectly in this version but I'll test a close alternative. Overlay option may be limited to a single color.

wled00/FX.cpp

Each cell now only needs 2 bits of data instead of 6 bytes.

Brandon502 · 2024-02-27T01:19:14Z

@softhack007 I committed a low memory version. I focused on getting memory down, everything appears to all function the same. Dominant color is affected slightly but not really noticeable. This version uses 2 bits per cell/pixel. Without the overlay option it is just 1 bit. I added two functions to help get and set bit values. I didn't know if there were anything similar already built in so I just made my own for testing. I haven't reworked repeatDetection yet.

Simplified repeat detection and code cleanup. Only storing 2 crc values. prevAlive and counter no longer needed.

Fixed small bug in new detection method. Start and final frames are displayed slightly longer.

ewoudwijma · 2024-02-29T07:46:50Z

Hi Brandon, thx for your great work on GOL! I am a bit busy so look from the sideline, but happy to test it out soon!

softhack007 · 2024-05-07T19:32:47Z

Hi Brandon, sorry we kept you waiting for so long...
We have decided to add your effect into the next MM release, which is scheduled for May 15th (tentatively).

I think that you did a great job in reducing the memory footprint, thanks 👍

So we'll try to do some testing this week. I'll make a start with some new review comments.

softhack007 · 2024-05-07T19:37:56Z

wled00/FX.cpp


+  uint16_t &generation = SEGENV.aux0;
+  uint16_t &pauseFrames = SEGENV.aux1;


using reference pointers here means that any modification to "generation" will also update SEGENV.aux0? It's a bit unusual for me as you've basicially created an alias. Would be nice to have a code comment to explain it.

softhack007 · 2024-05-07T19:41:02Z

wled00/FX.cpp

-} colorCount;
-
-uint16_t mode_2Dgameoflife(void) { // Written by Ewoud Wijma, inspired by https://natureofcode.com/book/chapter-7-cellular-automata/ and https://github.com/DougHaber/nlife-color
+bool getBitValue(const uint8_t* byteArray, size_t arraySize, size_t n) {


Looks like arraySize is not used? If the function is not too time-critical, it might be worth to add a sanity check like if (byteIndex >= arraySize) return 0;

softhack007 · 2024-05-07T19:41:24Z

wled00/FX.cpp

+    uint8_t byte = byteArray[byteIndex];
+    return (byte >> bitIndex) & 1;
+}
+void setBitValue(uint8_t* byteArray, size_t arraySize, size_t n, bool value) {


Another thing : I think that your helper functions could be made static?

@softhack007 I'll look into modifying the helper functions. I had a more recent version that used a struct to store all cell values along with crc and other values to make the code more organized, I never pushed it to github and I can't seem to find it locally.

Uses struct to store values. Changed glider check method.

Brandon502 · 2024-05-07T20:01:48Z

@softhack007 found and pushed the updated struct version. I still need to look at the helper functions and add comments on the pauseFrames and generations aliases. Also need to incorporate a fail safe check that I have run into on a 24x16 matrix that can produce an insanely long repeating pattern that is undetected.

Helper functions are now static and the arraySize parameter was removed. Added failsafe for repeat patterns.

Brandon502 · 2024-05-07T21:36:48Z

Made the helper functions static and removed arraySize which wasn't used. Added comment regarding aux0 and aux1. This alias can be removed if needed, was just to help improve readability while coding. Fail safe was added to disable wrap on every generation with a multiple of 1500. This is a very niche case that will likely only apply to very specific rectangle matrices. I have already seen this fail safe work on a 24x16 display.

Brandon502 · 2024-05-07T21:38:25Z

20240507_172218.mp4

Quick example video showing game of life overlayed on rainbow background. You can see the game pause briefly at the end of a game and start.

ewowi · 2024-05-10T08:52:15Z

I tested latest commit (by migrating it to StarLeds ;-)

MoonModules/StarLight@41b7c7c

My comments / changes /questions, although you should read it in StarLeds context:

StarMod specific simplifications

No matrix testing
No struct as StarMod has a simpler data bind mechanism (?)
Used leds.size.x/y instead of row/col
Aux0,1, step, etc in data bind
No overlay functionality in effects

WLED specific

No struct might be needed if done in binding ???
Added fill black color at start/reset
Used CRGB instead of RGBW32
Byte instead of uint8_t for (future)cells
set/getBitValue simpler possible if using bytes?
Use byte type in crc (call)?

Result is not as it should but probably something went wrong in conversion (too many pixels coloured ???)

Screen.Recording.2024-05-10.at.10.42.24.mov

Any idea what could cause this?

ewowi · 2024-05-10T09:37:26Z

Regarding a more readable solution for a bit array:

http://www.gotw.ca/gotw/050.htm

vector(bool) uses one bit per bit !

this might be considered as it is more readable, no set/get functions needed and ‘maybe’ it’s faster.

so a nice to have , but in case of low performance worth the try

ewowi · 2024-05-10T10:31:21Z

Memcpy third parameter is Number of bytes to copy.

so don’t you copy 8 times as much ?

… okay, I got confused by sizeof(byte). As sizeof returns the number of bytes, you are asking how many bytes is a byte which in this universe is always one isn’t it ? So we could remove sizeof(byte)?

ewowi · 2024-05-10T12:34:24Z

And what about pauseFrames?
Can you explain what it does what normal return FRAMETIME is not doing?
(It looks to me that an effect should not manage their timings but the effect system should do)

Brandon502 · 2024-05-10T15:35:40Z

And what about pauseFrames? Can you explain what it does what normal return FRAMETIME is not doing? (It looks to me that an effect should not manage their timings but the effect system should do)

This was one of the things I added last and it can definitely be done in a less confusing manor. It's basically stops the game from ending/starting and allows you to see the starting and ending pattern before it starts/resets. Pauses updates for about a second. Before this the game immediately restarted a new game which made knowing when a new game started and debugging difficult. It was added in a way that game speed didn't effect how long the pause was. I can't think of a way to implement this without an extra variable, any suggestions?

Brandon502 · 2024-05-10T15:42:42Z

Memcpy third parameter is Number of bytes to copy.

so don’t you copy 8 times as much ?

… okay, I got confused by sizeof(byte). As sizeof returns the number of bytes, you are asking how many bytes is a byte which in this universe is always one isn’t it ? So we could remove sizeof(byte)?

Yeah sizeof(byte) seems redundant. Didn't catch that when I converted everything to use bits.

Regarding a more readable solution for a bit array:

http://www.gotw.ca/gotw/050.htm

vector(bool) uses one bit per bit !

this might be considered as it is more readable, no set/get functions needed and ‘maybe’ it’s faster.

so a nice to have , but in case of low performance worth the try

I haven't run into performance issues yet with current implementation, but I've only tested on esp32s. This may be an option, I can't really wrap my head on how to implement it yet though.

Brandon502 · 2024-05-10T15:59:00Z

I tested latest commit (by migrating it to StarLeds ;-)

MoonModules/StarLeds@41b7c7c

My comments / changes /questions, although you should read it in StarLeds context:

StarMod specific simplifications
* No matrix testing
* No struct as StarMod has a simpler data bind mechanism (?)
* Used leds.size.x/y instead of row/col
* Aux0,1, step, etc in data bind
* No overlay functionality in effects

From your video it looks like dead cells are not disappearing. I'll have to look at the code and test it, but it looks like something may have gone wrong when removing overlay functionality. You can get the same effect in wled if you enable overlay without a background segment.

WLED specific

* No struct might be needed if done in binding ???

The version before this didn't use a struct and just had different variables. Either method is fine with me. Struct seems a bit more organized/readable.

* Added fill black color at start/reset

Fill black I don't believe is needed. When the game starts if overlay is off it sets dead cells to the background color.

* Used CRGB instead of RGBW32

Not sure if this is referencing wled or not, but I can swap to CRGB instead of RGBW32. I kept all of your RGBW code to keep the all colors option available.

* Byte instead of uint8_t for (future)cells
* set/getBitValue simpler possible if using bytes?
* Use byte type in crc (call)?

I'm not sure the differences between uint8_t and byte so this change may be better.

Brandon502 · 2024-05-10T16:17:27Z

MoonModules/StarLeds@41b7c7c

@ewowi line 1440 needs to be converted to something like leds.setPixelColor(leds.XY(x,y), backgroundColor);
Haven't tested it yet, but that is likely causing it to misbehave.

Edit: Tried testing on an esp32, but the fixture preview window does not work on my end. The pin viewer works though.

Brandon502 · 2024-05-10T19:08:50Z

GoL63x63.mp4

Stress tested on large matrices. Seems to work up to 63x63, hard to tell performance through live view though. 64x64 causes a integer division by 0 error.

softhack007 · 2024-05-10T21:17:33Z

Used CRGB instead of RGBW32

Not sure if this is referencing wled or not, but I can swap to CRGB instead of RGBW32. I kept all of your RGBW code to keep the all colors option available.

Hi, I think for WLED-MM it's better to use RGBW32. We support white channel strips (RGBW) and CRGB cannot handle 4 color channels.

ewowi · 2024-05-11T20:10:05Z

Hi Brandon, your code is fine, no need to change anything, except remove sizeof(byte) as it is always 1.
And you are right, I got it working with your suggestion 🙏

Softhack,
Rgbw is fine too, although the fourth parameter is 0 but leave it as is.

my comments where mainly valid in a StarMod context.

so, if softhack is fine, and saw it working 🙂we can merge it

Removed sizeof(byte)

Brandon502 · 2024-05-11T21:09:04Z

Hi Brandon, your code is fine, no need to change anything, except remove sizeof(byte) as it is always 1. And you are right, I got it working with your suggestion 🙏

Softhack, Rgbw is fine too, although the fourth parameter is 0 but leave it as is.

my comments where mainly valid in a StarMod context.

so, if softhack is fine, and saw it working 🙂we can merge it

Hey, glad it is working in StarMod. Now you just have to edit the neighbor loop to allow 3D cubes :D. Wish I could figure out how to get it working properly on my end so I can play around with things. Can't connect WiFi and preview feature doesn't work.

ewowi · 2024-05-12T08:21:54Z

@Brandon502 ,

Yeah 3D would be a think, also a giant cube needed !!! What is the largest ws2812 3D Cube existing?

There was a guy called Trackerr on Discord who reported something similar, which I just turned into an issue: ewowi/StarBase#55. Is this related? Please add to the issue if you have a different situation

Brandon502 · 2024-05-14T03:25:34Z

@Brandon502 ,

Yeah 3D would be a think, also a giant cube needed !!! What is the largest ws2812 3D Cube existing?

There was a guy called Trackerr on Discord who reported something similar, which I just turned into an issue: ewowi/StarBase#55. Is this related? Please add to the issue if you have a different situation

Largest cube I've seen is just the 16x16 ws2812b panels. The bigger panels just get too expensive to buy multiple of. Just tried starleds again on a different esp32 and no dice. I'll make a post there.

ewoudwijma · 2024-05-17T11:24:15Z

Hi @Brandon502 ,

I see quite often that gol will repeat itself endlessly.
Is that by design?

trim.45DA33E0-E7D6-48B3-BA98-A67907C75AC7.MOV

Brandon502 · 2024-05-17T13:34:49Z

Hi @Brandon502 ,

I see quite often that gol will repeat itself endlessly. Is that by design?

trim.45DA33E0-E7D6-48B3-BA98-A67907C75AC7.MOV

I played with this last night. Starmod version is pretty broken lol. I'm slowly trying to debug it. Everything looks right in the code so it's frustrating. One thing is that generation is not actually being increased by generation++; changing to generation += 1; fixes that. No clue why. Without generation increasing repeat detection is basically disabled.

ewowi · 2024-05-17T13:46:58Z

Ah, sorry for the inconvenience 🤭, reason is that generation is a bind var so is a pointer so to increase it we need to do (*generation)++ instead of *generation++.

I changed that and now it is much better! (MoonModules/StarLight@940e91a)

PS: you have to set blending to 0 to have pixels fading away - This is the way of overlaying in StarLeds - idea is to do that outside effect code, but I need to think about it a bit more (or if you have ideas ;-) )

Brandon502 added 2 commits February 25, 2024 21:06

Game of Life - Rework

7b89098

No longer uses ColorCount struct. Removed randomness. Improved infinite pattern recognition. Adds color mutation slider and wrap option.

Game of Life - Added overlay option

ce1b811

Added option to overlay alive cells. Required more memory usage.

softhack007 requested changes Feb 26, 2024

View reviewed changes

softhack007 reviewed Feb 26, 2024

View reviewed changes

wled00/FX.cpp Outdated Show resolved Hide resolved

Game Of Life - Reduced Memory Usage

73091d1

Each cell now only needs 2 bits of data instead of 6 bytes.

Brandon502 added 2 commits February 28, 2024 15:20

Game of Life - changed repeat detection

ba57316

Simplified repeat detection and code cleanup. Only storing 2 crc values. prevAlive and counter no longer needed.

Game of Life - Bug fix

987a662

Fixed small bug in new detection method. Start and final frames are displayed slightly longer.

Brandon502 requested a review from softhack007 March 6, 2024 22:27

softhack007 reviewed May 7, 2024

View reviewed changes

Game of Life changes

9b3f644

Uses struct to store values. Changed glider check method.

Game of Life changes

86a18ce

Helper functions are now static and the arraySize parameter was removed. Added failsafe for repeat patterns.

Game of Life Change

6d2fc50

Removed sizeof(byte)

ewoudwijma merged commit 83d3f27 into MoonModules:mdev May 16, 2024
34 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Game of Life Rework #120

Game of Life Rework #120

Brandon502 commented Feb 26, 2024

Brandon502 commented Feb 26, 2024 •

edited

Loading

ewoudwijma commented Feb 26, 2024

softhack007 left a comment

softhack007 Feb 26, 2024

Brandon502 Feb 26, 2024

Brandon502 commented Feb 27, 2024

ewoudwijma commented Feb 29, 2024

softhack007 commented May 7, 2024

softhack007 May 7, 2024

softhack007 May 7, 2024

softhack007 May 7, 2024

softhack007 May 7, 2024

Brandon502 May 7, 2024

Brandon502 commented May 7, 2024

Brandon502 commented May 7, 2024

Brandon502 commented May 7, 2024

ewowi commented May 10, 2024 •

edited

Loading

ewowi commented May 10, 2024 •

edited

Loading

ewowi commented May 10, 2024 •

edited

Loading

ewowi commented May 10, 2024

Brandon502 commented May 10, 2024

Brandon502 commented May 10, 2024

Brandon502 commented May 10, 2024

Brandon502 commented May 10, 2024 •

edited

Loading

Brandon502 commented May 10, 2024

softhack007 commented May 10, 2024

ewowi commented May 11, 2024

Brandon502 commented May 11, 2024

ewowi commented May 12, 2024

Brandon502 commented May 14, 2024

ewoudwijma commented May 17, 2024

Brandon502 commented May 17, 2024

ewowi commented May 17, 2024 •

edited

Loading


		uint16_t &generation = SEGENV.aux0;
		uint16_t &pauseFrames = SEGENV.aux1;

Game of Life Rework #120

Game of Life Rework #120

Conversation

Brandon502 commented Feb 26, 2024

Brandon502 commented Feb 26, 2024 • edited Loading

ewoudwijma commented Feb 26, 2024

softhack007 left a comment

Choose a reason for hiding this comment

softhack007 Feb 26, 2024

Choose a reason for hiding this comment

Brandon502 Feb 26, 2024

Choose a reason for hiding this comment

Brandon502 commented Feb 27, 2024

ewoudwijma commented Feb 29, 2024

softhack007 commented May 7, 2024

softhack007 May 7, 2024

Choose a reason for hiding this comment

softhack007 May 7, 2024

Choose a reason for hiding this comment

softhack007 May 7, 2024

Choose a reason for hiding this comment

softhack007 May 7, 2024

Choose a reason for hiding this comment

Brandon502 May 7, 2024

Choose a reason for hiding this comment

Brandon502 commented May 7, 2024

Brandon502 commented May 7, 2024

Brandon502 commented May 7, 2024

ewowi commented May 10, 2024 • edited Loading

ewowi commented May 10, 2024 • edited Loading

ewowi commented May 10, 2024 • edited Loading

ewowi commented May 10, 2024

Brandon502 commented May 10, 2024

Brandon502 commented May 10, 2024

Brandon502 commented May 10, 2024

Brandon502 commented May 10, 2024 • edited Loading

Brandon502 commented May 10, 2024

softhack007 commented May 10, 2024

ewowi commented May 11, 2024

Brandon502 commented May 11, 2024

ewowi commented May 12, 2024

Brandon502 commented May 14, 2024

ewoudwijma commented May 17, 2024

Brandon502 commented May 17, 2024

ewowi commented May 17, 2024 • edited Loading

Brandon502 commented Feb 26, 2024 •

edited

Loading

ewowi commented May 10, 2024 •

edited

Loading

ewowi commented May 10, 2024 •

edited

Loading

ewowi commented May 10, 2024 •

edited

Loading

Brandon502 commented May 10, 2024 •

edited

Loading

ewowi commented May 17, 2024 •

edited

Loading