ClumpPlacer seems to use incorrect formula to determine if placement failed for a given failFraction allowed.

[Inari]

To check if placement has failed, ClumpPlacer allows to pass a failFraction to allow partial placement to still count as success. (ps/trunk/binaries/data/mods/public/maps/random/rmgen/placer/centered/ClumpPlacer.js#L103​)

The formula to check if placement has failed seems a bit off though. It keeps track of a failed variable, increasing that by 1 each time a point fails to put.

In a quick test for an area with size 1200 there seem to be roughly 13500~15500 points that it attempts to put. It varies a bit because of the randomness involved in the placement.

So it can't just check that failed variable against the size multiplied by failFraction. It should instead keep track of how many points it tried to put and multiply that count by failFraction to check against failed.

ChainPlacer seems to do this already (ps/trunk/binaries/data/mods/public/maps/random/rmgen/placer/centered/ChainPlacer.js#L110​).

This is also why #4810 seemed to be an issue, originally it passed a failFraction of 1, allowing for placement to proceed in any case. But that seems to not have worked as intended.

[Inari]

I created a patch for this as I ran into it again while trying to work on another ticket.

[Itms]

Thanks for the patch! And sorry for our clogged review queue. We shouldn't take much longer in committing it.

[wraitii]

Famous last words™

[fsincos]

The patch is correct in that it actually uses a fraction of the positions that were checked, but it still doesn't check the "ineligible" fraction of the area:

At the center, the point density is (a lot) greater than at the perimeter. For instance, the center point is counted at least intPerim times (if it is "invalid").

In total, every point in the circle is checked (on average) eight times(*), so that explains some part of the discrepancy (1200 -> 9600). The rest is caused by the "random part" (if I understood the code correctly, this part yields an expected factor of 2 - coherence, something like 9600 -> ~14000, I guess).

To fix these issues, the "gotRet" part should also be used for the "else" branch.

We still need the actual count of tiles as the "size" doesn't accurately reflect it because of randomness (and also parts being cut off by the map's edges - should those tiles count as invalid or should they just be ignored?).

(*)

perim * radius = 8 * PI * radius * radius = 8 * size

[Stan]

In 26503:

Make ClumpPlacer determine the correct fraction of failed attempts to place points
Comments by: @smiley, @elexis
Patch by: @Inari
Fixes #5092
Differential Revision: ​https://code.wildfiregames.com/D1414