Jump to content
Unity Insider Forum
Sign in to follow this  
chrische5

Managerklassen

Recommended Posts

Hallo

 

In einem anderen Thread entstand eine Diskussion, ob Managerklassen grundsätzlich abzulehnen seien, weil der Name unaussagekräftig sei. Ich will das nicht so richtig einsehen, aber da Sascha der Diskussionteilnehmer mit der anderen Meinung war, bin ich natürlich eher zurückhaltend.

Ich wollte also mal eine meiner Managerklassen als Beispiel hier posten und fragen, warum man das nicht so machen sollte. 

public class TentManager : MonoBehaviour
    {
        [ShowInInspector] [ReadOnly] private bool _isPlayerInRange = false;

        private IRangeResponse[] _responses = default;
        void Start()
        {
            _responses = GetComponents<IRangeResponse>();
        }
    
        // Update is called once per frame
        void Update()
        {
            if (_isPlayerInRange)
            {
                foreach (var response in _responses)
                {
                    response.InRange();
                }
            }
            else
            {
                foreach (var response in _responses)
                {
                    response.OutRange();
                }
            }
        }

        private void OnTriggerEnter2D(Collider2D other)
        {
            if (other.CompareTag("Player"))
            {
                _isPlayerInRange = true;
            }
        }

        private void OnTriggerExit2D(Collider2D other)
        {
            if (other.CompareTag("Player"))
            {
                _isPlayerInRange = false;
            }
        }
    }

Ich selbst bin mit dem CompareTag noch unglücklich, aber die Erweiterbarkeit der Klasse scheint mir toll zu sein. Vor allem muss ich dabei den Manager gar nicht mehr anfassen.

 

Danke

Christoph

Share this post


Link to post
Share on other sites

Ich sehe nicht so ganz, was daran eine Managerklasse sein soll. Ein ABCManager ist eine Klasse, die ABCs managed. Du hast also zum Beispiel eine Enemy-Komponente und dann hast du noch einen EnemyManager, der denen sagt, wie sie zu funktionieren haben. Und ohne diesen Manager funktioniert die Enemy-Klasse dann nicht mehr. Warum heißt deine Klasse "TentManager" und nicht einfach "Tent"? Meine Kritik ist zum Einen an der Benennung, weil das, was ein Manager tut komplett obskur ist, und zum Anderen an der Architektur dahinter (siehe EnemyManager). Wenn du aber einfach nur eine ganz normale Komponente hast und da "Manager" hinter den Namen schreibst, hat das mit meiner Kritik nichts zu tun. Wenn Unity morgen die "Light"-Komponente in "LightManager" umbenennt, dann wäre das zwar irgendwie bescheuert, aber das macht die Klasse an sich ja nicht schlechter. Nur der Suffix, der ergibt irgendwie keinen Sinn :)

Share this post


Link to post
Share on other sites

Hallo

okay. stimmt natürlich ich könnte das ganze einfach "tent" nennen, wobei ich irgendwie so denke, dass "tent" das komplette gameobject ist und eben nicht nur das script. das ist aber vielleicht nur in meinem kopf. ich verstehe deine kritik an der architektur nicht. die klasse sah vorher so aus, dass ich jedes verhalten direkt in ihr hatte. dann sah ich ein solid video und dort wurde diese lösung entwickelt. ich finde die eigentlich total schmuck und macht es einfach verhaltensweise hinzuzufügen und oder zu löschen.

danke

christoph 

Share this post


Link to post
Share on other sites
vor 10 Stunden schrieb chrische5:

ich verstehe deine kritik an der architektur nicht.

Ich hab ja auch keine geäußert? Also... ich finde Interfaces auf UnityEngine.Objects ungut (Grund hier) und finde Actions (bzw. Delegate) besser für Reaktionen. Aber die grundsätzliche Idee nennt sich Observer Pattern (oder "Beobachtermuster") und ist völlig gut und üblich, und dagegen hab ich nichts gesagt. Meine ursprüngliche Aussage war "wenn deine Klasse das Wort 'Manager' im Namen hat, dann machst du etwas falsch". Und manchmal ist das Falsche einfach nur die Benennung :)

Damit du ein Bild davon hast, wie das mit Actions aussehen kann:

