Usually one can write Fixes #5445 to automatically close tickets.
@Itms, we now need a AtlasUI autobuild for windows, not sure that's automatic yet.

Sorry, should have been more specific.

Yeah I was waiting for something like D2009 which I would have done myself if someone hadn't come to the rescue. Will add the keyword soon.

In rP22323#34353, @elexis wrote:

Pass pointers instead of arrays

How are arrays passed?

If you have some time, maybe it would be nice to have a small video on youtube demonstrating the feature. Nothing fancy.

In D1337#83716, @wraitii wrote:

Rebased - needs some cleaning, like the anti-cavalry formation no longer has a walk animation which is annoying - I guess I should group the animations better. The limit of one variant per file is quite annoying because it means one must change a lot of files every time -_-
Still would work with D1901

Well it's up to wraitii to review those, so I guess up to him then.

In rP22295#34278, @elexis wrote:

Reviewed By: Wraitii

Reviewed By is reserved for people other than the author.
Comments By: Itms can be added when people influenced the patch without having accepting it.

In rP22266#34230, @elexis wrote:

No notes.
That these templates shall not appear in the list of placeable templates may be true, but there are also the generic templates that should not appear in that list.
Also that a template shall not appear to be placeable doesn't mean that it is impossible to place it if it would appear in the list.
Hence unplaceable/ would also be imperfect.
Another possibility would be hidden/, but that also is ambiguous. Everything is ambiguous.
Perhaps unplaceable/ and a README.txt defining unplaceable/ as not appearing in the Atlas list of entities would be the solution that satisfies all problems (including the clarification that a template that may technically be placed may be contained in that folder).

@elexis did you get the notes ? :)

Try to fix the build on Linux

The hue is slightly different.

• Whitespace
• Static_cast
• spaces to tabs
• rebase

Commandeering to rebase @Clockwork-Muse feel free to commandeer back if you want to continue working on it.

That sounds good and maybe actually better than being that precise though the shape should differ depending on the type of attacker. However I beleive that the spread parameter is made for that. One could use that to compute the circle.

@wraitii Thoughts on this ?

I think it would be nice to have this.

@wraitii @Freagarach.

Well maybe you guys could try to use my conan patch ? You will need to change the copy to osx instead of win32 and likely edit premake libs.

I'm abandoning this :) It probably can be reopened when we actually remove these references.

Raising concern for a warning reported by @Krinkle. No biggie but the entity parameter isn't used anymore.

Yeah I don't think that will work ever. I'm not sure where @fabio saw that it could be removed.

Maybe you can fix the last two warning (trailing dots)

The auto fix failed I guess.

In D1993#83328, @Krinkle wrote:

In D1993#83293, @Stan wrote:

Before this get committed you might as well want to fix the warnings that appeared in the Vulkan console.

The only remaining one should be for curly. Due to there being almost 50/50 adherence to that, I wasn't sure whether it is a style we still want to follow consistently or whether it might be something we want to disable and allow developer to choose on case-by-case basis whether curly braces are useful.

In D1993#83291, @Krinkle wrote:

Hoping to see this land relatively soon to avoid merge conflicts, after that I'll move on to the next area to lint :)

In D1950#83214, @Nescio wrote:

Ideally different projectiles would also be able to have different damage, therefore what is eventually desirable is support for multiple ranged attacks, e.g. a fortress able to fire both arrows (long range, low damage) and javelins (shorter range, higher damage).

Most of the time Anything between {} is part of an object. anything between [] is part of an array

Well if you can fix the ai that's nice.

broke debug build. instant segfault

Can you fix the capitals I reported as well in this diff ?

Yeah new diff. Not good combining fixes with other stuff.

This breaks sounds for me. I believe that maybe g_SoundManager->IdleTask(); gets called too often ?

I'm not really for this. If anything resource should start with a capital instead.

• Add aura tests.

In D1950#82752, @Freagarach wrote:

In D1950#82751, @Stan wrote:

I guess you broke the tests :)

Yeah, by removing those spaces?

I guess you broke the tests :)

Just for reference the vector issue might be http://trac.wildfiregames.com/ticket/5376

The bars are somewhat brighter also.

In D1846#82471, @wraitii wrote:

In D1846#82413, @Stan wrote:

I wonder if we shouln't have
GetTributableCodes, GetBarterableCodes, GetTradableCodes, to avoid hardcoding magic strings all over the place.

That's how it was before... I didn't like that because it means you have to add a whole new function for any new property. I don't think there's much difference between hardcoding strings and hardcoding function names.

In D1976#82461, @vladislavbelov wrote:

I think the patch should be tested on multi-monitor environment and with a custom color space.

I wonder if we shouln't have

• Do not break the test in the process

Fix warnings

• Use function for the tests to remove duplication.

In D1718#82355, @wraitii wrote:

It seems to me like your tests could be abstracted in a function to remove a lot of redundancy.

Easier to deal with auras and whatnot.

OK, so I guess your position is that you want to enable having a "faster fattening" aura/tech that changes ResourceSupply/Fattening/Value, instead of having a tech that changes ResourceSupply/Alive/Value and only affects items with Identity classes Organic, Domestic for example. That's internally coherent, and I do agree that on the surface it seems more craftier - both systems actually allow the same thing but using identity classes might force one to add arbitrary identity classes to templates, which seems undesirable.
However this doesn't seem obvious from reading the code, nor the tests. I think you should be able to add an Aura in the tests that only affects Fattening, not say Growth, to highlight that.

Didn't we have that discussion already that it was particularly tricky to add auras during the tests ?

I'm not sure where this would be best to document, perhaps in the schema as a a:help or as a comment?

In D1973#82305, @wraitii wrote:

On (2), I don't think this should be added to the Ranged attack root. Rather, it should be added to the projectile - because friendly fire for non-ranged attacks makes no sense, and "ranged" attacks only real difference is the presence of a projectile. See rP20676 and rP22184. If you add it to the projectile, the splash damage definition can keep its FriendlyFire attribute, since Melee attacks dealing splash damage with or without friendly fire makes sense. In fact, the splash from ranged attack should probably be moved under projectile but that's out of scope.

Haven't checked performance, but maybe that would be better to do data structures in C++ ? I'm sure boost or the stl have already such containers ?

Some more comments, maybe it would be nice to add one test or two in attack.js to show how to use it for test purposes.