Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More SDL_rand fixes #10064

Merged
merged 3 commits into from
Jun 20, 2024
Merged

More SDL_rand fixes #10064

merged 3 commits into from
Jun 20, 2024

Conversation

JKaniarz
Copy link
Contributor

Renamed SDL_rand() to SDL_rand_bits to avoid confusion with std::rand()

Description

Deliberately removed SDL_rand() to force users to use SDL_rand_n() like they should. Added SDL_rand_bits() for the few who need it.

Existing Issue(s)

I was unable to built the tests on my machine. Please run them.

test/testdraw.c Outdated Show resolved Hide resolved
@slouken
Copy link
Collaborator

slouken commented Jun 19, 2024

What's the use case for SDL_rand_bits()?

@JKaniarz
Copy link
Contributor Author

What's the use case for SDL_rand_bits()?

Everything else we don't implement. Like rand_double(). Or rand_Sint64(). Or if someone wants to verify my testing with PractRand.

Copy link
Collaborator

@slouken slouken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, can you squash the test changes to a single commit, once the tests pass?

src/stdlib/SDL_random.c Outdated Show resolved Hide resolved
@icculus icculus changed the title Morerandfixes More SDL_rand fixes Jun 19, 2024
@JKaniarz
Copy link
Contributor Author

@slouken The two commits that didn't build happened because those files weren't referenced in the Xcode project (and ctrl-f didn't show them to me) Is that an issue with the project file?

Also, I believe this PR is all set.

@JKaniarz
Copy link
Contributor Author

From @madebr

src/dynapi/SDL_dynapi.sym still contains SDL_rand_r and is slightly unsorted around SDL_rand

@slouken
Copy link
Collaborator

slouken commented Jun 19, 2024

This looks good, can you squash the test changes to a single commit, once the tests pass?

test/testffmpeg.c Outdated Show resolved Hide resolved
@slouken slouken merged commit 8a80f41 into libsdl-org:main Jun 20, 2024
20 checks passed
@slouken
Copy link
Collaborator

slouken commented Jun 20, 2024

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants