Question What do you think about my data holder class?

Imagine I have a huge scriptable object called AiSo.
There are many different classes in it

MovementSettings,
DashSettings,
AttackSettings,
GetHitSettings,
AnimationSettings...etc

And I have separate components that need this data.
I want my movement script to be aware only of movementsettings.
I want my attack script to only know the attack settings etc.

When Aiso changes during the game, I want all the settings in this component to change as well.

I thought of assigning such a dataholder to each object and sharing the necessary data through that dataholder. Do you think it makes sense?

Code (CSharp):

1. using System.Collections.Generic;
2. using UnityEngine;
3.  
4. namespace iCare.Core {
5.     public sealed class DataHolder {
6.         readonly Dictionary<string, object> _keyDataDict = new();
7.         readonly Dictionary<object, object> _typeDataDict = new();
8.  
9.  
10.         public void AddWithKey<T>(string key, T data) {
11.             if (!_keyDataDict.TryAdd(key, data)) {
12.                 Debug.LogWarning($"Key {key} already exists in the dictionary.");
13.             }
14.         }
15.  
16.         public void AddWithType<T>(T data) {
17.             if (!_typeDataDict.TryAdd(typeof(T), data)) {
18.                 Debug.LogWarning($"Type {typeof(T)} already exists in the dictionary.");
19.             }
20.         }
21.  
22.         public T GetWithKey<T>(string key) {
23.             if (_keyDataDict.TryGetValue(key, out var data)) {
24.                 return (T)data;
25.             }
26.  
27.             Debug.LogWarning($"Key {key} not found in the dictionary.");
28.             return default;
29.         }
30.  
31.         public T GetWithType<T>() {
32.             if (_typeDataDict.TryGetValue(typeof(T), out var data)) {
33.                 return (T)data;
34.             }
35.  
36.             Debug.LogWarning($"Type {typeof(T)} not found in the dictionary.");
37.             return default;
38.         }
39.  
40.         public void ChangeWithKey<T>(string key, T data) {
41.             if (_keyDataDict.ContainsKey(key)) {
42.                 _keyDataDict[key] = data;
43.             }
44.             else {
45.                 Debug.LogWarning($"Key {key} not found in the dictionary.");
46.             }
47.         }
48.  
49.         public void ChangeWithType<T>(T data) {
50.             if (_typeDataDict.ContainsKey(typeof(T))) {
51.                 _typeDataDict[typeof(T)] = data;
52.             }
53.             else {
54.                 Debug.LogWarning($"Type {typeof(T)} not found in the dictionary.");
55.             }
56.         }
57.  
58.         public void AddOrChangeWithKey<T>(string key, T data) {
59.             if (!_keyDataDict.TryAdd(key, data)) {
60.                 _keyDataDict[key] = data;
61.             }
62.         }
63.  
64.         public void AddOrChangeWithType<T>(T data) {
65.             if (!_typeDataDict.TryAdd(typeof(T), data)) {
66.                 _typeDataDict[typeof(T)] = data;
67.             }
68.         }
69.     }
70. }

 

What purpose does this serve?

