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

very oblong structures

The long siege wall segment is very oblong (36×4, i.e. 9:1), but I'm not talking about that; only structures that have local auras matter for this patch. The ptol and spart temples have obstructions of 17.5×35.5, which is less oblong (2:1) yet not much shorter than the previous example.

this is an unfortunate side effect, but I don't think it's really such a common problem that we need to revert auras to being "distance to centre".

The old situation certainly wasn't perfect, but this “side effect” from the new approach makes things a lot worse, at least for auras. Local auras have radii of up to 100. The largest structure in game is the pers wonder (obstruction of 59×59, i.e. radius 42), but the largest buildable structure with a local aura is the ptol and spart temple (obstruction of 17.5×35.5, i.e. radius 20). One can see obstruction radii are clearly smaller than aura radii, but more importantly, obstruction areas are much smaller than aura areas (621 vs 5027). So the old situation wasn't too bad, for aura areas.

And I don't think this disagreement can be settled by maths

Looking at numbers alone is rarely a solution in itself, nonetheless, using maths to understand the behaviour is helpful.

we probably should listen to player and moder feedback at large, I'm not sure what the best way to do that would be though.

Nor am I. Identifying problems is easier than finding proper solutions.

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

Comments.

Rebase. I did a nopchbuild locally, but I won't trust that and test on CI I think.

In rP24217#45721, @Nescio wrote:

Where are unit obstruction sizes hidden? Not in the templates.

It's the pathfinder clearance. So it's slightly less than 1 for most units, rams and elephants are 3 iirc.

Actually no, it's calculated from the circle around the obstruction in both case, it's just units have rather small circular clearances, so the math is "exact" and the approximation is far less bad anyways.

Where are unit obstruction sizes hidden? Not in the templates.

I'm not entirely up to date on footprint bs obstruction, but this mostly reads like the obstruction should be made smaller, no?

No: the obstruction size is chosen such that the obstruction area is slighly smaller than the footprint area. If the obstruction radius would be reduced to produce an obstruction radius that is slightly smaller than the footprint radius, then the obstruction area will be much smaller.
For example, a circular structure with a footprint radius of 15 has an obstruction size of 25×25; a square footprint for that obstruction would be 27×27. Those three result in areas of 707, 625, and 729, respectively. To make the obstruction radius a bit smaller than the footprint radius, the obstruction size would have to be reduced to 20×20, which is a much smaller area (400).

In D3169#140100, @Nescio wrote:

Another example: compare a temple of size 20×20 with one of 10×40, i.e. both obstructing exactly the same area.
Under the distance from centre situation, there is no difference between the free aura area of the two.
Under the distance from edge situation, the longer temple has a slightly larger aura area (because 10+40>20+20).
Under the distance from circle around obstruction, the former has an obstruction radius of 14.1, the latter of 20.6. As a consequence their free aura areas are 3263 and 4783, respectively, i.e. the latter has a 46.6% larger aura area.

Another example: compare a temple of size 20×20 with one of 10×40, i.e. both obstructing exactly the same area.
Under the distance from centre situation, there is no difference between the free aura area of the two.
Under the distance from edge situation, the longer temple has a slightly larger aura area (because 10+40>20+20).
Under the distance from circle around obstruction, the former has an obstruction radius of 14.1, the latter of 20.6. As a consequence their free aura areas are 3263 and 4783, respectively, i.e. the latter has a 46.6% larger aura area.

In rP24217#45718, @Nescio wrote:

If I understand correctly, for units distances are still calculated from the centre, but for structures from the circle around their obstruction.

Actually no, it's calculated from the circle around the obstruction in both case, it's just units have rather small circular clearances, so the math is "exact" and the approximation is far less bad anyways.

