Do we actually have any AIs that don't use this? And if we don't, is this still worth supporting?

const

Could be a range-based for loop, no?

With C++20 we could do for (const auto& templateNames = cmp->Bar(), usedTemplates.reserve(templateNames.size()); const auto& template : templateNames), but well.

Depending on the number of templates on maps this might not be optimal. (In the worst case we do about |m_LatestTemplates|*|usedTemplates| - |usedTemplates|^2/2 steps, which could be worse than adding everthing, sorting, calling unique (which would be |m_LatestTemplates|*log(|m_LatestTemplates|)).)

I'm not sure if this is worth worrying about, but would this be nicer if we just kept track of templates and a count? Or possibly returned more templates than needed by just using m_TemplateSchemaValidity? Or just using a map here for the looping (depending on the size of usedTemplates, that might be slower)?

I guess you could just check how long this takes on most maps and point out that it is faster than what we did previously and possibly add a TODO that this could be improved if one really wanted to.

Agree that we should get rid of that, I don't think there are any gain to create any new AI not using shared component. But that's not for that patch.

yep

Sure for the loop (I focused too much at the way the old code worked).

I did some tests (see below), and the main conclusion is that the time is completely negligible with our number of entities and templates, and going with an unordered_set seems the best for scaling with the number of templates.

m_TemplateSchemaValidity can't be used as not set when deserializing (nor when testing without validation), and that would look like a hack. We could nonetheless create a new unordered_set to keep track of templates, but not really worth as used only once.

&

Ah, I think that was just broken on gcc 4.6, then 4.7 had proper docs and 4.8 had it implemented. (and we dropped < 4.8).

I had a suspicion that it might just not matter for the amount of starting entities we have. Nice to have that confirmed with some tests.

Given those numbers I'm not sure if an unordered_set is that important, especially since we'd either have to sort it at some point, or run into some risks with differently ordered lists passed to JS.

Oh, this is not using vector anymore. Give the experimental results and the possibility for platform differences this way I do prefer the vector variant.

This seems odd

IIRC I kept it around back in the days for the tutorial AI, which may or may not have been removed. Agreed that it should be used for any new AI.

You might want to reserve an arbitrary number here, just to avoid uselessly reallocating too much (as I assume we generally will have more than 8/16 templates). Won't really matter but it's good form.

I don't really understand what this line means. What's the condition to be "validly passed"?

I've no idea what it is used for.
But anyway, all this component would be better with a good cleanup (and threading).

I don't know. This function is only called once, with about 100 entries currently (so we don't care of the overhead of reallocation), and we don't know a priori what will be the number of templates in mods. So it would just be a magic number.

I've just kept the same wording as was used before (for findAllTemplates). These templates are loaded using GetTemplateWithoutValidation, so they are expected to exist and be valid.