... effectively what I see here is a dynamic object (lots of scripting languages have dynamic objects, including C#). And this object allows you to insert members in it on the fly at run time by name (you also have a by type, I'll get to that later though).

The main benefit is that you can dynamically add entries at runtime rather than hard-coding them as concrete members ala:

Code (csharp):

1. class DataHolder
2. {
3.     public MovementSettings MovementSettings;
4.     public DashSettings DashSettings;
5.     //... etc
6. }

Which I mean this can be useful if you don't necessarily know what settings a given "AiSo" is going to need at runtime.

OK, but why?

You say:

OK, then why not just have these settings in their respective associated script?

Can't really say because I don't know what problem you're trying to solve with this.

 

When AiSo changes, naturally all movementsettings etc. will also change. So I will need to update all scripts runtime on every change.

I had opened a similar topic before and it was suggested that I do it via the interface, so I was using it until I found this method.

Code (CSharp):

1.  
2. public interface ISet<in T> {
3.     void Set(T value);
4. }
5.  
6. public interface IProvide<out T> {
7.     T Get();
8. }
9.  
10.  
11. using iCare.Common.Target;
12.  
13.  
14. using iCare.Core;
15. using iCare.Core.Attributes;
16. using Sirenix.OdinInspector;
17. using UnityEngine;
18.  
19. namespace iCare.CAi {
20.     internal sealed class AiProvider : MonoBehaviour, ISet<AiSo>, IProvide<ITargetFinder>, IProvide<AiSo> {
21.         [SerializeField, CanChange, Required] AiSo aiData;
22.  
23.         ITargetFinder _targetFinder;
24.         ISet<ScriptableListCTarget>[] _teamListSeekers;
25.  
26.         void Awake() {
27.             _teamListSeekers = GetComponentsInChildren<ISet<ScriptableListCTarget>>();
28.         }
29.  
30.         void OnEnable() {
31.             _targetFinder = new TargetFinder(aiData.SettingsDetection.TargetTeamList);
32.  
33.             _teamListSeekers.SetAll(aiData.SettingsDetection.MyTeamList);
34.         }
35.  
36.  
37.         public ITargetFinder Get() => _targetFinder;
38.         AiSo IProvide<AiSo>.Get() => aiData;
39.  
40.         public void Set(AiSo value) => aiData = value;
41.     }
42.  
43.  
44.  
45. internal sealed class TestMovementScript : MonoBehaviour {
46.     IProvide<RootMotionMovement> _movementSettingsProvider;
47.     RootMotionMovement _movementSettings => _movementSettingsProvider.Get();
48. }
49.  
50.  
51. }

I can handle it cleanly via the interface, but when it is the same type, it can cause confusion and I have to serialize it and assign it manually, etc.

-From what I researched, dependency injection is a bad choice because I have to inject it again every time data is changed.
-Scriptable Variable is a bad choice because I need to create a lot of it and it can easily become mass.


For some reason, I also like the idea of keeping the data that will be shared between classes in one place. (note every game object will have its own DataHolder so dictioanry wont be big)



The reason why I made the type and key separately is that if there is only one data type, I will save it directly with the type, so I don't have to deal with the string, if there is more than 1 data, I will save it with the key.

 

OK. But what is AiSo for in the first place?

What is it you're trying to solve in the first place?

What is the grand plan here?

Are you writing enemy AI logic whose configuration changes on some event that might occur mid game (i.e. the config is different based on if the player is sneaking or running?)

You appear to be bogged down in the engineering of something right now, but we don't know what you're engineering. We don't know what an AiSo is, we don't know what these settings are that scripts rely on, we don't know why it's important for the settings to exist both in AiSo as well as in their respective scripts that actually use them, we don't know why AiSo is changing independent of the scripts... why even have AiSo if it comes with all of these hiccups?

What IS AiSo?

...

Also... I'm afraid to start this tangent. But... this is scratching my head independent of your post:

Spoiler

Not if you rely on reference types, which if you're using classes you are.

Code (csharp):

1. var a = new SomeType();
2. var b = a;
3.  
4. b.Name = "lordofduct";
5. Debug.Log(a.Name); //prints 'lordofduct' because a and b reference the same object

...

With that said... lets scale back your problem and not over-complicate things with some abstract engineering tom-foolery.

What is your problem you're trying to solve?

Let's start there.

 

In general, my problem is that I do not know how to properly manage the data that changes in runtime.

What I want to achieve in this example is
A scriptable object that stores AiSo enemy settings.

These settings include the character's model, speed, power skills, etc.

And there is a spawn system where I spawn these enemies using object pooling.

What I want to achieve is that when I reactivate the deactivated object, it is updated with the new aiso data I gave it while it was disabled.

 

There we go... ok. So you have enemies that spawn and despawn and this stores their state/configuration separate from its visual state so it persists regardless.

Got it.

1 last question... why would the state of AiSo change separate from the scripts they're associated with? Why would MovementSettings change separate from the Movement script in a manner that Movement would have to be updated?

That final question could help me in giving you a solution to your problem. I already have one formulated in my head, but I don't want to make assumptions about that aspect of it and it turn out I'm wrong and waste a lot of time jibber-jabbering on about a solution that may not work for you.

 

Movement Script
or Health script or Attack Script is not directly related to AI. Components that work independently with the given data.

For example, there is a health script on every object. I fill the health data of some of them with a serialize field on the editor, some of them are given by AiSo, some of them are given by the player through the code.

That's why I try not to make a direct connection between AI and movement or health.

Code (CSharp):

1. [System.Serializable]
2. internal sealed class HealthData {
3.     [SerializeField] float maxHealth;
4.     [SerializeField] float maxArmor;
5.     [SerializeField] float maxFireResistance;
6.     [SerializeField] float maxIceResistance;
7.  
8.  
9.     internal float MaxHealth => maxHealth;
10.     internal float MaxArmor => maxArmor;
11.     internal float MaxFireResistance => maxFireResistance;
12.     internal float MaxIceResistance => maxIceResistance;
13. }
14.  
15. internal sealed class Health : MonoBehaviour, ISet<HealthData> {
16.     //ON DESTROYABLE OBJECTS WE DONT OVERRIDE IT AT RUNTIME
17.     [SerializeField] HealthData healthData; //CAN BE NULL IN HUMAN OBJECTS
18.    
19.  
20.  
21.     public void Set(HealthData value) { //ONLY CALLED IN HUMAN OBJECTS TO GET DATA FROM SCRIPTABLE OBJECT
22.         healthData = value; //NOT CALLED ON ENV OBJECTS
23.     }
24. }

Thank you very much, I appreciate your help. Of course you can ask as many questions as you want, it's my fault for not explaining properly.

 

I didn't ask relationship.

I asked. Why would MovementSettings change independent of the Movement script?

Or in this health example... why would HealthData change independent of Health script?

What condition is requiring us to call 'Set' aside from spawn/despawn? You'd mentioned you have to do this any time the data changes.

 

Oh because when AiSo changes, all her stats will change as well. I need to notify the components about this change in his max health, the skills he can hit, etc.

So, in object pooling, we spawned an AI with 500 health, then we disabled it and sent it to the pool. We will spawn it again as a different AI and this time its max health will be 300 instead of 500. That's why we need to change the data

 

OK, so it's still only on spawn/despawn that this is happening then.

The way you had previously written it implied that maybe mid existence of the visual side the MovementSettings would change and Movement had to be udpated.

Really though... all it is, is that these objects are pooled and therefore recycled. And so this time it reprsents Agent A who has 1 configuration, and next time its Agent B who has a different configuration. It's the same recycled prefab, but represents different objects.

So it appears my assumptions were actually correct for the most part.

...

With all that in mind I think you're sort of on track with your 'DataHolder' idea.

There is a certain aspect of the interface though that I'd keep. But I feel like we need to invert the control. Effectively take a page from the DI book. In most DI frameworks you have an initialize method that can take in a DiContainer or the sort which then the object can read out any dependencies it needs. This way we don't actually need to know what needs to be injected into the object (which is usually extrapolated via some attribute or shape of the initializer method).

You'll also notice that said 'DiContainer' source code often really is just your 'DataHolder' class with more bells and whistles attached (look nested containers and the sort):
https://github.com/modesttree/Zenje...ts/Plugins/Zenject/Source/Main/DiContainer.cs

Lets simplify this down though... make it work for what we need here.

...

So for starters I'm going to define some things to generalize stuff. For example you're using "AiSo" here, but like that implies this is for Ai. Why would it be for just that?

I like using the word 'Entity'. An 'Entity' is a GameObject and all of its children GameObjects and the components attached to all of them which is of significance. The top most GameObject of the Entity is its 'root'. Examples of entities could be NPCs, Mobs, the Player. But it doesn't just have to be characters/actors, it could be interactables like chests or doors or what nots. Effectively... they're significant. Set decorations aren't necessarily entities... a rock isn't necessarily an entity. But a rock that can be rolled around and interacted with and even has health so it can be broken... that's an entity.

Effectively... Entities have 'state' for which we care about.

So when I say Entity, that's what I mean.

In your case the entities are what are getting pooled.

...

Alright so what we will need to do is be able to:

1) Represent entities
2) Spawn/Despawn entities with the option to pool them
3) Represent the state of the entity independent of the entity itself
4) Initialize the entity passing along said state when spawned (regardless of pooling)
5) Retrieve the state of the entity when despawned for storage if necessary (save state? between scenes? whatever)

