@wraitii This year maybe ? ;)

Binary files should be removed from the diff :)

Needs to be rebased.

Might want to commit the whitespace changes separately :)

• Initialize private member before using it.

@vladislavbelov Can you recheck that everything is fine so I can commit it ?

• Fix incorrect function call
• Remove deleted whitespace

• Use the fast random function
• Make function and variables members of the SoundGroup.cpp class.
• Make them non static as it doesn't matter as they are constantly overridden.

That works however I'm not sure what are the bounds of the generated number. I tried to divide it by std::limits<int>::max() but I only get 10^-6 numbers.

@wraitii Any idea on what could be faster than a MT19973 ? srand() would have been perfect but we cannot use it.

Can you fix the JSDOC comment ? :)

@Itms @vladislavbelov Any other comments ?

In D1803#74329, @bb wrote:

changing vegetables to grain wouldn't work since f.e. the Chinese mod has rise fields instead. One could consider "crops" though.

The string but at reduced efficiency doesn't specify when the efficiency is reduced, so it doesn't help explaining what is going on. Maybe Harvest crops for food. Up to 5 units can gather, but each subsequent gatherer reduces the efficiency per worker.

In D1739#75033, @wraitii wrote:

This really needs to be used for components before I decide if it's worth merging, but as it changes hashes it's kind of annoying. I'll probably have to do some fancy tests at some point.

Any news on this ?

@wraitii usually the reviewer commits the patch. @Angen doesn't have commit access.

Yeah, it would be nice to have it for A24 but that's all.

boost::random::uniform_int_distribution & boost::random::uniform_real_distribution<float> → 1.47 https://www.boost.org/doc/libs/1_47_0/doc/html/boost/random/uniform_int_distribution.html Before it's not documented.

Well good thing that option doesn't exist because this means only enable-fat is needed.
I'm not sure that's an issue upstream more us supporting too many platforms with one build.

That you could already do. However what you couldn't do is have the same behavior clicking on the minimap

When creating your patch please add context using git diff -U5000 or svn diff -x -U5000, so that we can see the lines above and below :)

Fix years.

• Use srand(seed) instead. According to my tests it works way faster.

Will fix it asap for some reason the commit did not add all my changes... Thanks for the report.

@vladislavbelov Three questions

Closed the wrong differential.

Fixed by rP22175

Thanks for the review guys !

Well if distribution did matter we would use it everywhere no ?

That's a test so it doesn't matter much.

Thanks for the tip, I was able to make a new test (I had some troubles with static variables XD)

Maybe we should use boost after all https://stackoverflow.com/questions/20998470/random-numbers-c11-vs-boost

Here are some comments.

Fix include order.

Fix diff that should not have been able to build.

*Takes the artistic cap*

Maybe he can request changes :)

• Make rng static to cut the compute time by half
• Use a custom seed only when cmpVisual is set

Static rng;

Does setting the seed reset the rng ?

Can't really make it static since we always want to generate the same number for a given Actor seed.

https://pastebin.com/mFhKWCAu

1000000 Samples

Will try as long as I don't have to link boost should be fine :)

How should I test it ? I'm not really confident with profiling :)

In CPP casting to a smaller type will remove the leading byte.
https://stackoverflow.com/questions/6752567/casting-a-large-number-type-to-a-smaller-type

I don't think there is any benefit. Unless JavaScript does some weird cast when it's not there ?

Put header back on top

• Fix include order.

• Remove useless cast.