lronaldo / cpctelera

Astonishingly fast Amstrad CPC game engine for C developers
http://lronaldo.github.io/cpctelera/
GNU Lesser General Public License v3.0
229 stars 54 forks source link

cpct_rand and cpct_srand not working as expected #36

Closed ghost closed 8 years ago

ghost commented 8 years ago

Hello.

If I modify the random number generator in examples/easy to always use seed = 192, I only get the same sequence every two calls to cpct_rand(). The other one adds two numbers at the beginning of the sequence.

void initialize() {
   u32 seed;    // Value to initialize the random seed

   // Introductory message
   printf("\017\003========= BASIC RANDOM NUMBERS =========\n\n\r");
   printf("\017\002Press any key to generate random numbers\n\n\r");

   // Wait till the users presses a key and use the number of
   // passed cycles as random seed for the Random Number Generator
   seed = wait4UserKeypress();

   // Random seed may never be 0, so check first and add 1 if it was 0
   if (!seed)
      seed++;

   seed = 192;

   // Print the seed and seed the random number generator
   printf("\017\003Selected seed: \017\002%d\n\r", seed);
   cpct_srand(seed);
}

If I also modify the number of random numbers to generate from 50 to 52 I always get the same sequence when calling cpct_rand().

I think it is not working as expected.

lronaldo commented 8 years ago

Yes, it's kind of a bug. The thing is that random number generator has a internal 32-bits state (4 bytes). Setting the seed sets the value of that state, but does not set the index of the next group of 8-bits that will be returned as random. The generator only generates 32-bits values once every 4 calls. Rest of the time, it is returning one of the 4 8-bit groups.

Therefore, when you generate 52 numbers, which is divisible by 4, you are positioning the generator at the first element again for the next sequence, which always gives same sequence. Other values leave the sequence to start at indexes 1, 2 or 3, instead of at 0 index.

Tecnhically you are right. This is not the desired behaviour. You can simulate desired behaviour using cpct_nextRandom_mxor_u8 along with a 32 bits state that you can define yourself with a u32. Then, initialize your u32 seed to the value you want (except 0) and call cpct_nextRandom_mxor_u8 for generating every new 8 random bits. The 8 least significant bits from your value are the random ones, so you can do a simple cast to u8 to get the random value.

I'll change behaviour from cpct_rand and cpct_srand soon.

lronaldo commented 8 years ago

Fixed by commit c03c34bdd231998bd8d32ade2894ade9e6685cd5

ghost commented 8 years ago

Thanks for the super fast fixing.

lronaldo commented 8 years ago

No problem, I already knew that was there, so I had it in mind. Should have done it previous to release.

If you find anything else, please report again :).

ghost commented 8 years ago

Hello again.

I am not sure it is something related, but I am getting unexpected behavior in my game, after updating to this patch.

Depending on the value I use with cpc_srand(), my program hangs, corrupts graphics or does not call the game over function. Everything seems to work well if I don't call cpc_srand().

Do you think there is any problem with the patch? As I have modified other things in my program I am not sure the problem is here.

Thank you.

lronaldo commented 8 years ago

Did you recompile cpctelera library with make cleanall && make? Does not seem probable that this patch has anything to do, unless something was wrong with the linking due to previous object code being linked with new. Other possibility, if this is not the cause, is that something is causing a size effect that manifests on adding cpct_srand because that introduces new code and might position it in the place that's being modified.

Anyway, I'm checking this code for possible bugs, but does not seem probable.

ghost commented 8 years ago

In fact I downloaded and recompiled the whole CPCtelera. I cleaned and recompiled my project.

Don't waste time on this, let me find an example and if I can demonstrate it is caused by this function I will let you know.

Thanks.

ghost commented 8 years ago

Sorry, it was my fault.

There was an uncontrolled situation where I could overflow a buffer depending on the random generated value.

Regards.

lronaldo commented 8 years ago

No problem, man. That happens constantly to all of us. The important thing is being able to get it and fix it :smile: