You might want to add some description to this diff like:
What is its state? (TODOs, Bugs, etc)
Did this originate from the diff andy9595 (I think that's the wrong number ;) wrote or was it written by you?

Minor style fixes

Translate and use setStringTags

Use icons from Stan. No extra icon definition. Remove change crept in from other diff.

Rebase

Will commit this soon.
(Still not convinced about the reduction of the duplication. It is probably doable but doesn't get more readable.)

Rebase

In D2833#121464, @Freagarach wrote:

It's not really consistent in the rest of the file ;)

https

In D2832#121395, @Nescio wrote:

Nor is line 138. To me 138 seems to be in the same vein as e.g. “a true gentleman’s name appears in newspapers only three times in his life: to announce his birth, marriage and death.” Not being talked about was generally a virtue.

any real knowledge about the "real" position of women in this times.

Hmm didn't thought about it that way around. To me the quote just sounded like women should not matter.

Looks very good.
Notice my two comments.

In D2832#121349, @Nescio wrote:

How is this fundamentally different from e.g. line 52 or 199? Or the way female citizens are presented in game (i.e. cheap, weak, expendable workers)?
I don't really care about this particular quote, but once you start censoring ancient authors based on modern views of political correctness, you enter a slippery slope.

Rebase

Add frames for an animated ping.

Correct scissoring → Don't draw parts of the ping outside the minimap.

In D2825#120980, @wraitii wrote:

Hm, this technically could be a small performance regression if the uninitialisation is on purpose. Though I'm not sure it is from comparing with CVector3.

Yeah, in theory. But I doubt it has a real impact. And as you also noticed we already do it in CVector3D

I'm not sure we use CVector2 a lot so it's probably irrelevant.

Yep.

Seems like this does not work with GLSL disabled.

Isn't this another step into making the different civs indistinguishable?
Wrt rams vs rams: Afaik swordmen are quite effective against rams aren't they?

Use a circle texture.

I think you should split the string fix.
And you should show some translated explanation for each hotkey instead of it's config name.

In D2812#119946, @vladislavbelov wrote:

In D2812#119945, @Imarok wrote:

Does MSAA 32× make sense?

I don't think so, I see no difference with 32 (also I don't remember such value in most games).

Me neither xD

Seems like not working when changing in atlas.

It's disabled in Atlas, because it uses wxWidgets's canvas with own GL context.

aha. I guess that makes sense. (I personally have no idea, about that topic ;P)

Looks really sharp. Does MSAA 32× make sense?
Can change ingame without issues.
Seems like not working when changing in atlas. (FXAA is working)

In D2778#117793, @Stan wrote:

The mac build failiure in unrelated. Itms disabled Mac SVN build until he has some time to address the bug. Linux one is the usual segfault.

Hmm, it feels quite wrong to allow a disabled button to be pressed when it's disabled in general.
What about adding a special event for rightlclick on disabled button?
That way a disabled button is still disabled, but one can add a special event handler for a specific button if they really want to react to a right-click even if the button is disabled. (Like in case of unit buttons)

Small style fixes.

This patch is still WIP.

In D2362#116819, @wraitii wrote:

I kind of get how you want to do this. It sounds sane.
But I've never done something with our animations/variants etc. so I'll need to look into it first. I'll do that when I find the time.
If you think it's a quick no-brainer for you, feel free to commandeer. ;)

It's quite easy: you set the animation variant, and in the groups you have something like:

<group>
  <variant name="default0" frequency="0.5">
    <animation name='Idle' ... />
  </variant>
  <variant name="default1" frequency="0.5">
    <animation name='Idle' ... />
  </variant>
  <variant name="build_field" frequency="0">  <- this never gets picked, unless the animation variant is "build_field", in which case it always gets picked
    <animation name='Idle' ... />
  </variant>
</group>

Ah, that's how I imagined it :)

In D2362#116778, @wraitii wrote:

Anyways, don't bother with XML, just do like carrying animations: define a variant build_{generic building name} (instead of an animation) and set it when building. Then make sure to create a group with the build animations only, and let the default one(s) have some >0 frequency, and name one "build_farm" with frequency 0. Then your units will do what you want automagically.
(you'll need to use "Idle" for your animation name when Idle, but that's OK).

Note to self: According to Vladislav a texture should be used to draw circles. Scaling should be done with "adding tex coords and so on". Entry point for understanding how that works could be OverlayRenderer::RenderForegroundOverlays.

In D1849#87712, @elexis wrote:

Precisely because they have no idea how to fix this, the one line in options.json is the right fix isn't it?

Then the next issues comes straight ahead: The user needs to remap attack-move and garrisson.
Yeah, we could change that in the same go, but no idea to what key we should remap that.
(And not the free time to look into it)

Don't recreate the bug fixed by D2729

Good.

Never thought that someone might want to use orderone for preselcted actions xD

Don't upload changes from D2727

In D2727#115118, @elexis wrote:

I didn't understand how this function will be used exactly, but if CMinimap.cpp will consume a boolean, is it conceivable that it will consume a number or string (for example the type of the preselected action), or another GUI object type consuming another object?

No.
I kind of explained already why I need this diff. But I hope my next diff makes it clearer.

consistent return and correct include.

In D2727#115107, @elexis wrote:

In D2727#115103, @Imarok wrote:

In D2727#115077, @elexis wrote:

WithReturn can be avoided from the name, the functions are distinguished by signature already and do the same except return value.

Why? They have the same signature and only distinguish in the return value. Some parts of the gui need ScriptEvent to have no return value. (e.g. RecurseObject)

Is there a compile error if RecurseObject gets a function with ignorable return value instead of a void?
Aside from that one can still name the void and function the same way if both signatures are necessary and the compiler will chose the signature it needs.

No, you can't overload a function only by changing the return type.

This can be used to get a feedback if the function called through ScriptEvent handled the event.

It sounds like boolean would restrict the use cases of that function then, that the caller that expects a boolean can do that after calling the function and that other callers might consume something other than boolean.

So you propose just returning a JSValue?

I dont know the use case envisioned but the use case seems conceivable in gernal, so diff direction seems correct.

I already teased the use case in the diffs description: the bool will say whether this event has been handled or not.
I can't think of use cases, that use a nonboolean return value.

Just because we can't conceive use cases of non-boolean return values doesnt mean that its best to cast the return value to boolean in any case - the alternative is to let the caller decide, if a caller wants bools, it can cast to bool, if a caller wants a function, string or numbers as return value, it can use it, but not if it only provides a boolean. So the only reason to cast tobool in that function would be if there would be many callers that all want booleans (i.e. too much code duplication), or if returning the JS value would cost more performance, but as far as I know it only returns a pointer that already exists.
I.e. cast to bool inside the function would be useful if it improves performance or avoids 20+ bool conversions outside of the caller, otherwise the function becomes more versatile if its the caller that casts to bool.

In D2727#115077, @elexis wrote:

WithReturn can be avoided from the name, the functions are distinguished by signature already and do the same except return value.

Why? They have the same signature and only distinguish in the return value. Some parts of the gui need ScriptEvent to have no return value. (e.g. RecurseObject)

This can be used to get a feedback if the function called through ScriptEvent handled the event.

It sounds like boolean would restrict the use cases of that function then, that the caller that expects a boolean can do that after calling the function and that other callers might consume something other than boolean.

So you propose just returning a JSValue?

I dont know the use case envisioned but the use case seems conceivable in gernal, so diff direction seems correct.

Rebase upon D2719

Consistent return

Dot after comment and use a getter to comply with OOP

In D1751#114648, @badosu wrote:

This is great! Can we add a keybinding to ping on the center of the current camera view?

Looks like a sane and small scoped change for me. So I'll commit that in the coming days if nobody objects.

Remove superfluous newline

Add comment explaining why we disable preselcted actions on minimap