Changeset View
Standalone View
source/ps/Game.cpp
Show First 20 Lines • Show All 451 Lines • ▼ Show 20 Lines | const CColor& CGame::GetPlayerColor(player_id_t player) const | ||||||||||
if (player < 0 || player >= (int)m_PlayerColors.size()) | if (player < 0 || player >= (int)m_PlayerColors.size()) | ||||||||||
return BrokenColor; | return BrokenColor; | ||||||||||
return m_PlayerColors[player]; | return m_PlayerColors[player]; | ||||||||||
} | } | ||||||||||
bool CGame::IsGameFinished() const | bool CGame::IsGameFinished() const | ||||||||||
{ | { | ||||||||||
for (const std::pair<entity_id_t, IComponent*>& p : m_Simulation2->GetEntitiesWithInterface(IID_Player)) | SComponentDataGenerator gen = m_Simulation2->GetComponentsByInterfaceUnordered(IID_Player); | ||||||||||
while (const IComponent* cmp = gen.Next()) | |||||||||||
{ | { | ||||||||||
CmpPtr<ICmpPlayer> cmpPlayer(*m_Simulation2, p.first); | CmpPtr<ICmpPlayer> cmpPlayer(*m_Simulation2, cmp->GetEntityId()); | ||||||||||
Stan: Any reason why we don't cast but use query interface here? | |||||||||||
Done Inline ActionsNot that I see. Testing of (cmpPlayer) also seems questionable. Maybe there is some kind of race condition here? If a player leaves the game does that happen in a different thread from simulation? Mercury: Not that I see. Testing of (cmpPlayer) also seems questionable. Maybe there is some kind of… | |||||||||||
Done Inline ActionsIn theory the whole simulation context runs in the same thread. Which is underusing CPU cores a lot ^^ Stan: In theory the whole simulation context runs in the same thread. Which is underusing CPU cores a… | |||||||||||
Done Inline ActionsYeah, probably safe to simplify. Also can use GetComponentsByInterfaceUnordered here. Maybe some other places as well. Mercury: Yeah, probably safe to simplify. Also can use GetComponentsByInterfaceUnordered here. Maybe… | |||||||||||
if (cmpPlayer && cmpPlayer->GetState() == "won") | if (cmpPlayer->GetState() == "won") | ||||||||||
return true; | return true; | ||||||||||
Done Inline Actions
Can't it be written like so, avoiding the stack object creation? Or maybe like so if (static_cast<CmpPtr<ICmpPlayer>>(cmp)->GetState() == "won") In which case we should probably fix the other occurences Stan: Can't it be written like so, avoiding the stack object creation? Or maybe like so
```
if… | |||||||||||
Done Inline Actionsstatic_cast<ICmpPlayer*>(cmp)->GetState() works, had to remove const from cmp for some reason. More generally I don't think this method should exist. Wouldn't it make more sense to mark the game as finished at the same time as marking players as having won? Which other occurrences did you mean? Mercury: static_cast<ICmpPlayer*>(cmp)->GetState() works, had to remove const from cmp for some reason. | |||||||||||
Done Inline ActionsWhere we are creating CmpPtrs on the stack It might make sense. Maybe the js code doesn't have access to this code though. Stan: Where we are creating CmpPtrs on the stack
It might make sense. Maybe the js code doesn't have… | |||||||||||
Done Inline ActionsCmpPtr is used 475 times. I went through the first 25 and didn't see any of them making the same mistake. They seemed to be all checking for the existence of a component on a known entity id, rather then iterating ids. Mercury: CmpPtr is used 475 times. I went through the first 25 and didn't see any of them making the… | |||||||||||
} | } | ||||||||||
return false; | return false; | ||||||||||
} | } |
Any reason why we don't cast but use query interface here?