perf(fps): optimize Object::isHero with cached hero counter #1841
Conversation
bobtista
force-pushed
the
bobtista/optimize-object-is-hero
branch
from
bcbf169 to
386443b
Compare
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp
Outdated
Show resolved
Hide resolved
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp
Show resolved
Hide resolved
Generals/Code/GameEngine/Include/GameLogic/Module/OpenContain.h
Outdated
Show resolved
Hide resolved
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp
Outdated
Show resolved
Hide resolved
xezon
added
Minor
bobtista
force-pushed
the
bobtista/optimize-object-is-hero
branch
from
4482d08 to
df4e579
Compare
xezon
left a comment
xezon
left a comment
There was a problem hiding this comment.
Currently this will break Xfer Save and Load.
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp
Outdated
Show resolved
Hide resolved
| xfer->xferUnsignedInt( &m_stealthUnitsContained ); | ||
|
|
||
| // hero units contained | ||
| xfer->xferUnsignedInt( &m_heroUnitsContained ); |
There was a problem hiding this comment.
This requires Xfer version increment. And version history documentation. Also needs to be put behind RETAIL_COMPATIBLE_XFER_SAVE. See other code that does it.
In RETAIL_COMPATIBLE_XFER_SAVE, the count needs to be restored by iterating hero objects like the original code did it.
There was a problem hiding this comment.
Done. It's version 3 in MD and 2 in Generals now
| } | ||
| if( rider->isKindOf( KINDOF_HERO ) ) | ||
| { | ||
| DEBUG_ASSERTCRASH( m_heroUnitsContained > 0, ("Removing hero but hero count is 0") ); |
There was a problem hiding this comment.
Text is not quite accurate:
"OpenContain::removeFromContainViaIterator - Removing hero but hero count is %d", m_heroUnitsContained
Could add the same assert to stealth units above.
| * Version Info: | ||
| * 1: Initial version */ | ||
| * 1: Initial version | ||
| * 2: Added m_heroUnitsContained cached hero count (bobtista) */ |
There was a problem hiding this comment.
TheSuperHackers @tweak Serialize hero units contained count
Do not use trailing (bobtista) because this is not part of our comment convention for authors.
Can put */ in new line.
| { | ||
| m_heroUnitsContained = 0; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
This whole block looks overengineered. Can it be simplifed to just
if (version >= 2)
{
xfer->xferUnsignedInt( &m_heroUnitsContained );
}?
|
|
||
| } | ||
|
|
||
| #if RETAIL_COMPATIBLE_XFER_SAVE |
There was a problem hiding this comment.
Use same technique as in #2024 for particle cannon to condition it.
2 participants
Summary
Optimizes
Object::isHero()by caching hero count in containers instead of iterating through all contained objects on every call.