Search Unity

  1. Welcome to the Unity Forums! Please take the time to read our Code of Conduct to familiarize yourself with the forum rules and how to post constructively.
  2. Dismiss Notice

Question How to write code in Unity ?

Discussion in 'Scripting' started by ClearSY, Jul 15, 2023.

  1. ClearSY

    ClearSY

    Joined:
    Feb 11, 2023
    Posts:
    15
    I have experience working as a backend developer and I do game development as a hobby. However, whenever I attempt to create a game, the code ends up looking messy and unorganized. Additionally, I find it difficult to avoid using classes like "Game Manager" or "Time Manager", etc. Another question is that I'm confused because every class I've written is inherited from MonoBehaviour, so I don't have Core logic like Entities(and all my game classes depend on Unity API - is it okay ?) . Could you please suggest some architectural and design patterns that you use in Unity ? Free tutorials shows only quick solutions (like example they use public fields for each script) for popular questions avoiding any architectural patterns
     
  2. Chubzdoomer

    Chubzdoomer

    Joined:
    Sep 27, 2014
    Posts:
    105
    Brathnann likes this.
  3. Brathnann

    Brathnann

    Joined:
    Aug 12, 2014
    Posts:
    7,140
    The only suggestion I can give is to start writing code and keep going. There are tons of tutorials out there, so you should keep looking and reviewing what people are doing and what techniques they use that you like. Also, just reading a c# programming book can help you develop patterns that work for you.

    Also, Unity has several things in their blog section about programming styles, patterns, etc.

    If you get stuck, showing your code and asking questions can be a great way to start learning.

    But, it's difficult to just ask a broad question like that and really get a good answer. While many programmers have similar styles, you'll still end up with a bunch of different answers overall.
     
    Bunny83 and ClearSY like this.
  4. ClearSY

    ClearSY

    Joined:
    Feb 11, 2023
    Posts:
    15
  5. KillDashNine

    KillDashNine

    Joined:
    Apr 19, 2020
    Posts:
    449
    I'm also backend dev originally, so hello. I will answer from my own experience after a couple of years with Unity now.
    • Messiness is a bit of a thing because coding is often experimental. I started off by separating everything with interfaces but I got rid of them because they ended up being just clutter.
    • This is scripting and 100x simpler environment than backend. Everything ends up being simple in the end, therefore having clean everything is not as important as it is in the backend. If something doesn't work, you can usually figure out the problem in a relatively short time, and you basically can't cause too much trouble by being messy.
    • Use namespaces and place code in equivalent folder hierarchy
    • Use prefabs for everything. The prefab parameter override system can be a headache but it helps to try and edit the prefab objects directly - you can access them from the > sign in Hierarchy view
    • I use
      Debug.Assert
      (three parameter version) quite extensively. It sorta replaces testing which is not the same at all in Unity as in backend
    • Spend time naming your variables and parameters well and use static typing unless it's something very obvious.
    • Add lots of comment lines. Only don't add comment lines if you are sure it's redundant and your naming is good enough (Clean Code etc), I mean seriously write those lines there. The stuff you built last week you won't remember next week, so make sure to code-document why that one special bit was this particular way (cos you spent half a day googling and experimenting with it)
    • I also write Manager or Handler classes but I sort of see it as an AOP or microservice sort of thing in terms of design. One handler takes care of a certain thing solely. You don't need to worry about coupling because your game is a sole entity and will remain such.
    • For my gameobject design, I separate the Monobehaviours into aspects, one Monobehaviour handles one feature of a gameobject. It helps to keep Monobehaviours smaller, I don't think I have any that exceed 1000 lines. It also helps you decouple and create diversity into gameobjects because they don't all need to have all features.
    • A good way to decouple elements is to use events. I am using a third party event system, but Unity has its messaging. For example UI and your game should be decoupled with events.
    • Something I just recently started doing more: Build good Editor tools and make your gameobjects such that you can instantiate them from the game but also the Editor.
    So here's just a few, you're welcome. I'm back to coding :)
     
    DragonCoder, CodeSmile and ClearSY like this.
  6. ClearSY

    ClearSY

    Joined:
    Feb 11, 2023
    Posts:
    15
    Thanks !
     
    KillDashNine likes this.
  7. CodeSmile

    CodeSmile

    Joined:
    Apr 10, 2014
    Posts:
    3,899
    The word "Manager" in a type name is a code smell!

    This innocuous word gives away that the purpose of the type is not clear, or deliberately left open to evolve into a "monster" of a script, a "blob of code".

    What does a "Manager" do? Quite possibly a LOT of things. Try to come up with a definition out of the blue and you'll likely struggle to explain it. ;)

    Here's the first definition I found via google:
    It is absolutely vague. It also says it's a "process", and makes it clear that "controlling things" is its main purpose. Whereas "dealing with" couldn't be anymore open to interpretation - like, it could literally mean "does anything".

    So ... what does a GameManager do? Does it "manage" the game? What does that entail? Same questions for TimeManager, or any other "Manager" type class.

    That's where the problems start: the use of "Manager" classes in the Unity community is over-inflated to the point that it has become an almost de facto standard to name any class "SomethingManager". Unity's own API is to blame as well to some extent, though there are only 11 types with "Manager" in their name, their examples often use the "Manager" suffix.

    If you force yourself to NOT use "Manager" (or one of its many, similarly non-descriptive replacement words) in a class name, that's how you start with better separation of concerns. For instance, the GameManager for all I've seen in the past, could and should often be separated into classes like:
    • QuestProgress
      • quests completed, failed or in progress
      • quest counters / timers (for repeatable quests)
      • unlocked levels and locations
    • GameSettings
      • Volume, Difficulty, UI settings, input settings, etc.
    • SceneLoader
      • Load (or download) additional scenes, callbacks
    • PlayerStats
      • Current Health, Mana, Experience, Level, Skills, whatever
    • EnemySpawner
      • Spawns enemies but may also ensure they don't exceed a given number
    • Inventory
      • a database for storing instances of whatever the game considers "inventory items"
    Note how each of these classes would be less informative if they used the "Manager" naming scheme:
    • QuestManager (Sir, I urge you to proceed with quest #49 lest you may fail to attract the interest of the young princess of Castle Quagmire.)
    • SettingsManager (uh .. what kind of settings need management?)
    • SceneManager (hmmm ... does it also "manage" the active scene in some way and it's also in conflict with Unity's SceneManagement / EditorSceneManager class)
    • PlayerManager (it befuddles me that the player needs to be managed)
    • EnemyManager (hey, Ork #34 go over there! Imp #13 stop giggling, this is your last warning!)
    • InventoryManager (according to my records, we are running low on health potions, let's order some more)
    Observation on the side: I've never heard of anyone using an EditorManager for their editor scripting needs, but GameManager usage is abundant.

    Avoid MonoBehaviour where possible

    One other thing worth noting is to force yourself NOT to use a MonoBehaviour for everything. In theory and often in practice, the MonoBehaviour exists ONLY to provide context to some operations - by this I mean mainly the serialized fields shown in the Inspector and the Unity event methods such as OnEnable, Update, etc. and providing access to the GameObject instance the script is attached to.

    For complex systems specifically you should strive to use the MonoBehaviour as the ModelViewController to your underlying Model classes. For example whenever I need something that is more than a simple collection like a class that manages a grid (2D array), or possibly chunks of grids, then those will be non-MonoBehaviour classes. The MonoBehaviour also acts as an interface for other classes accessing/modifying that data but it will not give away the internals of its data structures.

    Possibly the worst thing you can do not just in Unity is to have a type that provides public write access (read-only is fine) through a property or worse, a field. Any collection type MUST be encapsulated in a class and NEVER exposed to any other classes outside the type. This is possibly the single most important rule to follow that prevents code to be heavily entangled, hard to change and maintain, and full of bugs.
     
    Last edited: Jul 16, 2023
  8. ClearSY

    ClearSY

    Joined:
    Feb 11, 2023
    Posts:
    15
    Could you please tell more about it ? You mean i should create non - monobeahivour entities ? What is View and Controller then ? Thanks
     
  9. _geo__

    _geo__

    Joined:
    Feb 26, 2014
    Posts:
    1,111
    MonoBehaviours are popular because they provide easy ways to
    A) hook into the objects lifecycle: https://docs.unity3d.com/Manual/ExecutionOrder.html
    B) store data that is editable in the inspector in the scene.

    Both things can be done without them though.

    A) You can use just one single monbehaviour to hook into the lifecycle methods and then use these to pump your game logic (you can also hook directly into the player loop). This way you keep control. The order of Unitys magic functions (which Awake() gets called first?) is not defined so this setup may save you a lot of headache when initializing a microservices system as mentioned above.

    B) Storing data on objects in the scene can be replaced (without losing editability in the editor) via scriptable objects: https://docs.unity3d.com/Manual/class-ScriptableObject.html
    Whether the data on your object is something that should be externalized to an SO is up to you. Personally I tend to externalize any data that may be referenced more than once (i.e. it's not only for that single object). A bit like D.R.Y. principle for data (you may know it as "normalization").
     
    ClearSY likes this.
  10. CodeSmile

    CodeSmile

    Joined:
    Apr 10, 2014
    Posts:
    3,899
    Yes. And at least one of them (the model’s “root” object) would be a serialized field in the monobehaviour. The Mb then just forwards any event method as needed.

    If the model needs configuration editable via Inspector I’d make another class or struct like “ModelSettings” that is [Serializable] and has public or serialized private fields. This object can be passed along to the model object either on start or with every update call depending on whether the settings are editor only or runtime settings.
     
    ClearSY likes this.
  11. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    36,563
    Welcome to gamedev! You're welcome to fight it but when your product lead decides to flip the entire game structure one week before worldwide, well, no time for a full rewrite, get out the ugly code and smash it into place with the big ugly stick and ship the game.

    But the impulse to "keep it clean" is a good one. Just recognize it can only get you so far. Be ready to think about systems used in a way they were not originally intended to, and be fast at wrapping your mind around the particular "areas of concern" that a quick refactor affects, etc.
     
    Ryiah and ClearSY like this.
  12. KillDashNine

    KillDashNine

    Joined:
    Apr 19, 2020
    Posts:
    449
    I'd just like to note, this is a scripting environment. If you write eg bash scripts, they have access everywhere and the writer is responsible for not wreaking havoc. Their writer has been granted system-wide trust.

    I think the rule of encapsulation is generally a good one, also in modern environments where we don't necessarily use OOP languages but we could use, say, bash inside a microservice container that exposes a public API. I just think that in the context of games your complexity rarely gets so bad that you'd actually need it, and rather the adherence to technical restrictions can give you unnecessary overhead.

    By saying this I definitely don't mean one should write web-like couplings in your games or any systems. I mean the fact that enforcing encapsulation by means of private variables is strictly not necessary, and, to some degree, goes against the spirit of the platform. You can (and should) have written coding conventions for your team how stuff is done, and everybody play by the rules. I think if stuff gets overly complex and buggy, this is rather indicative of lack of such coordination rather than OOP principles.
     
  13. zulo3d

    zulo3d

    Joined:
    Feb 18, 2023
    Posts:
    510
    I'm gonna tell my next manager that he's just a blob of human destined to become a monster.
     
    CodeSmile, CodeRonnie and MaskedMouse like this.
  14. ClearSY

    ClearSY

    Joined:
    Feb 11, 2023
    Posts:
    15
    I'm currently experimenting with everything I've read in this thread.I use ScriptableObjects, separate models from MonoBehaviour classes and call Unity Event functions in one script component. It would be great if someone gave a feedback
     
  15. KillDashNine

    KillDashNine

    Joined:
    Apr 19, 2020
    Posts:
    449
    When the player dies, you need to very slowly display a red "YOU DIED" and play a menacing sound effect.
     
    ClearSY likes this.
  16. Owen-Reynolds

    Owen-Reynolds

    Joined:
    Feb 15, 2012
    Posts:
    1,913
    I'm curious what back-end is here. In some frameworks it means working very much inside the framework, writing small callback functions, using mostly time-saving built-ins. Or in React, say, it means coding to build everything in modules with trees (letting it auto-tear-down and rerun that code as needed) with strict special rules for variable access. But in others it means simply knowing more about data-bases, writing larger programs, and being a better programmer in general than front-end coders.

    To put it another way, I think Unity either gives much more freedom than a back-end programmer is used to; or it gives less and seems to force using too many odd built-ins, depending.
     
    Kurt-Dekker likes this.
  17. ClearSY

    ClearSY

    Joined:
    Feb 11, 2023
    Posts:
    15
    For me, Unity programming is quite different from other types of programming. When I started I watched popular tutorials, and I noticed that many of them disregarded principles such as Single Responsibility, Encapsulation, etc .. which I'm used to follow in backend programming.Maybe it's a problem of tutorials that made by non professional developers
     
    CodeSmile likes this.
  18. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    5,769
    Worth considering most folks starting out with Unity are, well, kids, who want to make whatever the latest popular game is. Next to that is folks coming in with zero C# or programming experience in general. It's enough to learn Unity on it's own, adding C# and programming in general on top of that is a huge load. Imagine trying to learn SOLID on top of that?

    I imagine around 95% of people who set out to learn Unity (or game making in general, I should say) don't last a week.

    Most popular youtubers are personalities more so than professionals (as you've accurately noticed).

    You have the distinct advantage of being a programmer already. All you need to learn is Unity itself, and you can probably more or less apply your own preferred coding style and practices to your projects.
     
  19. KillDashNine

    KillDashNine

    Joined:
    Apr 19, 2020
    Posts:
    449
    Junior coders are usually eager to learn and eager to be coached and to have somebody to check their work and to provide assistance. They also need this and can't work on their own.

    Then theres more experienced coders who have acquired skills and can handle their energy, motivation, can learn things independently. But they often have a pitfall of being idealistic and adhering to strict rules, and they tend to think black-and-white and preach these rules as holy doctrines. It's a competitive thing, they want to advance and to show and prove their skill, and it's like that everywhere in life, not just IT. But then if everybody goes ahead and follows the idealist, this wont make things any easier for everybody because it's not about thinking for the product and the team in the first place but about showing off their knowledge and a competition of whose idea is right.

    Often it's only senior coders who know that strict adherence to doctrine is impractical, and all ideals need to be applied in a reasonable manner by weighing their benefits against their overhead. Wisdom is to know the rules, and to know when to break them. And this is something where one needs to prioritize product quality standards over personal preference. All rules are there to be used to produce high quality, but at the same time, all these rules are pointless and also counterproductive if they, when applied in the specific project at hand, fail to actually increase product quality.

    Having said that, in games, I just think that product quality requirements are significantly lower than, say, backend. And this all starts from the fact that if the game crashes, client data is not lost and you don't need to pay millions in damages and to be the news headline for "Company X customer data stolen, idiot coders to blame".
     
    Bunny83, ClearSY and Yoreki like this.
  20. SisusCo

    SisusCo

    Joined:
    Jan 29, 2019
    Posts:
    1,104
    I don't think I've personally experienced any pain points in practice from using "manager" classes myself. I feel like they are a pretty decent way of grouping multiple members that are all related to the same thing in a centralized location, where they are easy to discover.

    For example, let's say we had a class called PlayerManager that contained a handful of events, properties and methods related to player objects:
    Code (CSharp):
    1. class PlayerManager : Manager<PlayerManager>
    2. {
    3.     event Action<Player> PlayerChanged;
    4.     event Action<Player, string> PlayerNameChanged;
    5.     event Action<Player, int> PlayerHealthChanged;
    6.  
    7.     Player LocalPlayer { get; }
    8.  
    9.     void Register(Player player);
    10.     void Deregister(Player player);
    11.     void SetLocalPlayerName(string newName);
    12. }
    This class arguably doesn't follow the single responsibility principle.

    If one wanted to be really strict about always following the single responsibility principle, they could refactor this class into something like seven smaller classes, with much more specific names than a "Manager":
    Code (CSharp):
    1. class PlayerChangedEvent : Event
    2. {
    3.     void AddListener(Action<Player> listener);
    4.     void RemoveListener(Action<Player> listener);
    5.     void Invoke();
    6. }
    7.  
    8. class PlayerNameChangedEvent : Event
    9. {
    10.     void AddListener(Action<Player, string> listener);
    11.     void RemoveListener(Action<Player, string> listener);
    12.     void Invoke();
    13. }
    14.  
    15. class PlayerHealthChangedEvent : Event
    16. {
    17.     void AddListener(Action<Player, int> listener);
    18.     void RemoveListener(Action<Player, int> listener);
    19.     void Invoke();
    20. }
    21.  
    22. class GetLocalPlayerQuery : Query
    23. {
    24.     Player Execute();
    25. }
    26.  
    27. class RegisterPlayerCommand : Command
    28. {
    29.     Execute(Player player);
    30. }
    31.  
    32. class DeregisterPlayerCommand : Command
    33. {
    34.     Execute(Player player);
    35. }
    36.  
    37. class SetLocalPlayerNameCommand
    38. {
    39.     void Execute(string newName);
    40. }
    Now is this objectively better?

    I think there are some downsides to this approach:
    1. Encapsulation suffers; there's less opportunity for hiding implementation details when multiple classes need to implement similar kind of functionality.
    2. The dependency graph gets more complex with more objects being injected to more objects.
    3. Discoverability gets worse; it can (in my experience at least), be easier to find functionality grouped inside 25 manager classes rather than 250 tiny classes.
    4. There's more code, more classes, more "noise".
    5. The API for all clients becomes a bit more complicated. Instead of acquiring one object and calling three methods on it, they'll need to acquire three different objects and call a method on each one.
    I've worked on an MMO game that had more than 50 manager classes, and I don't think this was ever a pain point of any kind for anybody in the team.

    But I think having the concept of managers did make it easier to know how new features should be implemented into the game. Need to implement notifications? Start by adding a NotificationManager.

    I think the abstraction of a manager helped make the code base more unified. It also made it easier to plan, discuss about and enforce architectural rules (e.g. "UI layer should not communicate with managers directly, there should always be a Data Handler in the middle").

    We did have a couple of managers that ended up with more than 1000 lines of code in them, which didn't feel ideal, and did make it a bit more difficult to work with them - but still I feel like having the concept of managers was a net positive for reducing the overall complexity of the project.

    So I think these sorts of higher level abstract concepts like "manager", "service", "handler", "builder", etc. can be useful to have in a code base, to help give everything order and increase cohesion.


    That being said, I do also recognize that sometimes people seem to add totally redundant suffixes to classes at random, that only serve to complicate things. Why is this component called a "PanelController", and not just a "Panel"? Why "InventoryHandler" and not just "Inventory"?

    Sometimes such things have some logical reason behind them, like avoiding name collisions, but sometimes people just stick the name of the design pattern being used at the end of the class name, which I don't personally think is always such a good idea.
     
    Last edited: Jul 17, 2023
    cerestorm, Ryiah and Chubzdoomer like this.
  21. KillDashNine

    KillDashNine

    Joined:
    Apr 19, 2020
    Posts:
    449
    I understood @CodeSmile s comment about Manager class code smell as directed only against naming classes in such a way. And I agree with him, and with the suggested name replacements, I use a similar approach myself.

    By code smell it is meant that "this is possibly a place of bad design also, as the naming is so bad".
     
    Last edited: Jul 17, 2023
  22. SisusCo

    SisusCo

    Joined:
    Jan 29, 2019
    Posts:
    1,104
    It's a good point that my refactoring example was probably taking things further than what @CodeSmile was suggesting.

    My main point is I personally haven't found the use of manager classes to be an issue in practice, so I don't personally see that word as a red flag.

    But I do agree with the sentiment that simple and exact names are good. I really like it when an API is pretty much like reading correctly spelled English:
    Player.Killed
    ,
    Inventory.GetItem
    etc.
     
  23. ClearSY

    ClearSY

    Joined:
    Feb 11, 2023
    Posts:
    15
    It's a good class(but PlayerHealthChanged ?). i was talking about "Manager" with > 300 lines of code
     
  24. KillDashNine

    KillDashNine

    Joined:
    Apr 19, 2020
    Posts:
    449
    It's about Clean Code and the idea that time spent to naming things is a part of the design process. XXXManager is the first word that comes to mind, but the idea is to spend a bit more time thinking about the names. This will improve readability of code but it also can improve design because that name says nothing about the actual purpose of the class. When the name is too generic, "GeneralManager", it sort of invites you to lump functionalities there that could be separated into separate aspects, and your manager can also grow too big in size.

    So the take-home from this would be to keep the manager classes smaller in size and more specific to purpose.
     
  25. SisusCo

    SisusCo

    Joined:
    Jan 29, 2019
    Posts:
    1,104
    I totally agree that spending time and effort to optimize names is very important.

    I think where my opinion starts to deviate a tiny bit, is in whether or not adding the
    -Manager
    suffix to class names can be more optimal than not.

    In a project with good architecture, I believe it should ideally be trivially easy to figure out where new code should be added when new functionality is needed. Grouping classes under generic, high-level categories like "managers" can help give some guidance in this regard, in my experience. When you're working in a code base with thousands of classes, stuff like this can matter quite a bit.

    In general I think that class names should be optimized a lot for discoverability and intuitiveness, while method names should be optimized a lot for definiteness.

    With this I agree 100% :)
     
    Last edited: Jul 17, 2023
  26. KillDashNine

    KillDashNine

    Joined:
    Apr 19, 2020
    Posts:
    449
    Point is, the suffix doesn't really add information. I think it's a remnant from SOA ages where it sounded cool to name things "Managers" because it highlighted that this is a service. It was cool that it was a service and not just a monolith subroutine, and you could call it from outside.
     
  27. SisusCo

    SisusCo

    Joined:
    Jan 29, 2019
    Posts:
    1,104
    To me the -Manager suffix on something like PlayerManager does imply quite a bit of information:
    1. It is clearly distinct from Player (it is not a player, it's something that oversees players).
    2. It exist on a higher layer of abstraction than Player.
    3. It exist on the same layer of abstraction as all other Managers in the project.
    4. It probably has a one-to-many relationship with Player (like managers in workplaces most often have more than one subordinate).
    5. If an object needs to acquire a reference to a Player, the way to acquire it is probably by requesting it from the PlayerManager.
    In addition to this, the word "manager" can encompass a lot more information within a particular project. You can add a summary to a base Manager class, and after a new developer joins the project, they can read this one summary, and immediately gain information relevant to dozens of classes in the codebase. For example, perhaps an instance of any Manager-derived class can automatically be retrieved from a service locator.

    A very similar example is the concept of "systems" in ECS. As a word "system" is also very high-level and nonspecific. But at the same time, in the context of the ECS package, it is an extremely useful concept to have for a lot of reasons, and I don't think that adding the suffix -System to various classes in a project is a code smell.
     
    Ryiah likes this.
  28. SisusCo

    SisusCo

    Joined:
    Jan 29, 2019
    Posts:
    1,104
    Some uncollected thoughts:

    Could rename WarriorFactory.CreateWarrior to just Create, because the type of the created object is already apparent from the name of the class (and more exact too from the name of the concrete class).

    Could rename AbstractWarrior to just Warrior for simplicity and brevity.

    Could wrap WarriorFactory members in a class like WarriorStats, and then remove _maxHealth, _maxStamina, _weight, _strength from AbstractWarrior, and instead just pass in a reference to WarriorStats in the constructor (flyweight pattern).

    AbstractWarrior.TakedDamage could be renamed to Damaged to be more consistent with the naming of the other events (and fix a mispelling).

    The abstractions IDamage, IAttack, IHealth feel off to me. You can now damage a warrior through any of these three interfaces - so which one should be used in which situation? Perhaps something more like this would be more intuitive:
    Code (CSharp):
    1. public interface IAttackable
    2. {
    3.     bool CanBeAttackedWith(Weapon weapon);
    4.     void Attack(Weapon weapon);
    5. }
    6.  
    7. public interface IKillable
    8. {
    9.     bool IsAlive { get; }
    10.     float CurrentHealth { get; set; }
    11.     float MaxHealth { get; }
    12.  
    13.     void Kill();
    14. }
    There are quite a few abstract classes related to one warrior entity: AbstractWarrior, WarriorFactory, WarriorView, WarriorPresenter, WarriorMovement. It feels like adding additional warriors using this system could be quite arduous and lead to code duplication. Using composition more instead of inheritance could potentially help avoid this. In Unity functionality is most commonly composed out of multiple components (attach Attackable component to an entity to make it attackable, attach Killable component to make it killable, attach Rigidbody component to give it a mass etc.).

    Consider making the Weapon class derive from ScriptableObject, to make it easier to create new weapons without any code modifications, and to tweak their stats on-the-fly during playtesting.

    The WarriorPresenter abstraction also feels weird to me. It's called a "presenter", but then it handles functionality like stamina regeneration, movement, jumping and updating UI views. I would try to get rid of this abstraction completely, and make all those different unrelated functionalities be handled independently by individual components (e.g. StatRegenerator, MoveHandler, JumpHandler).

    There's a class called KnightMono that derives from WarriorPresenter, which feels inconsistent; I'd expect all the derived classes to have the same suffix as the base class.
     
    Last edited: Jul 17, 2023
  29. ClearSY

    ClearSY

    Joined:
    Feb 11, 2023
    Posts:
    15
    Agree, it looks more reasonable.
    IDamage - for entity which can take damage(enemies, player, destructable objects, etc..)
    IAttack - for entities that can attack other objects
    IHealth - for entities that has health(player, enemies)
    I separate IHealth and IDamage because they have different purposes : for example when player takes damage he can lose stamina,etc.. so IHealth here responsible only for health.

    WarriorPresenter here is P from MVP : it presents AbstractWarrior(Model) to Unity(View).
    Using composition instead of inheritance sounds good, i will try it. Thanks for clarification
     
    Last edited: Jul 17, 2023
  30. KillDashNine

    KillDashNine

    Joined:
    Apr 19, 2020
    Posts:
    449
    I read through your list and didn't find anything to promote naming your class "Manager".

    You don't want any hierarchy to your game but events. This suits games particularly well. Your game responds to events. You have e.g. "Weather".. what it does is schedules and publishes weather events. Whomever needs to respond to weather events, listen to Weather.

    Your Player input consists of events. Player hit Menu key? Send event, and your GUI listens to it. Et cetera. These of course are a built-in platform feature.

    Your character is a huge source of events, stuff happens to him all the time! He changes movement, collides etc. All of these are events. Character hit the wall? Send event. Character got hungry? Send event. Pro character controllers have it already.

    EnemySpawner spawned an enemy? Send event. Enemy killed? Send event.

    All of this just with publish-subscribe pattern, and you're good.
     
    Last edited: Jul 17, 2023
  31. zulo3d

    zulo3d

    Joined:
    Feb 18, 2023
    Posts:
    510
    The manager is all present and all knowing. It tells you when you're doing good ..or bad. And it tells you when your time is up.

    Do not disrespect the manager!.

    Because there's no game without it..
     
  32. SisusCo

    SisusCo

    Joined:
    Jan 29, 2019
    Posts:
    1,104
    You could also achieve the same effect with simpler components that raise events:
    Code (CSharp):
    1. public sealed class Attackable : MonoBehaviour, IAttackable
    2. {
    3.     public event Action<Weapon> Attacked;
    4.  
    5.     public bool CanBeAttackedWith(Weapon weapon)
    6.     {
    7.         foreach(var attackabilityAffector in GetComponents<IAttackabilityAffector>())
    8.         {
    9.             if(!attackabilityAffector.CanBeAttackedWith(weapon))
    10.             {
    11.                 return false;
    12.             }
    13.         }
    14.      
    15.         return true;
    16.     }
    17.  
    18.     public void Attack(Weapon weapon) => Attacked?.Invoke(weapon);
    19. }
    20.  
    21. public sealed class Killable : MonoBehaviour, IKillable, IAttackabilityAffector
    22. {
    23.     public event Action Died;
    24.     public event Action<float> Damaged;
    25.  
    26.     bool IsAlive => _currentHealth > 0f;
    27.  
    28.     bool IAttackabilityAffector.CanBeAttackedWith(Weapon weapon) => IsAlive;
    29.  
    30.     public float CurrentHealth
    31.     {
    32.         get => _currentHealth;
    33.  
    34.         set
    35.         {
    36.             value = Mathf.Clamp(value, 0f, MaxHealth);
    37.             if(_currentHealth == value)
    38.             {
    39.                 return;
    40.             }
    41.          
    42.             float damageTaken = _currentHealth - value;
    43.             _currentHealth = value;
    44.             Damaged?.Invoke(damageTaken);
    45.             if(value <= 0f)
    46.             {
    47.                 Kill();
    48.             }
    49.         }
    50.     }
    51.     public void Kill() => Died?.Invoke();
    52.  
    53.     private void OnEnable()
    54.     {
    55.         if(TryGetComponent(out _attackable))
    56.         {
    57.             attackable.Attacked += OnAttacked;
    58.         }
    59.     }
    60.  
    61.     private void OnDisable()
    62.     {
    63.         if(_attackable != null)
    64.         {
    65.             attackable.Attacked -= OnAttacked;
    66.         }
    67.     }
    68.  
    69.     private void OnAttacked(Weapon weapon) => CurrentHealth -= weapon.Damage;
    70. }
    These events could then be used to trigger different kinds of components that are attached to the same GameObject. For example:
    Code (CSharp):
    1. public sealed class OnDamagedAdjustStamina: MonoBehaviour
    2. {
    3.     [SerializeField] private Killable _killable;
    4.     [SerializeField] private Stamina _stamina;
    5.     [SerializeField] float _adjustAmount = -10f;
    6.  
    7.     private void OnEnable() => _killable.Damaged += OnDamaged;
    8.     private void OnDisable() => _killable.Damaged -= OnDamaged;
    9.     private void OnDamaged(float damageTaken) => _stamina.CurrentStamina += _adjustAmount;
    10. }
    You can also use UnityEvents to make it possible to hook the events up to any methods via the inspector (this is very powerful, but can also be quite fragile, with serialized data easily breaking when methods are renamed for example).
    Code (CSharp):
    1. public sealed class OnDamagedIntTrigger : MonoBehaviour
    2. {
    3.     [SerializeField] private Killable _killable;
    4.     [SerializeField] private UnityEvent<int> _onDamaged;
    5.  
    6.     private void OnEnable() => _killable.Damaged += _onDamaged.Invoke;
    7.     private void OnDisable() => _killable.Damaged -= _onDamaged.Invoke;
    8. }
    My worry with having three different methods for dealing damage to the warrior is that it could be very easy to forget to raise all the relevant events in the same order when damage is dealt through each of them.
     
    Last edited: Jul 17, 2023
  33. SisusCo

    SisusCo

    Joined:
    Jan 29, 2019
    Posts:
    1,104
    Ah, I see... then I would just remove the WarriorView class completely, and just hook WarriorPresenter directly to the Image components. I would consider the Image component to already be a View.

    After this change it makes sense to me for WarriorPresenter to act as a mediator between the Warrior and its views (images, texts, buttons...).

    (I would personally maybe also rename the class to something like WarriorUI for simplicity, but that's just me :D)
     
  34. KillDashNine

    KillDashNine

    Joined:
    Apr 19, 2020
    Posts:
    449
    Why hook up events to methods? Events are not messages. They don't travel, they just occur. Just write the events to your topics/queues and let anybody subscribe to listen who wants to.

    upload_2023-7-17_22-55-11.png

    Event data is serializable game data but luckily your events won't get too coupled and also you don't even need to carry much data in the events because you don't want to restrict your component visibility to one another. So, if your Player GUI part is triggered with "Player movement style changed" update from the event topic, this code can read the Player class to get the data that he needs. The important bit was that he knows he needs to update the GUI cos the event occurred and now's the time to do it.

    Any changes to saved serialized data needs save file version increase.

    If you don't know how to build this, you need some sort of event broker system for it. I myself use the one that comes with Opsive character controller, but any proper one will do that supports publish-subscribe and doesn't blow up. I think there's event systems on Asset Store or you can build your own. I was using my self-made one before switching to this other one cos I happened to have it. You can build it with Unity Events or without them. Opsive is using a DLL which is supposed to load GC less.
     
    Last edited: Jul 17, 2023
  35. SisusCo

    SisusCo

    Joined:
    Jan 29, 2019
    Posts:
    1,104
    If an event is raised and no one is around to hear it, does it make a sound?

    You do also want to hook up subscribers to the events - otherwise why raise them in the first place?
     
    Ryiah likes this.
  36. KillDashNine

    KillDashNine

    Joined:
    Apr 19, 2020
    Posts:
    449
    You don't hook them up, they subscribe. Now or at a later stage. In designing the events, an event is described as "something significant happened" and they should be a part of your game design, whether someone listens or not.

    Want to make an on-screen event log? Just hook it up. Want to build a second UI? Just hook it up. You don't need inspector for this, just a single line of code, typically in Start or Awake. And you cause no side effects so you don't need to control it manually. The broker handles the coupling.
     
  37. tleylan

    tleylan

    Joined:
    Jun 17, 2020
    Posts:
    521
    KillDashNine likes this.
  38. John_Leorid

    John_Leorid

    Joined:
    Nov 5, 2012
    Posts:
    620
    This is specifically about programming a game, right? And we want to be efficent, right?
    Then we should look at what is the core of programming any game, what takes the most time:
    Experimentation and Production. Those two often go hand in hand.

    Production means iteration, you add a new enemy, you change it, you remove it. A new weapon, a new combo attack, a new item with a very special behaviour. To be able to do this without ending up with a huge plate of spaghetti your code has to be flexible.
    (except the game is super simple, like hyper casual, a game jam game, walking simulator, .. but as you are worried about code architecture, I expect your games to have some degree of complexity)

    Flexible Code = Efficient Code for Games. And to make it flexible, you have to decouple it, so you can add/change/remove features without rewriting (or even affecting) 90% of the game each time.

    Decoupling always works in the same way: instead of Connecting two classes (A, B) like so:
    A <-> B
    you add something in between, like so:
    A <-> X <-> B

    X can be an interface, a seperate class, a scriptable object, an event system, even reflection or a text file (save game).
    What you use is entirely up to you, there is no silver bullet, I use pretty much all of them.

    Also there are some established patterns for certain use cases. As previously mentioned, the PubSub-Pattern is popular when it comes to things like "sending damage to an enemy". Statemachines are used for Enemy-AI & Player-Character-Controllers (sometimes even for menus). Component-Pattern for a list of weapons (all weapons deriving from WeaponBase, referenced in WeaponController which enables/disables them for example). The list goes on and on.

    The core is that you want flexible, decoupled code so you don't end up with Frankenstein-Classes which you basically have to rewrite entirely whenever you have to fix a bug or add/change/remove a feature.

    Btw. I'm working on a video on this topic since over one year, which hopefully should be released soon, link to my youtube channel is in my signature - there I will explain all of this in greater detail with a ton of animations (making those text-animations (highlighting code and stuff) is driving me insane but I'll push through it eventually)
     
    KillDashNine likes this.
  39. CodeRonnie

    CodeRonnie

    Joined:
    Oct 2, 2015
    Posts:
    280
    I like your description of decoupling. You have to put something in between. We would even probably call that thing a coupler in some cases in traditional physical engineering. To me the issue is whether that thing is simple or abstract enough for universal application, and whether it is widely adopted, just like adapters in the real world.
     
  40. KillDashNine

    KillDashNine

    Joined:
    Apr 19, 2020
    Posts:
    449
    It's not at all as simple as just putting something in between. Event-based architecture is very different from interfaces, they serve a different purpose.

    If you use an interface, you are basically creating a contract between two modules about how these modules act in relation to one another. However, in event-based architecture your new module just adds to the event stream and makes itself usable. So, rather than putting two things together and something in between, in event-based architecture you have a grid, and by bringing in a new module, you are adding something to the grid that produces a new event stream and can be used by any other modules. Of course, if you then unplug your module, those modules that use its event stream won't work anymore, but they won't break, and you don't need code changes. They just don't get input anymore.

    I personally don't use interfaces at all in Unity and I haven't found a place where I'd need them. I use only abstract classes. This is because interfaces are rigid contracts, and development just rarely goes the way that first you design the interface, then everybody follows that. Most of the time you are experimenting and the rest of the time you are cleaning up.
     
  41. CodeSmile

    CodeSmile

    Joined:
    Apr 10, 2014
    Posts:
    3,899
    I tend to disagree. That particular "PlayerManager" has three events that others can subscribe to, and the number of events would naturally grow as development continues. But its single purpose is to dispatch events. It's just that ... the name is off. :p

    I would simply call it: PlayerEvents.

    And most certainly not subclass it from a class "Manager<T>" which is ... wow. Absolute worst-case scenario: a "Manager" base class! :eek:

    This ought to be "EventHandler<T>" or something similar. Like this:
    class PlayerEvents : EventHandler<PlayerEvents>

    Instead of:
    class PlayerManager : Manager<PlayerManager>


    Problem is, if you start naming that particular class a "PlayerManager" it completely deludes the meaning of Manager for all other classes in the project / team since there is absolutely nothing that is being managed here - informed, relayed, notified, raised, registered, forwarded, dispatched perhaps but not managed.

    If anything, the very least it ought to have been called "PlayerEventManager". But then "PlayerEvents" would just be as descriptive, and shorter.


    Of course, not all "Manager" classes grow into a problem, and not in every team, and sometimes the name even makes sense. But I would call it a smell if a project has 50 Managers of all kinds (no wonder if there really is a "Manager" base class :p) because they are certainly not all having the same set of responsibilities and I doubt there is a team-wide consensus as to what responsibilities or tasks a "Manager" class is expected or allowed to perform. I'm sure about half of them could be easily renamed to provide more meaning and limiting the class' scope for future extensions, especially after seeing that particular PlayerManager. ;)

    Issues with bad naming often do not surface until the team rotates, a senior "Manager" leaves, new hires are onboarded, a major redesign is necessary, etc. That's when it shows whether a Manager class has had someone watch over the responsibilities of that class or whether it's an entangled mess with lots of unrelated dependencies and tasks that are really painful to decouple. Said PlayerManager is no problem now, but it will be as soon as devs start adding player stats to it, input handling, savegame functionality, acount management, achievements, ... after all, all of that is "Manager" territory.
     
    Ryiah and SisusCo like this.
  42. SisusCo

    SisusCo

    Joined:
    Jan 29, 2019
    Posts:
    1,104
    Did you by any chance miss the four other members? :)

    The example class was based on an actual manager in a project I've worked on. It's main purpose in practice was more about being a communication layer between backend services and world entities as well as the UI layer, delivering requests and responses (via the events) related to player entities between them. It would also act as a service locator for retrieving a reference to the local player entity, or one of its components. A little bit like a ViewModel in MVVM, but for the game logic layer instead of the GUI layer.

    Basically almost any time any entity or UI data handler needed to do anything related to a player entity, they'd request a reference to the PlayerManager object to be delivered to them via dependency injection, and then use its members to get the job done.

    Maybe the Manager abstraction could also have been named something like... ServiceGameAndUILayerCommunicationMediatorAndLocalGameLayerDataAndReferenceProvider. Or CrossLayerDataMutatorAndAccessor. In theory "Mediator" sounds like it could pretty much fit the bill, but in practice
    PlayerMediator
    just doesn't feel at all intuitive to me o_O

    Okay, I can now kind of see where you're coming from with this... a dictionary definition for "manage" would be something along the lines of "to be in charge of" or "to conduct affairs of", so perhaps naming a class "PlayerManager" could be seen to imply that it should have full authority over all the behaviour of player entities, or even that it should issue them commands in an autonomous manner. But in practice managers in software development tend to have a much more narrow set of responsibilities; like just initializing and disposing instances of a particular class and providing references to them on demand.

    That being said, words do also evolve over time, and can grow to have very different denotations in different contexts. I think that in the context of software development, when a developer sees an InputManager or a ResourceManager class, seeing just the name does in most cases already form a pretty good idea about the responsibility of the class in their mind. An InputManager probably is responsible for detecting user inputs and notifying relevant parties about them. A ResourceManager can probably be used to load assets using some sort of key.

    In some cases you could probably just as well use the plural form of the "managed" class instead of the
    -Manager
    suffix for the name:
    Resources.Load
    ,
    Inputs.GetKey
    etc. But I feel like this is in many cases only intuitive for static classes, when you tend to follow the name of the class with the name of a member. If there was a
    variable definition
    Audios audios
    , I don't think it'd be necessarily very clear for most developers what an object assigned to the variable can be used for - but I at least wouldn't have this problem if I saw the definition
    AudioManager audioManager
    instead.

    Actually the concept of managers in the project did always click really fast with everybody.

    The project had pretty clearly defined architectural structures for most things, so code had a tendency to get split properly between all the different categories of classes very naturally without much thinking being required.

    I've seen more than a dozen programmers work with the code base - many of them having joined only later into development - and nobody ever had any issues with the managers. I believe the same architecture with managers was also used in the company's previous game project.

    So in this particular case, it's hard for me to imagine that the manager pattern was a big hinderance, but nobody during several years happened to notice. We did do a lot of refactoring in the project too to improve naming whenever we noticed opportunities for this, so it's not like there was a culture of ignoring problems like this in the project.

    That's crazy talk! Clearly those things should be implemented in an InputManager, SaveManager, UserAccountManager and AchievementManager respectively :D
     
    Last edited: Jul 20, 2023
  43. KillDashNine

    KillDashNine

    Joined:
    Apr 19, 2020
    Posts:
    449
    I think in software development, when a developer sees those class names he's gonna be sent on a history trip to at least 15 years back in history aka stone age. Then he's gonna make a quick google search to see what Clean Code principles recommend regarding class naming such as here or here. And he will verify his intuition that said from the start that class names should be nouns or noun phrases and names like -Manager should be avoided.

    Maybe you pulled your project example from similar times?

    Please don't say that in a recruitment interview when they ask you this, or when they ask you to draw a design of a software on a whiteboard. Constrain yourself.

    I think this comes down to what I said earlier, that in games, quality control is just significantly less important, and precisely if you're making games in a pipeline, reusing your codebase with the same people, you'll repeat the same cos in games it's just whatever works.

    If your code works and that's how you've been doing things, don't fix what isn't broke. But this is a different question from the original question of how to properly name classes and to write code.
     
  44. SisusCo

    SisusCo

    Joined:
    Jan 29, 2019
    Posts:
    1,104
    One architectural pattern I highly recommend that could help, is grouping code by feature, and using assembly definition assets to promote decoupling between.

    So your project structure could resemble something like this:
    • Scripts
      • Worldmap
      • Persistence
      • Inventory
      • Tutorials
      • Cutscenes
      • Entities
        • Components
      • Combat
      • Logging
      • UI
        • Panels
        • Popups
      • Dialogues
      • ...
    You can group scripts for each feature under its own namespace, and separate them into their own assemblies. Try make it so that your game logic related assemblies (combat, entities) don't know anything about the existence of the UI assembly, for example.
     
    Last edited: Jul 20, 2023
  45. SisusCo

    SisusCo

    Joined:
    Jan 29, 2019
    Posts:
    1,104
    This is totally okay in my opinion.

    Pretty much the only reason you would want to spend a lot of time and effort trying to decouple everything from Unity, is if you think that there is a reasonably high chance that you might want to port the entire game to another engine at some point.

    Well, unit testing can be another reason. Unit testing MonoBehaviours can be a lot more difficult than testing plain old class objects. But there are ways to remedy this pain point, other than avoiding using components altogether,

    Components and scriptable objects are such a core part of Unity, that trying to avoid them could end up making your life much more difficult than it needs to be. You could find it more difficult to utilize great features like scenes, prefabs, addressables, LODs, animators, ECS etc. to their fullest, if you try to decouple your code from everything Unity.
     
  46. KillDashNine

    KillDashNine

    Joined:
    Apr 19, 2020
    Posts:
    449
    Assembly definitions are not an architectural pattern and have nothing to do with software architecture or code quality, and furthermore can only introduce more couplings to your game because cross-assembly references won't work unless the assemblies are linked. Asmdefs will help nothing to achieve modularity - if you want to separate your UI, like in your example, you need to do this in terms of architecture.

    Asmdefs are a Unity tool to control when code is compiled (they're compilation units), and the benefit is to potentially reduce compilation and load times and to target your code to different platforms.
     
  47. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    5,769
    I think this is a bit pedantic. Yeah assembly definitions are not, by the strictest term, an 'architectural pattern', but they are a good project organisation habit to get into.

    And honestly I find they reinforce good code architecture due to their one-way nature. Good code comes about in the ways we restrict ourselves from doing things that would result in poor or messy code. Assembly definitions are part of this, incentivising us to write reusable code with clean API's.

    If anything they're a bit of a 'super-architecture', maybe a paradigm, even, that sits above the rest of your code architecture.
     
    SisusCo likes this.
  48. KillDashNine

    KillDashNine

    Joined:
    Apr 19, 2020
    Posts:
    449
    The code level tool to affect visibility is the namespace, which exist exactly for this purpose. Separate your UI from your game by enforcing namespace.

    I repeat, asmdefs have no part in creating clean code. They are compilation units.
     
  49. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    5,769
    You say that like it's the only thing they do. A tool can have multiple uses.

    Namespaces are one means to organise code, but they do nothing to restrict access to a system some other system should have no access to. Also most IDE's don't give a naff about a namespaces and will still show everything and just auto-include namespaces when you use a type from them. So... so much for restricting visibility!

    Assemblies are a hard separation that you can use to ensure certain parts of your project don't touch others, and to keep relationships one way.

    They certainly make my code cleaner. :U
     
    SisusCo likes this.
  50. KillDashNine

    KillDashNine

    Joined:
    Apr 19, 2020
    Posts:
    449
    Your'e just mixing up code visibility (namespace) and deployment visibility (assemblies). You're mixing up what happens at compilation time and what happens at runtime.

    In fact, this "hard separation" you are in favor of, is hard exactly when it's enforced logically at compile-time, instead of practically at run-time. The said thing is just the same as static typing vs dynamic typing: if you want to have clean design and set hard boundaries, you need to use static typing.

    Speaking of how to write clean code, we are interested in source code. Runtime deployment is a separate topic. In Unity these might seem interrelated because there's not much going on with respect to deployment configuration, but you still need to keep the topic of source code separate from your runtime environment.

    The correct way to handle namespaces and import/using statements in your source code is the following. Agree on team-wide formatter rules or standardised coding conventions (such as the Google style guide) with respect to your import/using statements, and enforce these in your DoD.

    It is true that your deployment architecture has a lot to do with your overall architecture, but this doesn't apply in Unity which is a single-platform system, and which is exactly why the compilation and deployment environments get mixed up in the first place. That's why the asmdefs don't really have more functionality than just compilation or target platform targeting.
     
    Last edited: Jul 20, 2023