If I understand correctly, for units distances are still calculated from the centre, but for structures from the circle around their obstruction.
Now I wonder whether it wouldn't be better to use the footprint radius instead of the obstruction radius. For rectangular structures, the footprint is typically slightly larger than the obstruction size (usually a structure with obstruction of 2a×2b would have a footprint of (2a+2)×(2b+2)). However, circular structures have circular footprints, but square obstructions; as a consequence their obstruction radii are larger than their footprint radii; some examples:

civ, structure:  footprint ; obstruction
brit, tower:        r =  6 ;  7.5² → r =  5.3
brit, house:        r =  6 ; 10²   → r =  7.1
brit, rotary mill:  r =  9 ; 16²   → r = 11.3
brit, civil centre: r = 15 ; 25²   → r = 17.7
brit, fortress:     r = 17 ; 29²   → r = 20.5
gaia, stonehenge:   r = 30 ; 55²   → r = 38.9

This generally looks good to me, I couldn't find a TargetIsAlive that you missed. It works in testing, both in-game and struct-tree (well, showing nothing, but it functions).

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

Account for the correct radius, thanks Nescio.

I ran a script to test all maps, and elephantine was the only one that crashed. I had some issues with unknown.js but I guess my autostart params were wrong. So it's good to go I think.

I have to say, I'm pretty dumbfounded. The fact that it works with GCC7.5 c++14, not with GCC 7.5 C++17, but with GCC 8 C++17 makes little sense to me. It also works with clang and MSVC, suggesting something odd gcc-side rather than a real bug (particularly since that code is fairly old). One suspect is that it might have something to do with alignment, since that changed in C++17.

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

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

While I can't say I understand map script, these changes do seem to solve the problems in these two random maps.
(I'm not sure it's complete; there might be more maps, scripts, or lines that need to be edited; I don't know.)

Off by one error.

Who'd have thought that everything that is not a wall _is_treated as such,,,

Not really, since one can capture buildings and the civ-bonuses wouldn't be applied then.

They are: values are based on who built it, not who owns it. For instance, brit and gaul structures have −20% health, athen, mace, spart structures +10%; if you're e.g. cart and capture a brit and an athen barracks, those will still have, respectively, 1600 and 2200 health when they're yours, whereas the barracks you built yourself has the default 2000. In principle those civ bonuses could be easily replaced by inserting e.g. <Health><Max op="mul">0.8</Max></Health> in all relevant templates, though that would mean a lot more duplication and make it harder to change these civ bonus values later. The same is true for the Mercenary costs.

I fail to notice the oddity, could you enlighten me?

Kushite centre and walls and Greek houses and barracks.

(GCC 7 specific it seems.)

I totally missed your comments here ^^'

In D2685#139996, @Nescio wrote:

The same could be said about civ bonuses: in principle those changes could be done in the specific templates too.

Not really, since one can capture buildings and the civ-bonuses wouldn't be applied then.

In D2685#139996, @Nescio wrote:

Also unit_elephant_african.json and unit_elephant_indian.json, or unit_advanced.json and unit_elite.json.

Yep, not a fan of those either :) At least the latter two are considered a hack and work is in progress to be replaced by a different system. As to the former two, I guess I missed D2864.

With this replay

Thread 1 "main" received signal SIGSEGV, Segmentation fault.
std::__uninitialized_copy_a<std::move_iterator<void**>, void**, ProxyAllocator<void*, Allocators::DynamicArena> > (
    __alloc=..., __result=<optimised out>, __last=..., __first=...)
    at /usr/include/c++/7/bits/stl_uninitialized.h:275
275		    __traits::construct(__alloc, std::__addressof(*__cur), *__first);
(gdb) bt
#0  std::__uninitialized_copy_a<std::move_iterator<void**>, void**, ProxyAllocator<void*, Allocators::DynamicArena> >
    (__alloc=..., __result=<optimised out>, __last=..., __first=...)
    at /usr/include/c++/7/bits/stl_uninitialized.h:275
