- User Since
- Jan 19 2017, 7:09 PM (108 w, 2 d)
Aug 19 2018
Hmm, so apparently parts don't really work as it is now. Will investigate.
Aug 17 2018
Works for me on Arch Linux, and looks good.
Feb 24 2018
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).
Jan 26 2018
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 ^^
Jan 25 2018
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).
Dec 4 2017
Nov 19 2017
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
Nov 7 2017
I vote for the second option, we don't really need to support these long messages (just write them to some log if needed).
Oct 17 2017
Aug 20 2017
Aug 18 2017
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.
Aug 17 2017
Then it's ok, but could maybe add the comments for that in the future.
Jul 30 2017
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.
Jul 11 2017
Yes, I did: all places where this message is subscribed to use OnGlobalEntityRenamed, so this shouldn't change any behavior.
Add spaces and quotes.
Jul 10 2017
Jul 8 2017
Jul 7 2017
Jul 6 2017
- Fix assert failures in debug mode (add default constructor to SerializeableJSValue).
- Remove unneeded copy constructor for CSimulationMessage.
- Fix most (all?) issues found by leper.
Jul 5 2017
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)
Jun 23 2017
Seems to work well on linux, but can't test on windows atm, as my windows install is currently broken...
Jun 4 2017
May 18 2017
Address some of lepers concerns and rebase.
May 15 2017
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.
May 14 2017
Encountered the same issue and this fix works.
Cleaned up some left over things and added macros for net message / serializable structure creation.
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.
May 13 2017
May 12 2017
GetSimulationState indeed doesn't take any arguments, good catch!
May 11 2017
May 10 2017
With the suggested piece of code, HoldMessages() is still called when m_HeldDepth == 0 resulting in an out of bounds access...
Fix indentation style.
Mar 17 2017
If we change the function to return a double instead of an i64, this works well.
Mar 15 2017
The problem is that when the HoldLevel() == 0, in HoldMessages() the index into the m_HoldBuffers (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?
Mar 11 2017
Mar 10 2017
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)
Well, it seems strange to use some random value for an infinite value, and infinite values should in practice not be displayed on the chart.
I mean displaying a gap.
Wouldn't it make more sense for nothing to be displayed in the case of an infinite value?
Compiles for me.