In rP22197#32879, @elexis wrote:

but I don't really see the point (in terms of C++ code vs template cleanliness).

(I can't imagine that it would leave the C++ code in an uglier state)

Yup, and I think I've actually messed it up but I'll check with an upstream diff.

make the run multiplier optional and handle that specifically

That's what I was wondering about - whether there was a limit check in place before somewhere, I didn't find one from a quick look.

const-correct

(I guess the previous code was not incorrect, but adding the const adds benefits)

In rP22197#32874, @elexis wrote:

Formations have a 100 run multiplier which effectively sets their maximal walking speed at 100

https://code.wildfiregames.com/D438?id=7178#inline-34668

Sounds hackish

Indeed, why is that run multiplier 100 and not left undefined, or to be defined by the specific formations?

Formations have a 100 run multiplier which effectively sets their maximal walking speed at 100

https://code.wildfiregames.com/D438?id=7178#inline-34668

Sounds hackish

Indeed, why is that run multiplier 100 and not left undefined, or to be defined by the specific formations?

Very informative description by Angen that was used in the commit message, thanks for writing and chosing that.
(I hope you have examined possible edge cases)

Build failure - The Moirai have given mortals hearts that can endure.

@wraitii This year maybe ? ;)

Binary files should be removed from the diff :)

Successful build - Chance fights ever on the side of the prudent.

Needs to be rebased.

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Might want to commit the whitespace changes separately :)

Successful build - Chance fights ever on the side of the prudent.

Fix all whitespace I've regex-ed in water_high.fs, address vlad's comments.

Measured by time, after a make clean, running make pyrogenesis -o2 -j3

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Successful build - Chance fights ever on the side of the prudent.

Do not change minimal OSX version

const-correct, @Itms I'm putting this up for you to accept it.

Successful build - Chance fights ever on the side of the prudent.

• Initialize private member before using it.

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

Successful build - Chance fights ever on the side of the prudent.

• Fix incorrect function call
• Remove deleted whitespace

Build failure - The Moirai have given mortals hearts that can endure.

• 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.

Successful build - Chance fights ever on the side of the prudent.

I mean if we don't really care about actual randomness (which it seems we don't), maybe something from there: https://stackoverflow.com/questions/1640258/need-a-fast-random-generator-for-c
Like this should be super-fast and should be enough:

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 ? :)

as discussed on phab

concern fixed by rP20939

Successful build - Chance fights ever on the side of the prudent.

move comment to better place

Successful build - Chance fights ever on the side of the prudent.

This new method should be const. But apart from that this looks good to me.

@Itms @vladislavbelov Any other comments ?

In D1739#75035, @Stan wrote:

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.

When you committed last time it wasn't ?

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.

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.

Just update the diff, taking into account the suggestions raised in this discussion, then people can request further changes, or accept and commit this patch. The idea is to do things properly rather than quickly.

(wasn't there some earlier revision where it was discussed?)

Yes, D1036.

Any news on this ?

Cav got removed from ranges (which makes sense)

Thanks for the feedback guys, appreciated.