public class TentRangeActions : MonoBehaviour
{
  public event Action onPlayerInRange;
  public event Action onPlayerOutOfRange;

  void Update()
  {
    if (_isPlayerInRange)
    {
      onPlayerInRange?.Invoke();
    }
    else
    {
      onPlayerOutOfRange?.Invoke();
    }
  }
  
  // collision events here
}

Und die Komponenten, die vorher das Interface hatten, machen dann so etwas:

[RequireComponent(typeof(TentRangeActions))]
public class Foo : MonoBehaviour
{
  private TentRangeActions tentRangeActions;
  
  private void Awake()
  {
    tentRangeActions = GetComponent<TentRangeActions>();
  }
  
  private void OnEnable()
  {
    tentRangeActions.onPlayerInRange += DoStuff;
  }
  
  private void OnDisable()
  {
    tentRangeActions.onPlayerInRange -= DoStuff;
  }
  
  private void DoStuff()
  {
    // Actual stuff as long as the player is in range
  }
}

Jetzt sucht "TendRangeActions", oder wie auch immer man das nennen will, nicht mehr einmalig nach Komponenten, sondern Komponenten melden sich bei dieser Komponente an. Dadurch kannst du zur Laufzeit Komponenten hinzufügen, wegnehmen, deaktivieren oder wieder aktivieren, und es funktioniert alles wie man es erwartet.

Die reagierenden Komponenten sind mit [RequireComponent] markiert, sodass klar ist, dass sie ohne die TentRangeActions-Komponente nicht funktionieren. Foo kann sich bei onPlayerInRange anmelden, aber auf onPlayerOutOfRange verzichten. Und am wichtigsten: Kein Interface mehr :)

Was CompareTag angeht: Strings sind doof. Wie im Link erklärt, würde ich immer Komponenten stattdessen empfehlen. Statt

if (other.CompareTag("Player"))

hast du dann

if (other.GetComponent<Player>())

Aber das alles sind nur kleinere Änderungen, die grundlegende Idee ist dieselbe, die du schon hast. Und diese Idee ist absolut gut und richtig. Observer Pattern halt :)

Share this post


Link to post
Share on other sites

Hallo

 

Okay, das sieht sehr spannend aus. Ich werde das gleich mal probieren. Was mich allerdings grundsätzlich stört ist, dass es Aktionen in TentRange gibt, die Variablen des MainCharacters verändern. Nun muss entweder das Zelt den Character kennen oder noch schlimmer der Character alle Zelte. Das ist hässlich und lieber wäre es mir, wenn ein "globales" Event ausgelöst werden würde, auf das der Character dann lauscht. Ist das möglich? SODA sah etwas so aus, aber dein Healthbar-Beispiel war eher andersherum. Oder habe ich das falsch interpretiert?

 

Christoph

Share this post


Link to post
Share on other sites
vor 5 Stunden schrieb chrische5:

Ist das möglich? SODA sah etwas so aus,

Jau, du kannst damit globale Events (in schick!) machen.

vor 5 Stunden schrieb chrische5:

Was mich allerdings grundsätzlich stört ist, dass es Aktionen in TentRange gibt, die Variablen des MainCharacters verändern. Nun muss entweder das Zelt den Character kennen oder noch schlimmer der Character alle Zelte.

Actions sind quasi Methoden, die können Parameter haben. Du kannst also das hier machen:

public event Action<Player> onPlayerInRange;

und dann muss die registrierende Klasse auch eine Methode übergeben, die einen Player-Parameter nimmt:

private void DoStuff(Player player)

Das Event muss dann auch mit Parameter aufgerufen werden:

onPlayerInRange?.Invoke(playerInArea);

Was sich super mit der taglosen Variante zur Identifikation kombinieren lässt:

private Player playerInArea;
private bool playerIsInside => playerInArea != null;

 private void OnTriggerEnter2D(Collider2D other)
 {
   var player = other.GetComponent<Player>();
   if (player)
   {
     playerInArea = player;
   }
 }

Du kannst natürlich auch ein Player-Pseudo-Singleton bauen und den Spieler damit global bekanntmachen. Das geht mit Soda auch, in eleganter :P

Share this post


Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Loading...
Sign in to follow this  

×
×
  • Create New...