Represent the Entity:

Code (csharp):

1. public sealed class Entity : MonoBehaviour
2. {
3.  
4.     [SerializeField]
5.     private string _assetId; //this is an id that uniquely represents the prefab... 2 instances of the same prefab would have the same AssetId
6.                             //I would probably use a guid tied to the asset, see: https://forum.unity.com/threads/getinstanceid-what-is-it-really-for.293849/#post-9783387
7.  
8.     [System.NonSerialized]
9.     private string _stateId; //this is an instance id that unique to this instance of the entity... 2 instances of the same prefab would have different StateIds
10.  
11.     public string AssetId => _assetId;
12.     public string StateId => _stateId;
13.  
14.     public void Initialize(EntityStateContainer container)
15.     {
16.         _stateId = container.StateId;
17.         //will come back
18.     }
19.  
20.     public void OnRelease(EntityStateContainer container)
21.     {
22.         //will come back
23.     }
24.  
25. }

Represent the state of the Entity independent of itself (your 'DataHolder'?):

Code (csharp):

1. public class EntityStateContainer
2. {
3.  
4.     public string AssetId;
5.     public string StateId;
6.  
7.     private Dictionary<string, object> _data = new();
8.  
9.     public object this[string key]
10.     {
11.         get => _data.TryGetValue(key, out object data) ? data : null;
12.         set => _data[key] = value;
13.     }
14.  
15. }