#1  std::__uninitialized_move_if_noexcept_a<void**, void**, ProxyAllocator<void*, Allocators::DynamicArena> > (
    __alloc=..., __result=0x55556d1802cc, __last=0x55556d17f54c, __first=<optimised out>)
    at /usr/include/c++/7/bits/stl_uninitialized.h:312
#2  std::vector<void*, ProxyAllocator<void*, Allocators::DynamicArena> >::_M_realloc_insert<void*> (
    __position=0xffffcde000005555, this=0x55556d17e5b8) at /usr/include/c++/7/bits/vector.tcc:424
#3  std::vector<void*, ProxyAllocator<void*, Allocators::DynamicArena> >::emplace_back<void*> (
    this=this@entry=0x55556d17e5b8) at /usr/include/c++/7/bits/vector.tcc:105
#4  0x0000555555994f41 in std::vector<void*, ProxyAllocator<void*, Allocators::DynamicArena> >::push_back (
    __x=@0x7fffffffd0d0: 0x1437c, this=0x55556d17e5b8) at /usr/include/c++/7/bits/stl_vector.h:954
#5  CPatchRData::RenderBlends (patches=std::vector of length 38, capacity 64 = {...}, context=..., 
    shadow=shadow@entry=0x0, isDummyShader=isDummyShader@entry=false, 
    dummy=std::shared_ptr<CShaderProgram> (empty) = {...}) at ../../../source/renderer/PatchRData.cpp:948
#6  0x000055555591a698 in TerrainRenderer::RenderTerrainShader (this=<optimised out>, context=..., 
    cullGroup=cullGroup@entry=0, shadow=0x0) at ../../../source/renderer/TerrainRenderer.cpp:529
#7  0x00005555558f82b1 in CRenderer::RenderPatches (this=this@entry=0x5555572bb150, context=..., 
    cullGroup=cullGroup@entry=0) at ../../../source/renderer/Renderer.cpp:771
#8  0x00005555558fbe77 in CRenderer::RenderSubmissions (this=this@entry=0x5555572bb150, waterScissor=...)
    at ../../../source/renderer/Renderer.cpp:1355
#9  0x00005555558fc5fc in CRenderer::RenderScene (this=0x5555572bb150, scene=...)
    at ../../../source/renderer/Renderer.cpp:1718
#10 0x000055555587f464 in CGameView::Render (this=<optimised out>) at ../../../source/graphics/GameView.cpp:240
#11 0x00005555557b5419 in Render () at ../../../source/ps/GameSetup/GameSetup.cpp:234
#12 0x00005555555b19b8 in Frame () at ../../../source/main.cpp:424
#13 0x00005555555b49f8 in RunGameOrAtlas (argc=<optimised out>, argv=<optimised out>) at ../../../source/main.cpp:713
#14 0x000055555559e117 in main (argc=1, argv=0x7fffffffdff8) at ../../../source/main.cpp:764

$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/7/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 7.5.0-3ubuntu1~18.04' --with-bugurl=file:///usr/share/doc/gcc-7/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++ --prefix=/usr --with-gcc-major-version-only --program-suffix=-7 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib --enable-objc-gc=auto --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04)

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

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

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

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

Try implementing an ARB shader for flaring. Currently not working.

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

I'm not denying the math, but I don't think this tells the whole story. Area is hardly useful in 0 A.D., since you're rather unlikely to actually fill space up completely. Distance-to-aura matters much more, and that remains only about 4 meters in your example, or < .5 seconds for most units.
The fact that the previous calculations were buggy is a problem: imagine a wide enough entity, it might actually never be able to get inside the range of an aura smaller than its clearance, as the target size wasn't accounted for. We also directly had the range issues with buildings and units. Arrows that missed their target would also fail to pick all possible alternative targets.

In general (e.g. attack range) I agree, what matters is the distance. And I'm certainly not saying the old situation was perfect. However, this patch is on auras, and for auras (and territory) I believe what's meaningful is the area. Look at those of the monument, pyramid, and temple, how quickly your units can reach it is unimportant, the important thing is how many units benefit while fighting the enemy.

