Looks good to me (with the exception of the remaining discussion around the -1 cast)

@wraitii any update on this migration to SM91? Would be great to move to the newer SM version

Lgtm, could probably bikeshed more on the existing futures code, but let's move this along :)

Looks pretty good to me, nice win and good cleanup on the type erasure. I don't have any other major comments, so I'll leave it up to @Stan or @wraitii for a final accept.

From a 40,000ft view, libraries like libunifex already implement similar sender-receiver functionality à la the wg21 P2300 paper. Are there particular advantages of adding our own implementation of a subset of p2300 vs. adopting a library like libunifex which offers a much broader set of functionality?

Looks good from my end for the most part. A few lingering nit type comments left, but otherwise great.

Awesome perf win! I play lots of large multiplayer matches, and definitely appreciate seeing the late game speed up with many units.

Usually the overhead from std::function<> comes from one of two sources:

1. Heap allocation for the functor/function pointer/callable std::function is wrapping (https://devblogs.microsoft.com/oldnewthing/20200514-00/?p=103749), including CPU cache misses when following the pointer
2. Indirection when invoking the type erased std::function through a virtual method call

Curious as well what sort of tooling 0AD has for template scanning/validation? I saw the templateanalyzer under source/tools, but that appears to be about balance analysis, rather than validation.

Is it possible there are other templates with this same issue?

Update from feedback

Hey @wraitii are you planning on cleaning up and landing these changes soon? Asking since I'd like to pull in changes with the AddRecurrentTask API and can either wait for this to land, or put up a diff with an updated version of TimerThread :)

In D4673#198748, @Stan wrote:

Did you see https://trac.wildfiregames.com/wiki/Coding_Conventions?
Some braces are out of place ?

Did you check the logic change by running some networked matches ? :)

Looking at the details, the ip and port values inferred in the three conditional blocks are not used in the actual connection attempt. But rather those two values are only used to establish a Stun tunnel. For that reason, I'm not convinced that QueryConnectionDetails() would be semantically accurate for the name. Instead, something like SetupStun() would more accurately reflect the intent. Additionally, naively extracting the middle section of TryToConnect() which is gated on g_XmppClient into a second function requires some gymnastics around the return value. There are three possibilities for the return value:

1. One value indicating the original code had 'return true' in the initial TryToConnect() implementation, and the new TryToConnect() implementation should have an early 'return true' and not proceed further following the call to 'SetupStun()'
2. Another value indicating the original code had 'return false' in the initial TryToConnect() implementation, and the new TryToConnect() implementation should have an early 'return false' and not proceed further following the call to 'SetupStun()'
3. Another value indicating the original code extracted into SetupStun() had neither 'return false' nor 'return true' and TryToConnect() should continue to perform 'g_NetClient->SetupConnection(enetClient)' as before

In D4673#198742, @Stan wrote:

Hey thanks for the patch, can you add yourself to the credits ?

Added to programming.json credits file

I suppose the underlying assumption here is the queue is expected to be empty the majority of the time, and thus worker threads should be able to idle themselves to free up CPU cores.

Formatting fixes