mapbase-source / source-sdk-2013

This is Mapbase's public GitHub repo. It contains all of the code, but not the assets.
https://www.moddb.com/mods/mapbase
Other
224 stars 136 forks source link

[VSCRIPT] srand changes and seed function #246

Open celisej567 opened 1 year ago

celisej567 commented 1 year ago

srand() is useless since its literally empty. And its not very good that we dont have remapping function in vscript. Also we dont only want to generate ints, we want floats as well! So i made it!

Implementation

Here is how to do it:

  1. Go to common/randoverride.cpp and delete void __cdecl srand(unsigned int) entirely.
  2. Go to vscript/squirrel/sqstdlib/sqstdmath.cpp and add after #include <sqstdmath.h> this:
    #include "mathlib/mathlib.h"
    #include "vstdlib/random.h"
  3. After #define TWO_ARGS_FUNC declaration, add this:
    
    #define _FUNC(_funcname) static SQInteger math_##_funcname(HSQUIRRELVM v){ \
    sq_pushfloat(v,(SQFloat)_funcname()); \
    return 1; \
    }

void srand(unsigned int seed) { RandomSeed(seed); }

float randf() { return RandomFloat(0, VALVE_RAND_MAX); }

static SQInteger math_remap(HSQUIRRELVM v) { SQFloat p1, p2, p3, p4, p5; sq_getfloat(v, 2, &p1); sq_getfloat(v, 3, &p2); sq_getfloat(v, 4, &p3); sq_getfloat(v, 5, &p4); sq_getfloat(v, 6, &p5);

sq_pushfloat(v, (SQFloat)RemapVal(p1, p2, p3, p4, p5));
return 1;

}

4. Comment out entire `static SQInteger math_srand` function
5. Go a bit doen and after `SINGLE_ARG_FUNC(exp)` add:
```cpp
SINGLE_ARG_FUNC(srand)
_FUNC(randf)
  1. Go a bit down again and instead of this:

    _DECL_FUNC(srand,2,_SC(".n")),
    _DECL_FUNC(rand,1,NULL),

    replace this:

    _DECL_FUNC(srand,2,_SC(".n")),
    
    _DECL_FUNC(rand,1,NULL),
    _DECL_FUNC(remap,6,_SC(".nnnnn")),

    Changes

celisej567 commented 1 year ago

I spend about a month to understand how squirrel works and pretty proud that i figure out this.

mutezero commented 1 year ago

srand is empty in randoverride.cpp because that's the purpose of that file. Although some might've said that it could and probably should run RandomSeed instead of being completely empty. Either way it's not included in the vscript project, so it shouldn't affect it. Essentially vscript is using standard randomization, which doesn't necessarily require a replacement with Valve's, esp since there's no obvious gain from it.

srand is what initializes the (pseudo) random generator, "s" here stands for "seed". Replacing it with something else is counter productive. This fact renders the newly suggested randseed excessive and unnecessary.

map is not the best name, as it conflicts with a term used for data containers (think of a dictionary, but with any as a key, instead of strings, ie: enum<->value). If you really want to add this, give it the same name Valve gave it. And maybe add RemapValueClamped along the way, as i dont think there is Clamp function in squirrel.

celisej567 commented 1 year ago

srand is empty in randoverride.cpp because that's the purpose of that file.

What do you mean? Purpose of this file is rebind default functions with analog. And that fact that srand empty makes entire function useless. Of cource if srand will work as actual srand, then he should set RandomSeed and return random value as well, but if you will set the same seed, he will return the same vslue every time.

mutezero commented 1 year ago

What do you mean? Purpose of this file is rebind default functions with analog. And that fact that srand empty makes entire function useless.

That's the purpose of it, to make it not interfere with Valve's own RandomSeed, making sure it's not called from random places, such as third party inclusions. The sheer existence of this function is most likely a remnant from the old days when Valve's random generator was implemented differently, these days staying entirely within vstdlib (which is used in all valve's projects), instead of being part of the VEngineRandom001 interface (that'd require loading of the engine.dll, which is unreasonable for tools).

Also you've missed the follow up statement, where I said this override not affecting the vscript project, because it's not included. Even more, this file is only ever included in client, server, and engine projects. (EDIT: and srand is never used in any of them)

Also srand isn't meant to return anything.

PS: you're doing the "he" thing again :slightly_smiling_face:

celisej567 commented 1 year ago

Then i think the part with srand in randoverride can still be there. But im just wasnt sure, because when i pressed in srand with ctrl, vs showed me exacly this definition.

celisej567 commented 1 year ago

Ok then i will rewrite code a bit and remove randseed and rename map

mutezero commented 1 year ago
celisej567 commented 1 year ago

remap isn't really related to the random number generator, it should be moved into a separate task

It is, because we already have RAND_MAX that used for generating random numbers as maximum, and since we cant set maximum by yourself, we can remap what we have got instead.

like:

local rnd = rand(); //will return number from 0 to RAND_MAX
local mappedrand = remap(rnd, 0, RAND_MAX, 0, 100 ); //uses already existed RAND_MAX constant to remap value how needs to
celisej567 commented 1 year ago

I'm not sure what's the goal here with replacing of the entirety of math_srand

im just trying to make it look it understandable. but yeah, basically we can just make empty math_srand that will not write stack and will just call the function.

mutezero commented 1 year ago

since we cant set maximum by yourself, we can remap what we have got instead.

local mappedrand = rand() * 100 / RAND_MAX;
local mappedrand = randf() * 100;
celisej567 commented 1 year ago

Also ive made _FUNC define to make functions, that only returns value. Basically i can make it by hands, but later for each time for each command i will have to write new function from scrach, but with this define i can just put it in defile and do not care about it.

Im just making it more readable and less messy.

mutezero commented 1 year ago

im just trying to make it look it understandable

Hiding it behind macros wont make it easier to read, as the next person to work with it would have to also look up what the given macro is like.

celisej567 commented 1 year ago

ye, local mappedrand = rand() * 100 / RAND_MAX; will work fine i guess, but that looks more like crutch

mutezero commented 1 year ago

If you really want to get control over the rand's boundaries, you can implement optional params for it, by replacing the entire math_rand with the following:

static SQInteger math_rand(HSQUIRRELVM v)
{
    switch(sq_gettop(v))
    {
    case 1:
        sq_pushinteger(v,RandomInt(0,VALVE_RAND_MAX));
        return 1;
    case 2:
        {
            SQInteger i;
            if(SQ_FAILED(sq_getinteger(v,2,&i)))
                return sq_throwerror(v,_SC("invalid param"));
            sq_pushinteger(v,RandomInt(0,i));
            return 1;
        }
    case 3:
        {
            SQInteger p1,p2;
            if(SQ_FAILED(sq_getinteger(v,2,&p1)) || SQ_FAILED(sq_getinteger(v,3,&p2)))
                return sq_throwerror(v,_SC("invalid param"));
            sq_pushinteger(v,RandomInt(p1,p2));
            return 1;
        }
    default:
        return sq_throwerror(v,_SC("wrong number of parameters"));
    }
}

The same can be done for randf.

Don't forget to change _DECL_FUNC(rand,1,NULL), to _DECL_FUNC(rand,-1,_SC(".nn")), in mathlib_funcs down below.

Also I'd suggest changing sq_pushinteger(v,RAND_MAX); to sq_pushinteger(v,VALVE_RAND_MAX); at the bottom as well, for the reasons mentioned above the VALVE_RAND_MAX definition.