The fact is that even with a perfect edge-to-edge check system, bigger buildings would have bigger auras. It is possible to reject that notion entirely, but I'm not certain that I'd call that better. Larger buildings are also harder to place, etc.

Yes, they would, but under the distance to edge, the increase in area is simply proportional to the size.

Edit: in fact, compared to before, a building larger than the "expected" size will go from a large negative range penalty to a small positive one, and a building smaller than the expected size will go from a large positive bonus to a small negative one. The polarities are reversed, but the magnitude is likely to decrease.

You're right that before smaller structures were better, now larger are at an advantage. It's probably an improvement for attacks (at least archers can no longer hit a fortress without it shooting back). However, for attacks it's only the distance that matters, for auras it's the area. Because it's squared, small differences have more significant consequences.
In the brit vs spart temple example above, previously the difference was −115/1600π=−2.3%, now it's 304π/1600π=+19%.
Please correct me if I'm overlooking something.

In D3169#140038, @Nescio wrote:

[...]

Not quite, since you need to account for this diff, which reduces the range by [average size of temples, or about a]. The difference will depend on the size of the temple and the size of the range, but it's not that unfair.

The brit temple has an obstruction radius of about 16 and the spart temple of about 20. The distances are quite close, but what matters is the area, which is squared. Let's say the new aura radius is 20 instead of 40. The brit temple then covers an area of 1296π, the spart temple 1600π. Because, a, b, and r are all positive (>0), the difference in area (here 304π or about 955) is always much larger than the difference in obstruction size (here 17.5×35.5−22.5×22.5=115). The effect is disproportionate: the larger the structure, the greater the free area.

In D3169#140036, @Nescio wrote:

Yes, but the difference is now larger:
Previously, a temple of size a×a covered an area of πr² - a².
Now, the same temple covers an area of π(r+a)² - a² = πr² + 2πar + (π - 1)a².

Not quite, since you need to account for this diff, which reduces the range by [average size of temples, or about a]. The difference will depend on the size of the temple and the size of the range, but it's not that unfair.

Before, some temples covered areas smaller than others. A large temple might even have covered no area at all.

Yes, but the difference is now larger. The larger the temple is, the disproportionally larger the area becomes.
[EDIT] For oblong structures of size 2a×2b, we get a free area of:
πr² − 4ab previously
π(r+√(a²+b²))² − 4ab = πr² + 2π√(a²+b²)r + πa² + πb² − 4ab currently
πr² + 4ar + 4br under the costlier distance to edge.

In D3169#140034, @Nescio wrote:

Is it more predictable, though? Now obstruction size is taken into account, it means some temples cover larger areas than others.

Before, some temples covered areas smaller than others. A large temple might even have covered no area at all.

Also, range visualizations seem to be still calculated from the centre:

Yeah I need to fix that

Is it more predictable, though? Now obstruction size is taken into account, it means some temples cover larger areas than others.
Also, range visualizations seem to be still calculated from the centre:

@Angen, you pointed out some of the Barter/Market/Trade classes ought to be changed, however, it's not always clear to me into what exactly; see earlier posts.

In D3169#140021, @Nescio wrote:

The old way was problematic, but the new situation is similarily problematic.

It's not quite as problematic though, because at least you don't need to think about the source & target shape sizes to pick 'correct' ranges. I prefer predictable broken-ness over unpredictable one...

@Angen, interested in reviewing this one?

Hmm, why did you remove the platform detection?
You also broke the headers in case of no PCH I think
If you want to remove the duplication, you should put it in lib.h instead.

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

The old way was problematic, but the new situation is similarily problematic.
I'm not even sure which one I prefer. For gate pass radius and territory influence calculating from the centre is probably better, but for structure attack vision it's not.

I do agree that it'd be good to have better range checking, but it has performance impacts...

