List Shuffle Fix & Test

Reporter: RobertBColton  |  Status: open  |  Last Modified: March 19, 2020, 11:38:32 PM

Updated the CI test I just added for data structures to prevent #1107 from coming back. I wasn't sure if I could get this to work deterministically, that's why It was not included in #1650 with my other list tests. Apparently I can test it out reliably, hence I am submitting this pull request.

The documentation does explicitly state that list shuffle should be connected with the game's random seed.

NOTE: This function will shuffle the list items to the same positions every time the game is run afresh due to the fact that GameMaker: Studio generates the same initial random seed every time to make debugging code a far easier task. To avoid this behaviour use randomize at the start of your game. This is only true when testing and debugging the game, as the final executable package will not show this behaviour and will be random every play.
https://docs.yoyogames.com/source/dadiospice/002_reference/data%20structures/ds%20lists/ds_list_shuffle.html

RobertBColton  
No, you didn't and the CI is actually full of non-fatal errors about that. But yeah, thanks for putting the kibosh on this, you are right that #1324 is a blocker to merging this.
RobertBColton  

Ok, now that #1706 is in, this should pass deterministically.
RobertBColton  

Alright, I ran onto a bug in the shuffle function, it's never moving the last element of the list. Once I fix it, I also know how to regression test that.

var list_str;
list_str = ds_list_create();

ds_list_add(list_str, "bank");
ds_list_add(list_str, "alarm");
ds_list_add(list_str, "cash");

for (var i = 0; i < 6; ++i) {
	random_set_seed(69378);
	ds_list_shuffle(list_str);
	show_message(string(ds_list_find_value(list_str, 0)) + " " +
				 string(ds_list_find_value(list_str, 1)) + " " +
				 string(ds_list_find_value(list_str, 2)));
}

codecov[bot]  

Codecov Report

Merging #1654 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1654      +/-   ##
==========================================
+ Coverage   28.32%   28.34%   +0.02%     
==========================================
  Files         167      167              
  Lines       17023    17023              
==========================================
+ Hits         4821     4825       +4     
+ Misses      12202    12198       -4
Impacted Files Coverage Δ
...stem/Extensions/DataStructures/data_structures.cpp 51.03% <100%> (+0.41%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c64307...8ede607. Read the comment docs.

RobertBColton  

The bug I mentioned occurs because the user random functions are inclusive of x which goes against the docs for random_shuffle.
http://www.cplusplus.com/reference/algorithm/random_shuffle/

The function shall return a non-negative value

Please sign in to post comments, or you can view this issue on GitHub.