Msvc 2013 changes #2621
Msvc 2013 changes #2621
Conversation
…obs, they were legacy and just for development, probably unsafe for use in production anyway. Add a safety check for receiving a patch job result.
…elease builds where the assert is omitted we still get sane default values out.
Add profiling to StarSystem, not sure why it wasn't done before since it's so expensive.
| std::map<SystemPath,StarSystem*>::iterator i = s_cachedSystems.begin(); | ||
| while (i != s_cachedSystems.end()) { | ||
| StarSystem *s = (*i).second; | ||
| assert(s->GetRefCount() >= 1); // sanity check | ||
| // if the cache is the only owner, then delete it | ||
| if (s->GetRefCount() == 1) { | ||
| if (s && s->GetRefCount() == 1) { |
There was a problem hiding this comment.
I don't think there's any situation where a null item in s_cachedSystems is valid, right? In which case this should be assert(s) before the if statement, not just skipping null items. (Or just leave it as it was, since if null is s then s->GetRefCount() should crash quickly).
There was a problem hiding this comment.
I think this one was just a paranoia check so it can just be removed.
|
Given that the |
|
Err, why is that stuff removed? It used to be necessary to quickly abort the terrain thread after config change and on hyperspace. Does that not work any more? Seems like a big change for a "compiler tweaks" PR... (on my phone right now) |
|
@johnbartholomew it is removed, in the same commit :) @robn don't panic, that stuff became redundant a long while ago and has been sat disabled in the code for... I don't know how long but 6 months? Maybe longer? Not needed or used anymore anyway. |
|
Cool then! Sorry, not a panic, I honestly wasn't sure and wasn't able to check right this second. Thanks :) |
|
@fluffyfreak Oh, so it is! I was looking up where |
|
Merging now. I'm changing most of the pointer checks you added to be asserts, because silently ignoring invalid state just helps to hide bugs. I'm leaving the one in PicoDDS, because I think a 0 result from that function is a reasonable response to failure there (not that the return value is checked but what the heck). |
Some things pointed out by the MSVC code analysis I was playing around with.
Mostly just trying to make sensible changes too a few things, remove redundant overloads and get rid of some rubbish code etc.