Way to initialize as well as retrieve state:

Code (csharp):

1. public interface IEntityMember
2. {
3.     void Initialize(EntityStateContainer container);
4.     void OnRelease(EntityStateContainer container);
5. }

And the factory to spawn them (I'm super simplifying this with comments cause I don't feel like writing it all):

Code (csharp):

1. public class EntityFactory : ScriptableObject
2. {
3.  
4.     [SerializeField]
5.     private Entity[] _prefabs;
6.  
7.     [System.NonSerialized]
8.     private Dictionary<string, EntityPrefabEntry> _prefabMap = new();
9.  
10.     void Awake()
11.     {
12.         foreach (var prefab in _prefabs)
13.         {
14.             _prefabMap[prefab.AssetId] = new EntityPrefabEntry() {
15.                 prefab = prefab
16.             };
17.             //initialize the pool however... could late initialize if you want too
18.         }
19.     }
20.  
21.     public Entity CreateInstance(Vector3 position, Quaternion rotation, EntityStateContainer state)
22.     {
23.         if (_prefabMap.TryGetValue(state.AssetId, out EntityPrefabEntry entry)
24.         {
25.             var instance = entry.CreateInstance(position, rotation);
26.             instance.Initialize(state);
27.         }
28.     }
29.  
30.     public EntityStateContainer Release(Entity entity)
31.     {
32.         var container = new EntityStateContainer();
33.         entity.Release(container);
34.         //TODO - put in pool whatnot????
35.         return container;
36.     }
37.  
38.  
39.     class EntityPrefabEntry
40.     {
41.         public Entity prefab;
42.         public IObjectPool<Entity> pool;
43.      
44.         public Entity CreateIntance(...) => //return instance from pool or directly... whatever
45.      
46.     }
47.  
48. }
49.  

And I'm going to retroactively go back and modify our Entity for those "will come back" comments:

Code (csharp):

1.     public void Initialize(EntityStateContainer container)
2.     {
3.         _stateId = container.StateId;
4.         //we're back
5.         foreach (var o in this.gameObject.GetComponentsInChildren<IEntityMember>())
6.         {
7.             o.Initialize(container);
8.         }
9.     }
10.  
11.     public void OnRelease(EntityStateContainer container)
12.     {
13.         container.AssetId = _assetId;
14.         container.StateId = _stateId;
15.         //we're back
16.         foreach (var o in this.gameObject.GetComponentsInChildren<IEntityMember>())
17.         {
18.             o.OnRelease(container);
19.         }
20.     }

And then finally an exampel EntityMember:

Code (csharp):

1. class Health : MonoBehaviour, IEntityMember
2. {
3.  
4.     [SerializeField]HealthData _healthData;
5.  
6.     void IEntityMember.Initialize(EntityStateContainer container)
7.     {
8.         _healthData = (container["HealthData"] as HealthData) ?? throw new System.InvalidOperationException("EntityStateContainer requires a HealthData");
9.         //unsure how you want to actually deal with not retrieving data... your factory pattern should probably have something for that
10.     }
11.  
12.     void IEntityMember.OnRelease(EntityStateContainer container)
13.     {
14.         container["HealthData"] = _healthData;
15.     }
16.  
17. }

 

Yes, the way you showed it makes much more sense, I hadn't thought of that. Your code cleared my mind so much, now I know what to do.

Thank you very much for taking so much time to explain it in detail.