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. We have updated the language to the Editor Terms based on feedback from our employees and community. Learn more.
    Dismiss Notice

Architecture & Tight Coupling #1

Discussion in 'General Discussion' started by GarBenjamin, Mar 23, 2017.

  1. Dustin-Horne

    Dustin-Horne

    Joined:
    Apr 4, 2013
    Posts:
    4,568
    Where I find it useful is in easily extracting real logic away from presentation. I view components as behaviours or definitions and it's useful to extract the logic to make it modular and more testable. There are drawbacks though. Extracting the logic into a Singleton service makes thread safety and even bigger concern.
     
  2. frosted

    frosted

    Joined:
    Jan 17, 2014
    Posts:
    4,044
    If you look at the code you presented as an example. It is simply renaming methods on a dictionary<type, obj>.

    I don't think this is bad or wrong or anything. I actually think there is still value in this approach you shared.

    But the value really has nothing to do with the code itself (again, replace verbatim with Dictionary<Type,object>) - the value is in how you think about it, how the 'idea' influences your code base and the little benefits like "Find Usage" having more value.

    Just look at the code itself in a vacuum. It doesn't actually do anything. It's just there to communicate an idea to the user. But there is still a lot of real value in that !

    To take it a step further - you could functionally simplify the code example as this:

    Code (csharp):
    1.  
    2. public static class ServiceLocator<T>{
    3.   public static T Service{ get;set; }
    4. }
    5.  
    This is, essentially what the example does once you really break it down.
     
    Last edited: Mar 24, 2017
    Dustin-Horne likes this.
  3. Dustin-Horne

    Dustin-Horne

    Joined:
    Apr 4, 2013
    Posts:
    4,568
    Obviously a stripped down basic example but yeah. It provides some additional benefit by allowing generic constraints. Dictionary<type, object> would let you add anything. _references.Add(typeof(IDisposable), int) for example. The wrapper prevents that by giving you generic constraints, as well as disposal support for IDisposable objects, or whatever else you need to add to it.

    You could actually take it further and provide factories that would instantiate those services for you when needed, or whatever you needed to do. The point was less about the implementation and more about the idea behind the pattern and most importantly using the statics as a proxy and not the actual services themselves since they then cannot be replaced / faked / easily tested.
     
    Last edited: Mar 24, 2017
    frosted likes this.
  4. GarBenjamin

    GarBenjamin

    Joined:
    Dec 26, 2013
    Posts:
    7,441
    I just had a quick look at the code in your example @frosted ul (unfortunately it doesn't seem to load correctly in my version of Unity I got a message telling me the project was made in a newer version... but Notepad++ works fine to view the code of course)

    This seems like a lot of plumbing... lot of complexity just to get references out of the scene. That is my first thought. BUT... again I also see this stuff as a very personal thing. So if this your preferred approach more power to you.

    Ha ha! This thread is a tremendous example of why projects at companies that have lasted many years and went through many different developers are a random mess of different styles. It really is amazing how much variety there is in implementations. From someone directly wiring up the references per GameObject in the Editor to the way you are handling it. So many ways of accomplishing even the simplest task. This also explains why so few programmers can actually work together as an effective team.
     
    Last edited: Mar 24, 2017
  5. frosted

    frosted

    Joined:
    Jan 17, 2014
    Posts:
    4,044
    It may be a little hard to imagine if you can't see it in editor. The quick reference example has very close to least plumbing possible in terms of implementation.

    Screenshot

    These are simply fields. There is zero logic.

    Code (csharp):
    1.  
    2. public class IAmQuickReference : MonoBehaviour{
    3.   public WorldStuff SceneWorld;
    4.   public GuiStuff SceneGui;
    5. }
    6.  
    7. [Serializable]
    8. public class WorldStuff {
    9.   public Camera Camera;
    10.   public Light Sun;
    11. }
    12. ...
    13.  
     
    Last edited: Mar 24, 2017
  6. GarBenjamin

    GarBenjamin

    Joined:
    Dec 26, 2013
    Posts:
    7,441
    That one seems straightforward other than the serializable part. I use Serializable when exporting class data to XML for example. So not sure what you're getting from it here or actually using it for. Maybe there is some proprietary thing involving Unity automagically (I hate that lol) going on when it sees that attribute?

    Anyway it seemed like you had two different examples with the other implementing a messaging system and lot of abstraction. But again I only had a minute at most to look at all of the code files.
     
  7. frosted

    frosted

    Joined:
    Jan 17, 2014
    Posts:
    4,044
    The pub/sub is a little more complex, but not really.

    If you reduce my example the same way I reduced @Dustin-Horne's example it pretty much looks like:
    Code (csharp):
    1.  
    2. public static class PubSub<T>{
    3.  public static List<Action<T>> Subscribers;
    4.  public static void Publish( T msg ){ foreach( var s in Subscribers ) s( msg );
    5. }
    6.  
    Although instead of static i just dump it into a 'root' node in the scene. The example code does a little more than that - but this is the important part. Dustin's example wraps Dictionary<Type,object> mine wraps Dictionary<Type, List<Action<T>>>, so there's more a bit more complexity.
     
    GarBenjamin likes this.
  8. GarBenjamin

    GarBenjamin

    Joined:
    Dec 26, 2013
    Posts:
    7,441
    So the question is why did you write all of that other code if this is actually how it could have been written? Do you like complexity? Or did it just "grow" because you didn't have a clear view of what you were building when first writing it? LOL :)

    I'll take another look at it when I am off work later. Like I said I only looked at everything briefly. Basically just opened each file saw how much code was in them. All together I saw a fair amount of code which implied it must be fairly complex (considering the goal of getting references).
     
    frosted likes this.
  9. frosted

    frosted

    Joined:
    Jan 17, 2014
    Posts:
    4,044
    Basically yeah - I kept using it, and ran into annoying details - so I had to add x and y and z in order to deal with the stuff that got annoying.

    I also didn't realize that you could so often simplify the Dictionary<Type, x> thing with just using MyClass<T> so easily. It was actually Unity's internal ListPool<T> implementation that tipped me off (first saw it a couple months ago). I thought it was neat and let you cut out a lot of extra code when wrapping Dictionary<Type,...> which is really pretty common.
     
    GarBenjamin likes this.
  10. Dustin-Horne

    Dustin-Horne

    Joined:
    Apr 4, 2013
    Posts:
    4,568
    Yep, the only reason I generally still prefer the Dictionary approach is that you only have one static class instantiated and you can dump services when you no longer use them. If you have ServiceLocator<T> and you call ServiceLocator<ISomeService> and you don't need ISomeService after the first scene... you can still null out the service property but the ServiceLocator<ISomeService> hangs out forever. With the Dictionary approach you only have your ServiceManager as a static class and you can dump any of the registrations. It's trivial, and all down to personal preference.

    I actually implemented a similar Pub/Sub system with a couple of differences. It used types for the message data but used magic strings for the message names. I was always going to rework it but never did. But in addition to doing pub/sub it worked as both a queue and a stack. You could use it to deliver the message to all subscribers, or you could let the first listener peel it off the queue (so the first person that's ready for the message processes it).

    It was independent of Unity code, but there was also a Subscriber component that could be attached to any game object. The component would handle de-registration when it was destroyed. It also could be configured to process the internal message immediately when received or to queue it for the next game loop. You could configure it to have "X" number of messages on the queue, so if it received that particular message 5 times and you had it set to 4, it would purge the first message and add the new one, etc, to avoid overloading the queue. And you could set it to process the messages in either Update or FixedUpdate. The message hub itself could also be used from any regular .NET code.

    http://parentelement.com/documentation/messaging
     
    frosted likes this.
  11. Dustin-Horne

    Dustin-Horne

    Joined:
    Apr 4, 2013
    Posts:
    4,568
    It also used the WeakReference class to reference subscribers, so it didn't leak references.
     
  12. GarBenjamin

    GarBenjamin

    Joined:
    Dec 26, 2013
    Posts:
    7,441
    Ah I just did a quick check and I see... I think... using Serializable attribute in Unity does have an added automagical result of allowing data in classes to show up inside the Inspector. I guess this would be like the case where a monobehaviour class may have a field that is an instance another class and using Serializable would allow this object to actually show up expanded (i.e. list its public fields / public properties) in the Inspector.

    Is that the idea?

    LOL! I really should take the time to go through the Learn tutorials. Probably a lot of these things are covered there. All I used previously for my Unity game projects were Intellisense and the Unity API documentation looking up specific methods. Well I did see tidbits of tutorials on YouTube with people always dragging references to script fields in the Editor.
     
    Last edited: Mar 24, 2017
    frosted likes this.
  13. frosted

    frosted

    Joined:
    Jan 17, 2014
    Posts:
    4,044
    yeah - thats how you can get little foldouts in the vanilla inspector w/o having to write a custom editor.

    other advantage in storing values in a vanilla object like that is you can pass settings around to other stuff or copy it or whatever. But monobehaviours would work too, it doesn't really matter I think.

    The important thing is that the component acts as a repo for the code to find stuff at the same time it helps the user figure out whats in the scene. You can use more abstraction or less. At the end of the day, you want to have a nice clean place that does as much of the configuration as simply as possible.

    The real value add in @Dustin-Horne's service manager isn't the implementation of the host, it's this kind of code you write somewhere else:
    Code (csharp):
    1.  
    2. public void SetupScene(){
    3.   Services.Register<IScoreManager>( x );
    4.   Services.Register<IBadguyManager>( y );
    5.   Serivces.Register<IWhatever>( z );
    6. }
    7.  
    ^^ Having a block like this is the real payoff since it let you see everything in one clear little package. This acts as a guide to the dependencies and makes it much easier to find stuff. It gives info to other code and to the programmer at the same time.
     
    Last edited: Mar 24, 2017
    Dustin-Horne and GarBenjamin like this.
  14. GarBenjamin

    GarBenjamin

    Joined:
    Dec 26, 2013
    Posts:
    7,441
    Yeah absolutely the store is a good practice. That I completely get and and same for serializable in the conventional C# sense for saving objects to a file, generating XML, sending via TCP/IP, etc that I have done many times for work.

    The Unity proprietary stuff I am not that knowledgeable on. Well some of it anyway. I never cared to get into the Editor more than was absolutely needed. Since Unity seems so focused around the Editor this is why I am now focusing on a sort of fresh start to learn more of this proprietary Unity stuff and fill in the holes.

    Previously I started out to do that to a degree than opted to just use it as a means to load images and sounds then render and play them. Just using it like any other game oriented api.
     
  15. GarBenjamin

    GarBenjamin

    Joined:
    Dec 26, 2013
    Posts:
    7,441
    Finally the workweek is done. Gotta run for a haircut and when I get back I'll upgrade Unity and take a proper look at your project @frosted
     
  16. GarBenjamin

    GarBenjamin

    Joined:
    Dec 26, 2013
    Posts:
    7,441
    Geesh... finally got back and am done with busy work for the night... I think.

    I just checked out the project again @frosted. Actually the message was saying your project was made in an older version of Unity not a newer. And it ran fine. I think when I checked at the very end of my lunch break I was just so pressed for time haste makes waste kind of thing and maybe I accidentally messed something up click and drag or something.

    Anyway, first I checked out the message system. I see what you're doing. Just running through all of the components in the scene and registering them (if they implement interface AutoSub) and the ManualSub GO just registers itself during the startup.

    I also took a look at the Quick method and it is good. I used the scene structure (not the Inspector like you're doing here but I do like the idea of having a ReferenceManager for a single connection point) to do this kind of thing in my Space Invaders game last year and the other two (Atlantic Crisis and Treasures of Ali-Gar).

    Except I also used Find to find various parent GOs and then looped through the GOs listed beneath them and added them to collections. Anyway, my point of this is yes using the scene itself to organize the data in a way that makes it easier to work with in code finding references makes a lot of sense. I finally came to that conclusion just last year. lol

    And with that in mind another way to handle the simple Player example in the OP would be to have an empty GO folder even for single items such as the Player GameObject and name that parent "Player". Then if the child (real) Player GO had its name changed it wouldn't matter. The parent could even be named PlayerDoNotRenameThis. lol

    While both of your examples would work fine my thinking is more along the lines of the Quick method for the bulk of things anyway.
     
    Last edited: Mar 25, 2017