Fixing Bad Architecture

Hi guys,
I’m new to developing with Unity and could need your advice. I created a App which works but it’s arcitecture feels really smelly from my point of few. I guess I still have problems to adopt the component driven design of Unity.

But lets focus on my problem:
What the app does is receiving a collection of object via JSON. There objects can basicly seen as Cubes, Spheres or other 3D Objects. The Variables of these Objects specify the Object like if it’s a Cube, it’s position, size, orientation, etc. The Objects are received and updated every update from a 3rd party system.

So what I would normaly do is implement the class of these objects in a way that I could simply instanciate the objects.

But what I learned it’s not possible to instanciate classes in Unity which inherit MonoBehavior.
So what I did was to create one class to save the variable values which are received via JSON lets call it ObjectParameters.
Then for each of the received objects I create a GameObject and add a script (lets call it ObjectManager) to it. This script has a setter to receive the ObjectParameters which hold the objects variables. This script then applies all the object parameters to the game object.

So something like this:

foreach (ObjectParameters params in ObjectsParameters)

{
GameObject obj = new GameObject();
obj.AddComponent();
obj.GetComponent().setZoneParameters(param)
}

This feels like making a simple thing more complicated than it should be. There must be some better way to do this.

I’m sorry that I’m restricted to not show any code by my company.

Thanks in advance.

I think it’d be a lot more handy if you don’t have the ObjectManagers attached to the objects at all, but just have one component responsible for handling all of the objects.

Assuming that each object has an ID in the ‘ObjectsParameters’, it’d be something like this:

Dictionary<uint, Transform> controlledObjects = new Dictionary<uint, Transform>();

void Handle(List<ObjectParameters> objParams) {
    foreach(var param in objParams) {
        Transform t;
        if(!controlledObjects.TryGet(param.ID, out t) {
            t = CreateFromParam(param);
            controlledObjects[param.ID] = t;
        }
       
        SetTransformValues(t, param);
    }
}

This’ll be a lot easier to manage, and you won’t have to keep all of those ObjectManagers around that you have to send the object parameters to.

This is, by the way, the same advice I’d give for a non-Unity project. The ObjectManager class doesn’t do anything interesting - it just reads values and passes them on to a different object. OOP doesn’t mean that you need to wrap every method in it’s own object.

Why are companies like this? Living in a country with software patents, or are your bosses just idiots?

1 Like

^ agreed that it’d be better to have a single ObjectManager that tracks all the objects, and/or serve as a container for all these objects.

If you’re receiving a JSON message from, say, the network, the ObjectManager would add itself as a listener to the NetworkManager to receive and process JSON messages. This gives you more flexibility to create/edit/destroy objects.

Also, a quibble with this example:

foreach (ObjectParameters params in ObjectsParameters)
{
   GameObject obj = new GameObject();
   obj.AddComponent<ObjectManager>();
   obj.GetComponent<ObjectManager>().setZoneParameters(param)
}

You know that AddComponent returns the component added, right? No need to call GetComponent immediately after adding it.

I can actually relate. I once worked at a company that was very protective of its trade secrets and IPs, even if the code itself was literally a copy from a stackoverflow article. Post code anywhere, and you risk having the CEO coming over to have a talk with you. So it’s best to not post at all.

I got out of that company as fast as I could.

2 Likes

Thanks guys, that seems to remove my confusion.