Search Unity

  1. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

Does it make sense to use serializereference to add components to objects?

Discussion in 'General Discussion' started by CPTANT, Oct 4, 2023.

  1. CPTANT

    CPTANT

    Joined:
    Mar 31, 2016
    Posts:
    80
    Hi guys,

    I have a question on how to best organize my objects. So what I usually have is some enemy and this enemy has several distinct scripts - movement - attack - health etc

    What I currently have is that these are all monobehaviours on an enemy gameobject, usually with one overarching class that has references to everything.

    Now I was wondering if this is the best thing to organize, because all these components don't necessarily need to be monobehaviours and I think having them be MB makes everything a bit messy to organize because it just becomes an ever growing list of components.

    So I was thinking, does it make sense to use serialize reference for this? I have one base script and I use this to hold the serliazed references to the other components like you can see in the image. This way I can easily divide everything in tabs and everything that belongs together also stays together.



    Does this make sense or am I missing something?
     

    Attached Files:

  2. PanthenEye

    PanthenEye

    Joined:
    Oct 14, 2013
    Posts:
    2,012
    Why not? Use it however you like. Anything that removes the Monobehaviour overhead is good in my book.
     
  3. neginfinity

    neginfinity

    Joined:
    Jan 27, 2013
    Posts:
    13,489
    First you need to stop and being to think what are you trying to achieve by doing this. If you can't answer this question, you're trying to do something strange.

    Normally when you work with monobehavior, you'd want to have as little manual configuration as possible. If you're adding references, I suppose it means you'll have to configure fields manually. Which means that you'll forget to do it in practice and will have broken objects you won't notice until you run the game, wasting time.

    Another issue is that you may be splitting components too much. There's no point in making an "Attack" MonoBehavior. You'd normally want "MovementController", "AiController", maybe "Weapon" or "WeaponCollider". See the difference? Higher level, larger concept. And "Health" would be a contained within something like "Combatant", which in turn can be either a player or an NPC.
     
  4. CPTANT

    CPTANT

    Joined:
    Mar 31, 2016
    Posts:
    80
    What it achieves is extendable classes with single responsibility whose parameters can be easily set through the inspector. It groups all functionality together that needs to go together without dumping everything in one huge script is is easier to organize than monobehaviours.

    Why would I reference fields manually? GetComponent does it for mono behaviours and for serialized objects they have to be there otherwise you can't set its parameters anyway.

    Why, that seems to break single responsibility really hard
     
  5. DragonCoder

    DragonCoder

    Joined:
    Jul 3, 2015
    Posts:
    1,635
    Are you familiar with ScriptableObjects?
    It sounds like you are using Monobehaviors to essentially store configurations and that's usually better delegated to SOs.
    Those can then nicely be assigned to Monobehaviors in the scene, other SOs and also prefabs without Serializablereference (just a serializable field of the corresponding SO type (including abstract parent classes) instead).
     
    Last edited: Oct 4, 2023
  6. DragonCoder

    DragonCoder

    Joined:
    Jul 3, 2015
    Posts:
    1,635
    Mh, depends on the definition. If the responsibility is "handle player movement" then MovementController fulfills that.
    If you limit a responsibility to something narrow as "applies forces to player rigidbody" then you'll end up with WAY too many classes in a game, if you ask me. Games are not web microservices.
    In Unity too many classes end up hard to maintain in the editor itself if your objects have two dozen each. Single responsibility does not magically make complexity disappear.

    I'd suggest a hybrid approach: Encompassing classes are okay but make methods be true "single responsibility".

    When a class that way really grows beyond a few hundred code lines, consider the "partial" keyword if full splittling isn't convenient. That keyword works well with Unity even with monobehaviors as it suffices if one file is named like the type for Unity to recognize it.
     
    angrypenguin likes this.
  7. PanthenEye

    PanthenEye

    Joined:
    Oct 14, 2013
    Posts:
    2,012
    Asset spam soon becomes an issue. You also can't new() an SO or have unique instances by default for runtime modifications. And CreateInstance() feels icky to use for what is an asset when you just need the data. I've tried many a scriptable object architecture framework and they always introduce more problems than they solve. Imo nested [SerializeReference] is far easier to manage since you get everything you need in a single prefab with all the benefits of a plain class that's freely creatable and modifiable in code.

    I wouldn't hard reference specific components though, I'd have a List<Component> that can contain 0-up to any amount of components with a common interface or baseclass. Albeit, data config is def easier to handle with tabs. Perhaps ergonomics outweigh flexibility in OPs case.
     
    Last edited: Oct 4, 2023
  8. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    7,279
    I compose behaviour via SerializeReference pretty much all the time at this point, among the many other uses SerializeReference provides. At this point I think plain C# classes outnumber my monobehaviours 10 to 1, if not more.
     
    CodeRonnie and PanthenEye like this.
  9. neginfinity

    neginfinity

    Joined:
    Jan 27, 2013
    Posts:
    13,489
    Do you need extensibility? Will that result in practical improvement of your game? And taking "single responsibility" too far results in bazillion tiny classes. You're experiencing that right now. That's why I do not follow SOLID.

    You can use C# classes to store small objectsa/concepts/data blocks and repeating patterns. Thing can be enclosed within monobehaviour without being monobehaviors. Use System.Serializable and SerializeField for that.
     
    Ryiah likes this.
  10. PanthenEye

    PanthenEye

    Joined:
    Oct 14, 2013
    Posts:
    2,012
    That's what OP is doing, just with [SerializeReference] instead of [SerializeField] for abstract classes and/or interfaces so concrete implementations can be guaranteed.
     
  11. angrypenguin

    angrypenguin

    Joined:
    Dec 29, 2011
    Posts:
    15,606
    That's what most people do, yes, but it's not a necessity at all. It's entirely possible to automate configuration for many things inside the Editor, which has the benefits of reducing manual configuration (and all of the issues which come with it) and reducing overheads at load time (usually small, but not always).

    For sure. I love the Single Responsibility Principle, but it's only useful after you've got reasonable practice at deciding on and defining responsibilities effectively. If you make responsiblities too small then it makes life harder rather than easier. If you make them too big then it doesn't give you a useful sanity check. And any which way, these are all useful prompts to help you to make better practical decisions, but they are just that - prompts.

    Could be a language thing, and each to their own anyhow, but that sounds a lot like throwing the baby out with the bathwater. I sometimes use SOLID, I sometimes don't. I pick and choose the right tool for the job. I certainly wouldn't discard -OLID in a case just because I decided S- didn't apply.
     
    CodeRonnie, DragonCoder and spiney199 like this.
  12. neginfinity

    neginfinity

    Joined:
    Jan 27, 2013
    Posts:
    13,489
    If you use it only sometimes, then it is not a very sound principle, as a sound principle should work in most cases. The most reliable approaches I know are KISS/YAGNI with a heavy dose of Murphy's law. The rest seem to be an acquired taste more often than not. And sometimes people get religious about it. Or start a cargo cult.

    The purpose of principles in the first place is to prevent you from wasting time and make you avoid programming traps. SOLID includes a programming trap within it, as demonstrated by the OP.
     
  13. angrypenguin

    angrypenguin

    Joined:
    Dec 29, 2011
    Posts:
    15,606
    Fine then, it's not "sound". I don't care, at all. It's useful, in that it prompts me to consider things I've found helpful, so that I can make a decision for myself. No more and no less.

    That might even be what you meant by "not follow", which is why I said it could be a language thing.
     
    DragonCoder likes this.
  14. CPTANT

    CPTANT

    Joined:
    Mar 31, 2016
    Posts:
    80
    What do you mean with automating configuration in this context? Where would this pull its values from?
     
  15. DragonCoder

    DragonCoder

    Joined:
    Jul 3, 2015
    Posts:
    1,635
    Not sure that's what they meant, but one way to do is to have GetComponent<> calls in the Awake or Start methods.
    This may result in less obvious behavior though and reduces flexibility. One can also do both to help with that by only calling GetComponent if the value is null; that way you have the option to override it in the editor.
    Note that GetComponent can be a little bit noticeable performancewise, making Object-Pooling more necessary if you use this for prefabs that are instantiated often or multiple at a time.
    If you are really brave, there's also GetComponentInParent and GetComponentInChildren, but some people here will probably throw rocks at me now for that suggestion :D

    P.s. for globaly unique objects, one can use the Singleton pattern as a source to pull the references from. Make sure to do allow optional overriding in the editor so that you can still test these and do not add too many ref-provider constructs to your project so there's never confusion on where the refs come from.

    Edit: If it's solely for the editor, one can use GetComponent in the OnValidation event which is being called by Unity when the object refreshes inside the editor.
    Naturally that won't always work for prefabs because OnValidation is not called for those when instantiated at runtime (but it does get called in the prefab-editor so you can use this system for all components that are part of the same prefab).
     
    Last edited: Oct 7, 2023
  16. neginfinity

    neginfinity

    Joined:
    Jan 27, 2013
    Posts:
    13,489
    Through GetComponent, GetComponentInChildren, GetComponentInParent
     
    Last edited: Oct 7, 2023
  17. angrypenguin

    angrypenguin

    Joined:
    Dec 29, 2011
    Posts:
    15,606
    From whatever logic you can program which will get the correct values. GetComponent and co are probably the most common approach, as suggested by @neginfinity already, but it can really be whatever you want.

    As one example, a project I'm working on has lights which turn on when all nearby things have been found. At first, each light had a list of relevant objects which humans added stuff to. You can guess how often the lists were out of date (hint: almost all of the time) and how long it took to update them (hint: a few minutes when people remembered and bothered to do it, but note that "a few minutes" stacks up when it has to happen each time a level changes).

    My solution was to add an Editor menu function which does the following:
    1) Find all of the lights in the open scene and clear their lists.
    2) Find all of the relevant objects.
    3) For each relevant object, look up the nearest light and add it to that light's list.

    It's less error prone, it's faster, people actually do it (most of the time). I could make it so they don't even have to press the button, but I'm always hesitant to write code which changes scenes or objects without very specifically being asked to do that ('cause what if one day we deliberately want an object hooked up to a different light?).

    You can also put context menu items on Components, make custom Editor windows and Inspectors, and hook into some Editor events. Basically, for most things with a single correct value that can be determined algorithmically, you can get the Editor to set it up for you.
     
  18. oscarAbraham

    oscarAbraham

    Joined:
    Jan 7, 2013
    Posts:
    431
    Hi from the future. I hope this is still useful. I think there are some purely technical problems with SeializeReference that may make you reconsider your approach. You should know they are a bit buggy and quirky, specially with with prefabs.

    For example, currently there are bugs when an instance overrides a managed reference field, or the order of references in a list, and the reference gets deleted from the prefab asset. It leaves prefab instances in a bad state that is hard to fix from the Editor, especially when the instances' scene is closed while it happens. A workaround is to use PrefabUtility.SetPropertyModifications in a custom Drawer/Inspector to ensure all [SerializeReference] fields (and the lists that contain them) are overridden before allowing them to be edited. This is just an example, there are more bugs related to prefabs like they can cause the Editor to crash, but they are harder to reproduce, and they sometimes change with Unity versions.

    There are other non-prefab related problems, like you can't really use them to edit multiple components when multiple Objects are selected. And they are a bit of a pain to handle when you rename a namespace or an assembly. I say this while also really liking [SerializeReference]. I just think there are a bunch of usability problems that MonoBehaviors don't have.

    If needed, the MonoBehavior could be instantiated with good defaults for its [SerializeReference] fields. Those defaults could be set in the constructor, the Reset method, or with a default preset that's editable in the inspector. That way, more than adding an essential reference, one would change it when it's necessary.

    If one is using ScriptableObjects mainly for configuration as DragonCoder suggested, there wouldn't be much need to instantiate them at runtime. That said, one approach I use when the data in a SO needs to be instantiated is to just instantiate the data, instead of instantiating the whole SO; here's a basic generic implementation that makes it easier. Sometimes I use ScriptableObjects as key's in a dictionary and their instantiated data as the values in that dictionary. I do prefer using prefabs to define these kinds of configurations, but sometimes SO's combination of modularity and reusability fits things better.
     
    CPTANT and CodeRonnie like this.
  19. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    7,279
    There is the
    UnityEngine.Scripting.APIUpdating.MovedFrom
    attribute that is still not documented. But yes it is something you need to consciously think about.

    You are right though that there's issues with SerializeReference, though these are more to do with Unity's wonky prefab serialisation and how various property drawers handle managed references, as opposed to the serialisation itself. I've noticed I hardly run into these issues with Odin Inspector drawers as their implementation is a lot better than Unity's (surprise surprise).

    At least these issues don't crop up with scriptable objects.

    And I don't think these issues should prevent you from using SerializeReference as the benefits pros far, far outweigh the cons.
     
    CodeRonnie likes this.
  20. oscarAbraham

    oscarAbraham

    Joined:
    Jan 7, 2013
    Posts:
    431
    Even if you know about it, it still can be a hassle. Can't it? When you rename an assembly, suddenly you have dozens of classes that need that attribute.


    The bugs I mentioned first are about serialization. You can try it:
    1. In a Prefab Asset with a [SerializeReference] array or list, add some managed references.
    2. Modify values inside the corresponding managed reference in a Prefab Instance.
    3. Close the scene that contains the Prefab Instance.
    4. Empty the [SerializeReference] list in the Prefab Asset.
    5. Now open the scene again. Errors should appear in the console.
    For another version of this issue, in step 2, instead of modifying values, try reordering the managed references. I don't remember which version of the issue is worse, but one of them leaves you unable to edit the component. I hate these kinds of issues because they can be noticed a good while after triggering them, and it's harder to fix them by then.

    I feel like Unity doesn't test a lot of their features with prefabs, at least it appears so with [SerializeReference]. There are a good number of bugs and edge cases.

    EDIT
    Fixed some typos.
     
    Last edited: Oct 10, 2023