New RNG

[VanNostrand]

Here is the new RNG I talked about in the forums and I also replaced all rand() calls from gemrb core to use the new RNG:

rand % foo becomes RAND(0, foo-1)
rand & foo becomes RAND(0, foo)

I'd also like to replace the following rand() calls from the plugins:

./gemrb/plugins/CREImporter/CREImporter.cpp:                            stat=randcolors[i][rand()%RandRows];
./gemrb/plugins/CREImporter/CREImporter.cpp:                            stat=randcolors[i][rand()%RandRows];
./gemrb/plugins/FXOpcodes/FXOpcodes.cpp:                        fx->Parameter1 = rndstr->at(rand()%rndstr->size());
./gemrb/plugins/GUIScript/GUIScript.cpp:                                int i=rand()%5;
./gemrb/plugins/GUIScript/GUIScript.cpp:        if (rand()%100 >= percent) {
./gemrb/plugins/OpenALAudio/AmbientMgrAL.cpp:   int index = rand() % soundrefs.size();
./gemrb/plugins/PSTOpcodes/PSTOpcodes.cpp:                      ieWord tmp =(ieWord) rand();

but as soon as I do it, I get undefined references to the RNG class from the linker, though it is included in libgem_core.so after building. This is probably a CMake thing.

[lynxlynxlynx]

On what platforms did you try to compile this?

We don't use exceptions, I think mostly due to msvc6 dying on them. Use some other fallback instead.

[VanNostrand]

What is not equivalent to what? [edit: I found the specific comment and answered there]
rand(0,0) = 0, because it is the only number from zero to zero. Where does this break the generator? [edit: dito]

I compiled it on Linux and (not gemrb but in another project) on Windows 7 with VS2010.
VC6 is... really... really old... As MS is giving out new compilers for free, I don't know if I would still support VC6. But if you want to have this code part changed anyways, there are some possibilities:

1. Assertions instead of exceptions. Problem: only functional in debug mode.
2. Ostrich algorithm. C++ still uses exceptions, so if if a division by 0 happens, an exception is thrown and the game crashes anyways. Finding out details needs a reproducable case and debugging then (that's how I found about e0a63ea). The problem could be logged though, I saw something like a Log() function in gemrb, can this be used?
3. Both.
   How do you like it? Assertion + log + cash for debug and log + crash for release?

[lynxlynxlynx]

Sure, you can use error(). MSVC6 is old, of course, but one of our main sages is using it still.

[lynxlynxlynx]

I wanted to merge this, but it seems to be causing corruption somewhere. I can pretty easily get it to crash in our (tile overlay) drawing code, but a valgrind run revealed nothing. Very fishy.

[VanNostrand]

I remember that I was able to load a BGT savegame with that and everything seemed to work so far. How do you produce the crash?

[lynxlynxlynx]

combat, but not immediately, so the exact trigger still evades me.

Oh, isn't the stdexcept include is now rendundant, with the exception gone?

[VanNostrand]

I created a new game, entered combat and also experience segfaults unless I kill the enemy with one hit. I opened the core dump and it tells me that the binary was unable to access some memory at plugins/SDLVideo/TileRenderer.inl:144
This is odd but I am also checking what is going on.

[lynxlynxlynx]

Good, you get the exact same problem. One thing I noticed is that many times the viewport jumped just before that, so perhaps the Viewport var gets corrupted (viewport of frame 3 is fine). Looking further ...

[lynxlynxlynx]

Yep, scoring a critical hit seems to be a way to trigger it (but works fine without this pull). A screenshake is associated with that, but it's delayed one tick, so it wasn't immediately obvious. I guess the algo can't handle negative arguments? ShakeY can be negative ...

[lynxlynxlynx]

Yep yep, double confirmed, it's the changes in GlobalTimer.cpp that are causing this. Since the shake can be ordered from scripts, we can't guarantee the params will always be positive, so I suggest two things:

• use abs there
• add a assert into the main call, so it triggers whenever min or max is negative when cast to signed (or a dumber hardcoded limit of a few thousand)

[VanNostrand]

I see... I did not expect negative values and as signed values are converted to congruent unsigned values, it has some unintended behaviour (like... crashing ^^).
I will modify the code to handle negative values.

[VanNostrand]

I renamed the random function from before and introduced a random function that handles the parameters first.
I tested it with (vanilla-gog) BG2: character creation, savegame, battle (including critical hits/misses) and dialogue worked and produced no crashes so far.

[lynxlynxlynx]

Thanks!