This is the code base you inherit, I don't see how it's material to this discussion. Someone needed to load a texture, LoadTexture was called. We have to work within the constraints we're given, which means you don't always get to design a system from scratch.[/quote]

Hence the discussion here. To make those that start from scratch not default to big ball of mud design.

So what do you do under these constraints, do you just dump it in the too hard basket, because you can't make your change as cleanly as you'd like?[/quote]

You do nothing. Suck it up, do the job.

And when someone asks how to design a system, you denounce the common mistakes, such as using global namespaces, global state or excessive coupling without thought.

So what do you do under these constraints, do you just dump it in the too hard basket, because you can't make your change as cleanly as you'd like?

You do nothing. Suck it up, do the job.
[/quote]

I personally consider it a cardinal sin to go in and 'fix' things, if there's no demonstrable benefit to refactoring, something that impacts the project in a tangible way either now or in the immediate future (not 'maybe' later). If it's going to help later, fix it later.

In the example cited, with texture loading, I think it clearly falls into this category of 'no demonstrable benefit of making this change'. I've seen systems like this persist for decades without a need for change. Unless you can outline how it improves your final product, given a finite budget you've necessarily lowered the quality of the end product, by neglecting higher priority systems that do have a tangible impact on your application.

over the years, one solution i've found is to always throw away anything that you call manager. once you write a class like "TextureManager", throw it away. a manager is the worst kind of construct you have. define what you want to really have. a manager is a be all do all thing, and thus will evolve into some global singleton bloat that you don't need.

Typically your texture loader will see a texture has already been loaded, and return an appropriately reference counted instance of that texture. That 'already loaded' texture directory needs to exist somewhere, just wondering where you store this data if not a texture manager?

I've got a few more examples I'd like to put out there, as I'm still in pursuit of a solution that makes all parties happy. Basically to elaborate on my previous example (to give a concrete situation where people can provide alternatives), in the loading routines each object is simply loaded from an XML node. You have a generic XML loader, which a graphics plugin (extension) can register custom types against. So <GraphicsEntity type="UserGraphicsEntity"/> is encountered, and the XML node is forwarded to some pre-registered load interface. The graphics engine knows nothing of what might be loaded, and thus cannot have any specific information 'passed in' to its load calls.



std::unique_ptr<IGraphicsEntity> UserGraphicsEntity::LoadGraphicsEntity(LoadContext& context)
{
// general loading

// resolve waveform interface
std::unique_ptr<IWaveFormData> wfd = context.ResolveInterface<IWaveFormData>(guid1);

// resolve temperature interface
std::unique_ptr<ITemperatureData> td = context.ResolveInterface<ITemperatureData>(guid2);
}


Now the nice thing here is 'guid' may refer to an object that doesn't implement ITemperatureData, but 'ResolveInterface' can choose to automatically construct an agent that does the conversion for you. It might instance a TemperatureAgent object, that does whatever is required to extract temperature data from the specified object - and the source object providing the temperature need never know about this intermediary interface. It creates a well defined place where the translation between modules takes place - whether that be waveform data from the sound engine, or temperature data from your dynamics engine. Don't think the source object should ever be available to other modules? That's fine, don't have it 'publish' its information, so it's effectively hidden from its peers.

So where does one access this 'ResolveInterface' method? You can certainly pass it around in the load context as above, you could have a singleton Publishers::getInstance().ResolveInterface method. Anyone can publish itself to the directory, and since we're using GUIDs, by definition you will never need more than one directory.

Now you could have this publisher interface contained in a factory, so instead we get


std::unique_ptr<IGraphicsEntity> UserGraphicsEntityFactory::LoadGraphicsEntity(XMLNode& node)
{
// general loading

// resolve waveform interface
std::unique_ptr<IWaveFormData> wfi = m_publishers.ResolveInterface<IWaveFormData>(guid1);

// resolve temperature interface
std::unique_ptr<ITemperatureData> ti = m_publishers.ResolveInterface<ITemperatureData>(guid2);
}


I think this would be a perfectly adequate substitute, I don't think it would require a terribly large amount of overhead to implement. It is a plausible alternative to use of a global instance that doesn't inhibit productivity, because when you add a new 'publisher', you do it in precisely one place, and you don't have to update a bunch of code to get access to this new object - it happens for free. This allows objects from any module to access data from any other module though, and I can't help but feel some will step in and say that's not acceptable. That's what a forum is for though.. The other thing is you'll get a proliferation of 'm_publishers' type entities scattered far and wide, an acceptable, but I can't help but feel a little messy, side effect of such an implementation.


One other thought is I believe people get too caught up in what is the bad part of globals (and their cousin singletons), it's not the 'how' that is the problem, meaning how you got a reference to that piece of data - but the 'why'. To illustrate

void Gun::Update()
{
if(g_triggerActive)
{
Fire();
}
}

