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

Improve random nick generation #3834

Merged
merged 3 commits into from
Nov 5, 2024

Conversation

Lpsd
Copy link
Member

@Lpsd Lpsd commented Oct 30, 2024

Fixes issue from #3777 where nickname parts could be "null" due to not updating the constants (which are now removed in place of better code).

@TracerDS
Copy link
Contributor

maybe add the random function to SharedUtils (SharedUtil.random.hpp?) and from there you can call that function in the nick generation func?

@shadylua
Copy link
Contributor

shadylua commented Oct 30, 2024

like :

#ifndef SHARED_UTIL_RANDOM_HPP
#define SHARED_UTIL_RANDOM_HPP

#include <random>

namespace SharedUtil {
    inline int RandomInt(int min, int max) {
        static std::random_device rd;
        static std::mt19937 gen(rd());
        std::uniform_int_distribution<int> dist(min, max);
        return dist(gen);
    }
}

#endif // SHARED_UTIL_RANDOM_HPP

@Lpsd
Copy link
Member Author

Lpsd commented Oct 30, 2024

You don't want to create a new generator for each part of the nickname. In-fact, we should probably share the generator between successive calls.

@TracerDS
Copy link
Contributor

Possibly a template there with pragma and optimized code but yeah, something like that

@TracerDS
Copy link
Contributor

You don't want to create a new generator for each part of the nickname. In-fact, we should probably share the generator between successive calls.

We can have a static (maybe const) generator that will be shared with related functions only.
That way it will not be generated again and the overhead would be minimal

@TheNormalnij TheNormalnij merged commit ad1da87 into multitheftauto:master Nov 5, 2024
6 checks passed
@Nico8340 Nico8340 mentioned this pull request Nov 17, 2024
1 task
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.

4 participants