Search Unity

Question about class responsibility and loose coupling

Discussion in 'Scripting' started by winterfive, Aug 30, 2018.

  1. winterfive


    May 21, 2018

    I am creating a game for mobile VR (1st person). It's pretty far along and I've tried to keep it loosely coupled but now I have an issue that I'm going around in circles with.

    I have a RaycastManager class that sends out a ray (from the player POV) and finds objects in the scene. It lets other classes know that it has found a new object (and a new normal) via C# events.

    The ReticleManager class listens to the RaycastManager. It will check the object found to see if it's an enemy (checks for three different tags as the enemy has moving parts with different tags). If so, it recolors and moves the reticle accordingly.

    In my GameManager class, I'm adding code to destroy the enemy when it's shot by the player. I need to check the object that has been shot. It needs the exact same lines of code that I used in the ReticleManager class.

    The conundrum: I do not like repeating code, ever. But... the RaycastManager's scope of responsibility shouldn't include checking the object found, only finding objects and letting others know. The ReticleManager certainly shouldn't be checking the object for the other classes. It's responsibility lies only with the reticle. Having the GameManager listen to the ReticleManager seems wrong.

    Where should I do my check for enemy? Should it be in it's own tiny little class? I'm trying to avoid creating tiny classes as they tend to multiply. Thoughts? Thanks!
  2. kru


    Jan 19, 2013
    Determining whether the object is an enemy could be a utility method in a class that is shared by your reticle and game classes.

    Another option is to cascade your event, giving more context with subsequent events. In this case, you'd have an enemy detector which listens to the raycast event, then fires its own event when the raycaster's event is hovering over an enemy. Now your reticle and game can listen to the enemy detector's event and make better decisions.
    winterfive likes this.
  3. Fido789


    Feb 26, 2013
    You could make an extension method IsEnemy(), something like:
    Code (CSharp):
    1. public static class GameObjectExtensions
    2. {
    3.     public static bool IsEnemy(this GameObject go)
    4.     {
    5.         bool isEnemy;
    6.         isEnemy = // your code here...
    7.         return isEnemy;
    8.     }
    9. }
    Code (CSharp):
    1. if (go.IsEnemy())
    2. {
    3.     KillTheBastard(go);
    4. }
    Last edited: Aug 30, 2018
    winterfive likes this.
  4. Owen-Reynolds


    Feb 15, 2012
    It seems as if the object being looked at is a primary part of the game. Why not have the raycastManager record all interesting stats about the hit object? It you have a generic raycast manager, maybe keep that as-is but have another masterRaycastManager, which runs the plain one and extracts stats. Or, if you have other ways of targeting, have a currentSelectedObject class which runs the ray caster (which knows whether to target using a player-cast, or tap, or TAB keys ... .)

    Then you could think of the reticle-drawer as a V in MVC, or an Observer. It's job is too look at the pre-computed stats and draw the appropriate thing.

    As far as clean code, it's about the same as cleanliness in general. Don't be a slob, but don't be a germaphobe.
    winterfive likes this.