This works on all platforms where you can use clang-format, which I assume is Windows as well (Visual Studio might even have integrated support).

I don't think the OOM can be related to this, as this code (or the result of using it) is never used again after initialization, as far as I know.

This code has long been planned to be removed (precisely because I expected things like this to happen), if things are difficult to fix a good alternative is just to stop calling this code, but it's all a bit messy to figure out where this code is called from. See also #4451.

Yeah. Well. It seems that we could probably get it to be consistent across all these platforms, but it wouldn't solve this problem. And I doubt it would be worth the effort after reading a bit. Some links:
https://randomascii.wordpress.com/2013/07/16/floating-point-determinism/
https://yosefk.com/blog/consistency-how-to-defeat-the-purpose-of-ieee-floating-point.html

An idea: why don't we rewrite the fixed types to wrap a floating-point number and code all functions modifying this number (like addition, multiplication, sin, cos, etc...) in assembly using the SSE(2) instruction set? All our target platforms should (currently) support SSE2 and we could probably provide software fallbacks for non x86 architectures if we where to ever support them. SSE should be fully specified and thus deterministic across processors, as far as I'm aware (and NEON on ARM may even be (mostly) consistent with it). Using assembly functions would ensure that the exact order of execution will be the same across compilers.

If someone wants to finish and rebase D492, then that should probably be done instead, but I won't be able to work on this for quite some time, so maybe?

Hmm, so apparently parts don't really work as it is now. Will investigate.

In D1331#54461, @mimo wrote:

Before going back to such a separation (which improved performance, but revealed itself hard to maintain in the long term), it would be good to know exactly where is the real gain: is it because we have less info to transfer? or because we have less info to compute?
If the latter is dominant, it may be more efficient to cache all these attack/armour infos instead of recomputing them every turn (applyValuesModifications is not specially fast) as is done for example in ResourceGatherer for the rates inside OnValueModification (note that this could be done better in ResourceGatherer as everything is recomputed with any change). Doing it that way could also help when fighting as also the simulation would not have to recompute all quantities each time.

Works for me on Arch Linux, and looks good.

D492

I think it's a bit of both, for attack in particular it's mostly the computation time but there is also some transfer cost. if we look at the entire time saved by this patch on my computer I'd estimate 6-7ms is in computation time, 3-4 ms in transfer time. In principle it's going to be difficult to make the selection really fast, simply because of the way our entity system is designed currently (ideally we'd rewrite it in the future).

I'm almost satisfied with the performance of gui updates (thanks to mimo), so when that's done I'll subject the turnmanager to the overhaul I've been planning for a long time now. Hopefully that bug will get caught as well.

If you change the pointers -> reference, it can be committed.

Seems to work well.

Ok, at least I will review this patch (I think it's fine, but I will need to get svn to apply the patch as it doesn't seem to apply on the latest state of the git mirrors, so will need about 30 minutes before I can test it).
Only thing I can think of is that maybe templateDataStatus could be renamed to modifiedTemplates, GetTemplateDataStatus to IsTemplateModified, but am not sure if that would be an improvement.

(also, are you planning more of these gui sim update performance related changes? I want to avoid that we do double work and am currently (and have been for a while) trying to improve the performance of selection gui updates)

My approach is a bit different though, wouldn't it be possible to only reload all the changed templates and not all templates of a player if something changed?

funny coincidence, I was just working on the exact same thing ^^

The check is unneeded, as the list is only modified in timer_AddClient. (and both clients / numClients are static, so will be zero-initialized).

Looks good to me (can't check if issue is fixed as I don't have the issue, but the return value should be checked in any case).

Looks good.

I personally don't agree with the = 123 syntax being ugly, and it seems to be the recommended way to do things in modern C++: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-in-class-initializer

I vote for the second option, we don't really need to support these long messages (just write them to some log if needed).

In D956#37500, @temple wrote:

(We'll need to decrease the turn length, at least for movement. Otherwise it's too hard to predict where units will be in order to steer around them or slow down before crashing into them. For example, Starcraft II does 16 simulation updates per second on normal speed, but in multiplayer we only do 2 updates per second.)

Some things I found (maybe not exhaustive) -- Some of these might easily be converted to use smart pointers and ideally most of this code should be rewritten to use smart pointers, but that might cost too much time.

Then it's ok, but could maybe add the comments for that in the future.

There are still some fallthrough warnings in AtlasObjectXML.cpp, is that expected?

Needs rebase (patch fails to apply currently), also, you can remove the now unused stats_cache() function and macro in lib/file/common/file_stats.*. Otherwise patch looks and works well.

Fix 2 more whitespace issues, which should hopefully have been the last remaining.

Fix 4 more whitespace issues.

• Replace some NULL occurrences with nullptr.
• push_back -> emplace_back in some places.
• Fix some more style issues (including some whitespace things).
• Remove leftover TODO and comment about scriptInterface.
• Add an easier syntax for deseralizing a message, as suggested by leper.

Yes, I did: all places where this message is subscribed to use OnGlobalEntityRenamed, so this shouldn't change any behavior.

Add spaces and quotes.

• Fix assert failures in debug mode (add default constructor to SerializeableJSValue).
• Remove unneeded copy constructor for CSimulationMessage.
• Fix most (all?) issues found by leper.

Looks fine to me, if those occurences of NULL are changed to nullptr.

This works well and looks good, maybe we should also disable the -Wimplicit-fallthrough warning? (or add comments for that; unfortunately we don't have [[fallthrough]] until C++17)

Seems to work well on linux, but can't test on windows atm, as my windows install is currently broken...

Address some of lepers concerns and rebase.

I think we should keep the number for now.

would change the text to Add units in batches (the batch size is 5 by default and can be changed in the options)

found some more (minor) style issues.

Encountered the same issue and this fix works.

Cleaned up some left over things and added macros for net message / serializable structure creation.

In D72#19856, @vladislavbelov wrote:

In D72#19853, @echotangoecho wrote:

Would be nice if we could move to VS2015 soon and use the full range of C++11 features, good patch!

We won't support full C++11 only with VS2015, we need to drop old gcc too.

Would be nice if we could move to VS2015 soon and use the full range of C++11 features, good patch!

Isn't it better to call std::abort() instead of std::terminate()? In the standard it seems that std::terminate() is only discussed in relation to exceptions, and it doesn't seem like it is a function one would call normally.

GetSimulationState indeed doesn't take any arguments, good catch!

With the suggested piece of code, HoldMessages() is still called when m_HeldDepth == 0 resulting in an out of bounds access...

Fix indentation style.

If we change the function to return a double instead of an i64, this works well.

The problem is that when the HoldLevel() == 0, in HoldMessages() the index into the m_HoldBuffers[8] (which is HoldLevel() - 1) is out of bounds. But like I said, I don't know how this code is supposed to work so I expected the HoldLevel() < 8 to be a bounds check, is this wrong?

There should be a legend for the player colors in the graph.

(I don't have time to review this but I will keep watching this)