DescentDevelopers / Descent3

Descent 3 by Outrage Entertainment
GNU General Public License v3.0
2.74k stars 231 forks source link

Add new C++ Random functions to replace old C ones. #390

Open Nakhr11n opened 1 month ago

Nakhr11n commented 1 month ago

Pull Request Type

Description

There are three uses of randomness in the codebase:

In all cases some variant of the C rand() function is used. In many cases this results in code that is very hard to read, which makes it difficult to understand in what range the randomness will lie.

I propose to add these three functions which add modern C++ equivalents to rand() for the three use cases. The new functions are faster, safer and clearer.

My thinking is to then go over the code and replace all uses of randomness with these functions, and then remove misc/psrand.cpp and lib/psrand.h.

Related Issues

Screenshots (if applicable)

Checklist

Additional Comments

bryanperris commented 1 month ago

Is this still work in progress, if so, can you make a draft instead?

Nakhr11n commented 1 month ago

@bryanperris My thinking was to gauge interest with this initial PR. And then to submit follow-up PRs that actually replace the old random code.

Should I split it up, or should I do everything in this PR?

winterheart commented 1 month ago

I'd recommend place it to misc directory and then reuse it as part of misc module. In that way we can gracefully replace old psrand() generator.

Please note, that psrand implementation uses some magic numbers on generating "random" numbers:

int32_t ps_rand(void) { return (((ps_holdrand = ps_holdrand * 214013L + 2531011L) >> 16) & 0x7fff); }

I cannot say if this really used in order to receive predictable random sequences, but it would be great if we somehow preserve this approach.

bryanperris commented 1 month ago

We can keep this to a single PR

GravisZro commented 1 month ago

Personally, I prefer the simplicity of srand and rand. Also, I found these when working on RAND_MAX overflow issue:

If you can make the existing D3 code more readable with some function shortcuts then I'm in favor of it though I'm not sure the PRNG needs work. From what I can tell, the existing code (ps_rand) is simplified explicitly to be faster than the lib C rand.

Lgt2x commented 1 month ago

Have you investigated the use cases of random functions in the game? I'm afraid that changing random functions used (and thus potentially distribution of values) could have side-effects affecting game play that are difficult to predict.