Performance is certainly important! I'm not a programmer and don't really understand code. Could you give an indication how costly various approaches are, compared to each other?

• distance to centre (old)
• distance to circle around obstruction (current)
• distance to ellipse around centre, with r_1 the obstruction width and r_2 the obstruction depth
• distance to corners (i.e. four circles)
• distance to edge (i.e. not a circle)

Looks good, will commit this soon, will make another patch for the SSE refactoring.

The purpose of this patch is to improve consistency and treat those formations on the same level. Currently the phalanx is the only one with separate unit templates; that's quite an inefficient approach; if we do that for all formations we'd have dozens of such files more, hundreds if we want to take into account combinations.

Why? What if I want my roman spearmen to use it ?

If that's desired I supposed we could add it into the units/rome/infantry_spearman_b.xml file.

In a normal game they won't, but they will if the campaign gives you some extra units, or if a building suddenly allows you to recruit them.

Good point! Any civ can get mercenaries from cart embassies or kush camps. I'm not sure those units are supposed to be able to form a testudo. If not, then perhaps it's better to remove that formation from the generic template_unit* files and insert it instead in the relevant specific units/rome/* files.

I believe anticavalry was removed because it's broken.

It's still present in the rome.json civ file. (The other two further questions refer to pers.json and kush.json.)

Works for me, not spurious crash when opening atlas, settings are restored correctly.

In D3169#140004, @Nescio wrote:

Distance from a structure, yes, but the new implementation is a distance to a circle around a structure, which is not quite the same thing. The more oblong an entity, the worse it becomes. And even for circular structures it's off.

Distance from a structure, yes, but the new implementation is a distance to a circle around a structure, which is not quite the same thing. The more oblong an entity, the worse it becomes. And even for circular structures it's off.

The same could be said about civ bonuses: in principle those changes could be done in the specific templates too. Also unit_elephant_african.json and unit_elephant_indian.json, or unit_advanced.json and unit_elite.json.
The advantage of this patch is that if someone wants to change these values, e.g. increase mercenary cavalry metal cost to +50, or reduce infantry to -10, that could be done by simply editing a single file, rather than having to go through numerous specific templates. Not everyone knows how to use grep.

*v141_xp

I've tested it again multiple time - can't reproduce the issue. I think it was my mistake.

rebase

That in the ticket iteration they moved counting from resourcegatherer to unitai to account for walking units

My take on this is D2915, which:

• doesn't hide that we're including a mercenary stub
• had no runtime cost
• lets people easily adjust the effect on a per-template basis if necessary

What I mind most is that it has a runtime penalty because of the technology. The hiding of template change is unfortunate, but I wouldn't block it on that account.

Let us know if you need help, I'd be happy to assist.

In D3169#139900, @Nescio wrote:

These reductions are too severe. Keep in mind a structure's radius is from centre to edge, thus to calculate it one should use half the width and half the depth of an entity, i.e. a structure with an obstruction size of 2a × 2b has a radius of c = √(a² + b²).

Mh, you're right, I systematically took the full width and height, not the half -_-

Did you manage to set up your dev-environment?

I agree that it hides template values.

(Also would it be nice to add some unit tests, but I can try to take care of that when the patch is nigh completion.)

In D3155#139110, @mammadori wrote:

it shows total gatherers under population icon too, which could be useful.

(It confused me what that number meant, at first I thought it was the number of houses.)

This patch does that. E.g. gaul spearmen won't be able to form a phalanx, because it isn't listed in the gaul.json file. Likewise, the syntagma formation is just included in the *_pikeman.xml templates, not in a separate level.

It's a little frustrating when women do more damage to siege engines than pikemen.

current pikeman  : 1 hack / 2 s = 0.5 hack/s
proposed pikeman : 2 hack / 2 s = 1 hack/s
female citizen   : 2 hack / 1 s = 2 hack/s

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

• rebased

(Needs to be rebased.)