What's wrong with the above? It's pretty evident, why would a global state, g_gunFiring, be used in a generic Gun class. Doesn't make a lot of sense. How about we get rid of the global

void Gun::Update(bool triggerActive)
{
if(triggerActive)
{
Fire();
}
}

Ignore for a moment it's a bit weird passing in the trigger state to the Update method, but that's not relevant. Now if we call

void main()
{
// ...

gun->Update(g_triggerActive);

Well it hardly fixes a thing. This is the problem I see with so called 'solutions' to the global instance problem. Simply storing the global shared instance as a member variable doesn't change the fundamental behavior of your program. It is mathematically identical in pretty well all respects, except for cache locality related items. Most notably, it changes precisely nil in terms of thread safety when we're talking about references to global data, an often harped on about benefit of not using globals (which I've yet to understand). The problem is not so much the global, but whether what the global represents really is global - and whether referencing that global state from the code in question makes sense.

in simple: it just shouldn't happen at that level. why can't a scene properly load it's textures (and all other data besides textures) without loading stuff multiple times? that's not part of a texture manager, it's just part of the scene to handle that.

scene {
std::<texturename, Texture> textures;
}

done.

there is your texturemanager, your complex global have to be thing that just does not need to.

Meaning things just perform the lookup by name, and the load code (what might be termed the texture manager by some, as *something* needs to facilitate on the fly load requests based on dynamic player inputs) will populate it in advance accordingly? I don't mind this approach (I presume all graphics entities have a scene passed in?), and assuming 'scene' is a class (not a namespace), the context object I refer to in earlier code is pretty much just 'scene'.

It is mathematically identical in pretty well all respects[/quote]

Yes it is - as per those Turing equivalence theorems, all code is identical, regardless of language or algorithm.

void Gun::Update(bool triggerActive)
{
if(triggerActive)
{
Fire();
}
}
[/quote]

Enter IOC/DI...
class SceneEntity {
GlobalFlags * pFlags;
}

class Gun : SceneEntity {
...
void update() {
if (pFlags->triggerActive) Fire;
}
}The pFlags gets passed in during construction, most likely inside a factory.

Most notably, it changes precisely nil in terms of thread safety when we're talking about references to global data, an often harped on about benefit of not using globals (which I've yet to understand)[/quote]

Well:class DistributedSim {
threadlocal<GlobalFlags> flags;
Gun * g;
DistributedSim() {
g = new Gun(flags);
}
}
Now I can run multiple simulations inside same process without any lock contention.

And if flags need to be shared, the above merely exposes just how complex of a problem that sharing is.

except for cache locality related items[/quote]

If flags is shared, things get much worse fast. If shared data, something like a flag is modified frequently (millions of times vs. once per frame/step), each such change triggers cache line invalidation, stalling all cores. "I know, I'll use lockless algorithms" - now you're stalling the CPU millions of times per second. Sharing is simply expensive.

Of course, nobody knows about that since it doesn't show up in profiler. Code just runs slower than in a single thread and nobody is any wiser.

By being explicit about which parts are shared, you might notice the actual complexity of synchronizing distributed code and choose different design or algorithm, perhaps by batching updates or using rolling buffer or perhaps something else altogether.


Finally, IT industry concluded in 2002 that singletons spell death of project maintainability. And IT isn't known for being at bleeding edge of experimentation. Computational performance was never relevant to them (except for that certain bug they found in VMs), but performance and scalability of development was. During the next year, everything shifted to IOC/DI container or dynamic languages. None of this can be proven mathematically, but falls under soft observations. It just works better in practice.

Well it hardly fixes a thing. This is the problem I see with so called 'solutions' to the global instance problem. Simply storing the global shared instance as a member variable doesn't change the fundamental behavior of your program. It is mathematically identical in pretty well all respects, except for cache locality related items. Most notably, it changes precisely nil in terms of thread safety when we're talking about references to global data, an often harped on about benefit of not using globals (which I've yet to understand). The problem is not so much the global, but whether what the global represents really is global - and whether referencing that global state from the code in question makes sense.

The problem is your example is, once again, bad code.
In any well designed system the 'trigger state' isn't going to be a global but will be a value which has passed down a line from a few systems to get to a semantically sane meaning at a given point.

An example might be something like;

[Input subsystems queries input state]->[input state is converted to in game value]->[passed onto 'controller' object which uses the state (and maybe previous states) to cause a reaction somewhere else]

The last step is vague because it depends on the game but if you take something like Unreal the input ends up on a 'controller' and that controller then drives the entity it is attached to (with the added bonus that your 'controller' doesnt have to be local, nor human driven, to Just Work(tm)), be it updating the camera, moving the model or passing the message onto a 'weapon controller' system which determains if a gun should shoot or not.

At the end of the day the only thing which sees a 'raw' trigger state is the input subsystem and that hangs around about as long as it takes to get converted to your 'in game' state values to drive other systems.