Differential D4718 Diff 20584 source/graphics/ObjectManager.cpp

Changeset View

Standalone View

source/graphics/ObjectManager.cpp

* This file is part of 0 A.D. * This file is part of 0 A.D.

* *

* 0 A.D. is free software: you can redistribute it and/or modify * 0 A.D. is free software: you can redistribute it and/or modify

* it under the terms of the GNU General Public License as published by * it under the terms of the GNU General Public License as published by

* the Free Software Foundation, either version 2 of the License, or * the Free Software Foundation, either version 2 of the License, or

* (at your option) any later version. * (at your option) any later version.

* *

* 0 A.D. is distributed in the hope that it will be useful, * 0 A.D. is distributed in the hope that it will be useful,

Show All 15 Lines

#include "ps/ConfigDB.h" #include "ps/ConfigDB.h"

#include "ps/Game.h" #include "ps/Game.h"

#include "ps/Profile.h" #include "ps/Profile.h"

#include "ps/Filesystem.h" #include "ps/Filesystem.h"

#include "ps/XML/Xeromyces.h" #include "ps/XML/Xeromyces.h"

#include "simulation2/Simulation2.h" #include "simulation2/Simulation2.h"

#include "simulation2/components/ICmpTerrain.h" #include "simulation2/components/ICmpTerrain.h"

#include "simulation2/components/ICmpVisual.h" #include "simulation2/components/ICmpVisual.h"

#include "simulation2/system/ComponentDataHolder.h"

bool CObjectManager::ObjectKey::operator< (const CObjectManager::ObjectKey& a) const bool CObjectManager::ObjectKey::operator< (const CObjectManager::ObjectKey& a) const

