Since we only have to change 3 lines (default.cfg, options.json, session.js), I'd rather not add [renderer] here and have every other renderer option not be part of renderer.
Since we now have a hotkey to enable and disable it quickly it seems to more a user interface option than a renderer one

aurarange/ toggleauraoverlays
Better: aurarange/ toggleaurarange

"of selected units and structures", since it should be obvious to the user what it does (and since we don't show it for the building preview) and since it will be ignored for the other planned hotkey (that coincidentally will be on Tab by default, to show all auras on screen)

rendering -> display
Refs that not being a renderer option anymore

" of selected units and structures"
remember to update the Hotkeys wiki page accordingly.

of selected units and structures

Would put this above recalculateRangeOverlays in session.js. Additions to this messy file should be reserved to code that is either directly related to input processing (something with SDL keycodes or mousemovements) or functions that are very similar to the other ones.

Could have a JSdoc comment then

Not doing the String() here means doing one less conversion summa summarum

better two lines, at least we have it everywhere else in the GUI like it, makes it readable like a table and friendly to extend

Ok my assumption that this actually does make a difference if moved to OnTick was proven true, but no need to go for that, just costs performance for no reason, people can wait a turn (until it'unpaused in the worst case). People don't change the setting as often as the status bars appear

• no need for the default argument
• The entire function is used only once, so could be inlined, especially since we can't really reuse it (the hotkey to toggle auras for specific units was discussed with causative, but if so, would have to be done differently, f.e. not take a function argument)
• Function name could have been polished otherwise, also doesn't toggle but sets to enabled
• After merging the two, we can do all 4 Config calls in one go

✓

This block looks really nice!

The defaults might or might not be moved to the aura templates someday

That does the same thing as EnableVisualRangeOverlayType and can be nuked. We thereby remove the way to delete entries, but that doesn't seem needed if we can disable them.

well it makes the line below 4 characters shorter, IMO looks nicer without the continue

self-explanatory

better each argument on a separate line than 2 arguments on one line, 2 on another. Nice side effect of having them aligned

We could be more moderate and only add it to those that have an aura. But this going to be extended to attack and heal, so meh

✓

fits better in the second line

trailing whitespace

✓

Rename to AddRangeOverlay:

(03:12:34) elexis: renaming ResetRangeOverlays to DeleteRangeOverlays and SetRangeOverlay to AddRangeOverlay should make it more readable, right?
(03:14:21) Sandarac: I used Reset b/c that is the way it is worded in cmpoverlayrenderer
(03:15:08) elexis: that also has an AddSprite function :P
(03:15:18) elexis: -> ResetRangeOverlays AddRangeOverlay
(03:15:36) elexis: Set sounds like it mgiht modify an existing one
(03:15:41) Sandarac: ok

Guess m_RangeOverlayData.push_back({rangeOverlayDescriptor, rangeOverlay}); is nicer to read, since we're already constructing a temporary.

Some related discussion from 2015-08-29-QuakeNet-#0ad-dev.log

16:31 < Itms> elexis: #3293 is nice, just use emplace_back and I'll commit
16:53 < Philip`> Why would you ever want to use emplace_back?
16:53 < Philip`> All it saves you is the cost of a move constructor, if I remember correctly
16:58 < Philip`> It saves you nothing in almost all cases (since the move constructor is usually essentially free), and it's not supported by compilers that we probably care about, and its name is ugly

It landed with push_back in rP16963, nowadays we would add a reserve call (which isn't possible here)

http://en.cppreference.com/w/cpp/container/vector/emplace_back
http://www.cplusplus.com/reference/vector/vector/push_back/

(all range overlays or any range overlay)

unneeded comment

A more specific name would help for example distinguishing it from the "fancy" aura rendering (where the texture spans the entire disk)

maybe UpdateTexturedLineOverlay ?

agree

unsigned? not uint32 or similar?

This is the hidden bug I've been looking for hours.
The removed comment + early return was rightfully there.
As the comment states, we don't need to update overlays of buildings each RenderSubmit call.
This can be trivially fixed by adding a pair of braces to the call:

case STATIC_OUTLINE:
if (!m_BuildingOverlay)
{
 	m_BuildingOverlay = new SOverlayTexturedLine;
 	UpdateTexturedLineOverlay(&m_OverlayDescriptor, *m_BuildingOverlay, 0, true);
}

Confirmed with a debug_sprintf on the rms_new_demo map that has only one entity (before and after the vis aura patch).

The performance impact would be entirely neglible since we never select more than a handful of buildings simultaneously, while we will now very often select many units with aura (females), but those braces are gratis this morning.

cmpTerrain was unused, so ok

Yep, we now need the GetInterpolatedPosition2D since units move

Ok, we can reuse this indeed

Comment a bit pointless since it repeats the function name.
Some readers might not be aware of what a "Range Overlay" is, so adding an example for aura or attack range helps with both

Delete all range overlays to reduce the repetition