{ {

if (ObjectBaseIdentifier < a.ObjectBaseIdentifier) if (ObjectBaseIdentifier < a.ObjectBaseIdentifier)

return true; return true;

else if (ObjectBaseIdentifier > a.ObjectBaseIdentifier) else if (ObjectBaseIdentifier > a.ObjectBaseIdentifier)

return false; return false;

else else

▲ Show 20 Lines • Show All 121 Lines • ▼ Show 20 Lines Status CObjectManager::ReloadChangedFile(const VfsPath& path)

// Mark old entries as outdated so we don't reload them from the cache // Mark old entries as outdated so we don't reload them from the cache

for (std::pair<const ObjectKey, Hotloadable<CObjectEntry>>& object : m_Objects) for (std::pair<const ObjectKey, Hotloadable<CObjectEntry>>& object : m_Objects)

if (!object.second.outdated && object.second.obj->m_Base->UsesFile(path)) if (!object.second.outdated && object.second.obj->m_Base->UsesFile(path))

{ {

object.second.outdated = true; object.second.outdated = true;

changed = true; changed = true;

} }

const CSimulation2::InterfaceListUnordered& cmps = m_Simulation.GetEntitiesWithInterfaceUnordered(IID_Visual); SComponentDataGenerator cmps = m_Simulation.GetEntitiesWithInterfaceUnordered(IID_Visual);

StanUnsubmitted

Done

changed = true;

}

- SComponentDataGenerator cmps = m_Simulation.GetEntitiesWithInterfaceUnordered(IID_Visual);

+ const SComponentDataGenerator components = m_Simulation.GetEntitiesWithInterfaceUnordered(IID_Visual);

// Reload actors that use a changed object

Should it be const? The name of the function implies it returns a list of entities, but it's actually a list of components ?

Same question for three below.

Stan: Should it be const? The name of the function implies it returns a list of entities, but it's…

MercuryAuthorUnsubmitted

Done

I think const is correct as no object member data is being modified.

I agree about misleading function name.

Mercury: I think const is correct as no object member data is being modified. I agree about misleading…

StanUnsubmitted

Done

Usage is also not consistent among places.

Stan: Usage is also not consistent among places.

MercuryAuthorUnsubmitted

Done

My bad, actually const doesn't work here, generator needs to modify it's buffer pointer. By 'consistent' do you mean the variable name? I changed them all to gen.

Mercury: My bad, actually const doesn't work here, generator needs to modify it's buffer pointer. By…

StanUnsubmitted

Done

I see ents, cmp and gen

Stan: I see `ents`, `cmp` and `gen`

// Reload actors that use a changed object // Reload actors that use a changed object

for (std::pair<const CStrW, Hotloadable<CActorDef>>& actor : m_ActorDefs) for (std::pair<const CStrW, Hotloadable<CActorDef>>& actor : m_ActorDefs)

{ {

if (!actor.second.outdated && actor.second.obj->UsesFile(path)) if (!actor.second.outdated && actor.second.obj->UsesFile(path))

{ {

actor.second.outdated = true; actor.second.outdated = true;

changed = true; changed = true;

// Slightly ugly hack: The graphics system doesn't preserve enough information to regenerate the // Slightly ugly hack: The graphics system doesn't preserve enough information to regenerate the

// object with all correct variations, and we don't want to waste space storing it just for the // object with all correct variations, and we don't want to waste space storing it just for the

// rare occurrence of hotloading, so we'll tell the component (which does preserve the information) // rare occurrence of hotloading, so we'll tell the component (which does preserve the information)

// to do the reloading itself // to do the reloading itself

for (CSimulation2::InterfaceListUnordered::const_iterator eit = cmps.begin(); eit != cmps.end(); ++eit) while (IComponent* cmp = cmps.Next())

StanUnsubmitted

Done

I think we have ICmpPtr

Same question for all the occurrences of this.

Stan: I think we have ICmpPtr Same question for all the occurrences of this.

MercuryAuthorUnsubmitted

Done

I can't find any use of ICmpPtr.

Mercury: I can't find any use of ICmpPtr.

StanUnsubmitted

Done

My Bad. CmpPtr<ICmpVisual>

Stan: My Bad. CmpPtr<ICmpVisual>

MercuryAuthorUnsubmitted

Done

The CmpPtr template takes an entity as argument and gets the component through query interface so it isn't appropriate for iteration. Another constructor overload could wrap the static_cast but that seems like obfuscation to me.

Mercury: The CmpPtr template takes an entity as argument and gets the component through query interface…

static_cast<ICmpVisual*>(eit->second)->Hotload(actor.first); static_cast<ICmpVisual*>(cmp)->Hotload(actor.first);

cmps.Reset();

StanUnsubmitted

Done

Can we use the CCmpManager function for those too?

Stan: Can we use the CCmpManager function for those too?

MercuryAuthorUnsubmitted

Done

No, CComponentManager::BroadcastMessagesToInterface calls the HandleMessage method but these call Hotload.

Mercury: No, CComponentManager::BroadcastMessagesToInterface calls the HandleMessage method but these…

StanUnsubmitted

Done

Might make sense to make a hotload function then? Could be out of scope.

Stan: Might make sense to make a hotload function then? Could be out of scope.

MercuryAuthorUnsubmitted

Done

We would need two versions ( with and without argument ) and pass through functions in simulation2, doesn't seem worth it to me for only 3 instances.

Mercury: We would need two versions ( with and without argument ) and pass through functions in…

} }

if (changed) if (changed)

// Trigger an interpolate call - needed because the game may be paused & if so, models disappear. // Trigger an interpolate call - needed because the game may be paused & if so, models disappear.

m_Simulation.Interpolate(0.f, 0.f, 0.f); m_Simulation.Interpolate(0.f, 0.f, 0.f);

return INFO::OK; return INFO::OK;

} }

void CObjectManager::ActorQualityChanged() void CObjectManager::ActorQualityChanged()

{ {

int quality; int quality;

CFG_GET_VAL("max_actor_quality", quality); CFG_GET_VAL("max_actor_quality", quality);

if (quality == m_QualityLevel) if (quality == m_QualityLevel)

return; return;

m_QualityLevel = quality > 255 ? 255 : quality < 0 ? 0 : quality; m_QualityLevel = quality > 255 ? 255 : quality < 0 ? 0 : quality;

// No need to reload entries or actors, but we do need to reload all units. // No need to reload entries or actors, but we do need to reload all units.

for (CSimulation2::InterfaceListUnordered::const_iterator eit = cmps.begin(); eit != cmps.end(); ++eit) while (IComponent* cmp = cmps.Next())

static_cast<ICmpVisual*>(eit->second)->Hotload(); static_cast<ICmpVisual*>(cmp)->Hotload();

// Trigger an interpolate call - needed because the game is generally paused & models disappear otherwise. // Trigger an interpolate call - needed because the game is generally paused & models disappear otherwise.

m_Simulation.Interpolate(0.f, 0.f, 0.f); m_Simulation.Interpolate(0.f, 0.f, 0.f);

} }

void CObjectManager::VariantDiversityChanged() void CObjectManager::VariantDiversityChanged()

{ {

CStr value; CStr value;

Show All 14 Lines void CObjectManager::VariantDiversityChanged()

for (std::pair<const ObjectKey, Hotloadable<CObjectEntry>>& object : m_Objects) for (std::pair<const ObjectKey, Hotloadable<CObjectEntry>>& object : m_Objects)

object.second.outdated = true; object.second.outdated = true;

// Reload actors. // Reload actors.

for (std::pair<const CStrW, Hotloadable<CActorDef>>& actor : m_ActorDefs) for (std::pair<const CStrW, Hotloadable<CActorDef>>& actor : m_ActorDefs)

actor.second.outdated = true; actor.second.outdated = true;

// Reload visual actor components. // Reload visual actor components.

for (CSimulation2::InterfaceListUnordered::const_iterator eit = cmps.begin(); eit != cmps.end(); ++eit) while (IComponent* cmp = cmps.Next())

static_cast<ICmpVisual*>(eit->second)->Hotload(); static_cast<ICmpVisual*>(cmp)->Hotload();

m_Simulation.Interpolate(0.f, 0.f, 0.f); m_Simulation.Interpolate(0.f, 0.f, 0.f);

} }