0
\$\begingroup\$

Let's say I have a script that calls a method frequently, and when doing so wants to allow other objects to know about it and potentially alter what happens in that method. This is fairly simple to do using C# events:

public class PassByReference 
{
    public bool foo;

    public PassByReference(bool foo)
    {
        this.foo = foo;
    }
}

public class Example : MonoBehaviour
{
    public event Action<PassByReference> OnSomethingHappened;

    private void DoSomething()
    {
        var returnOut = false;

        // Do some stuff

        var passByReference = new PassByReference(returnOut);
        OnSomethingHappened?.Invoke(passByReference); // Some other object can read and alter this

        if (passByReference.foo)
            return;

       // Do some more stuff
    }
}

Is this a good way to do things if one anticipates that, based on a combination of the information given by the event, and their own properties, other objects may want to stop the second half of the Example method from running (or maybe alter what happens)? I guess what I'm trying to do is prevent the Example class from having to store references to many other classes that will interact with it only occasionally, and clog up the method with many if statements. My main concern, though, is that frequently creating new classes to pass data via reference in the event(s) can lead to performance issues, especially if many objects in my game are using the Example script.

\$\endgroup\$
6
  • \$\begingroup\$ I generally advise against questions that ask "is this solution good?". The only judge of that who matters is you. Does this solution let you accomplish what you want to achieve in your game? Is the performance adequate for your goals when you profile it? Do you find it intuitive and maintainable to work with? If the answer to all of those is yes, then it doesn't matter if Internet randos like us think it's terrible - if it works for you, it works for you. If you find GC pressure is your limiting factor, there are solutions to that problem \$\endgroup\$
    – DMGregory
    Oct 21 '20 at 3:26
  • 1
    \$\begingroup\$ Then I'd recommend asking that concretely: "What is a less hacky way to achieve X" or maybe more specifically "What is a way to achieve X without allocating a temporary object" to define what feels "hacky" about it, instead of "Is this good?" which seems to ask for opinion. \$\endgroup\$
    – DMGregory
    Oct 21 '20 at 4:20
  • 1
    \$\begingroup\$ To be honest your solution is more or less okay. Unity Actions cannot return a value by default although standard C# events can (only the last event handlers return value is returned after invoke is called) so you can use those cautiously if needed. Usually what people do is pass a refence to a class into the function like you have done (C# often does this with EventArgs). If you are worried about too many intermediate classes you can probably just use generics to have make PassByReference<T>, PassByReference<T, U>, and PassByReference<T, U, V> classes. \$\endgroup\$ Oct 21 '20 at 9:11
  • 1
    \$\begingroup\$ of course you should not instantiate a new class in a loop, no matter how light it is; create an Event class once and update it with new properties as needed \$\endgroup\$
    – ivan866
    Oct 21 '20 at 15:44
  • 1
    \$\begingroup\$ @BenjaminDangerJohnson or you could invoke each one individually with GetInvocationList and check the return value of each \$\endgroup\$
    – Ed Marty
    Oct 24 '20 at 9:40
1
\$\begingroup\$

If I interpret this example correctly, then you have a situation where you have a game object which is supposed to do "something" unless there is a reason why it can not do "something" right now, and there is a large number of potential reasons for that with different grades of obscurity which depend on various other gameObjects.

In this case, it might be an option to invert the responsibility. Instead of having the object check all possible reasons why it might be unable to do something, have the other objects tell it that it can not do something for some reason.

public class Shooting : MonoBehaviour
{

    private bool canShootThisUpdate;
    private bool canShootThisFixedUpdate;

    public void SuppressShooting() {
       canShootThisUpdate = false;
       canShootThisFixedUpdate = false;
    }

    private void Update()
    {
         if(Input.GetButton("Fire") && 
            canShootThisUpdate && 
            canShootThisFixedUpdate) {
             Debug.Log("Pew! Pew! Pew!");
         }
         canShootThisUpdate = true;
    }

    private void FixedUpdate()
    {
         canShootThisFixedUpdate = true;
    }
}

This code will make the object shoot unless at least one other gameObject called the SuppressShooting() method during the last update / fixed update.

So any object which wants to stop this gameObject from shooting, just needs to do so. Like for example a "no shooting zone":

public class NoShootingZone : MonoBehaviour {

   void OnTriggerStay(Collider other) {
        Shooting shooting = other.GetComponent<Shooting>();
        if (shooting) {
             shooting.SuppressShooting();
        }
   }
}

or a "weapon cooldown" behavior which is actually on the same gameObject:

[RequireComponent(typeof(Shooting))]
public class WeaponCooldown : MonoBehaviour {

   public float cooldownTime;

   void Update() {
        cooldownTime -= Time.deltaTime;
        if (cooldownTime > 0) {
             GetComponent<Shooting>().SuppressShooting()
        }
   }
}

Note that for this to work reliable, the execution order of the Shooting script must be before any script which calls SuppressShooting from FixedUpdate, OnCollision* events or OnTrigger* events. Otherwise the suppression effect will fail when there are two Updates without a FixedUpdate between them. If you don't like scripts being reliant on execution order (which I can totally understand) you could also go for an alternative architecture where other objects suppress shooting not for a single update but for a specific amount of time. This might also be more performance-friendly if you have many such timed effects:

public class Shooting : MonoBehaviour
{

    private float suppressionTime;

    public void SuppressShooting(float time) {
       if (suppressionTime < time) suppressionTime = time;
    }

    private void Update()
    {
         if(Input.GetButton("Fire") && 
            suppressionTime <= 0f) {
             Debug.Log("Pew! Pew! Pew!");
         }
         suppressionTime -= Time.deltaTime;
    }
}

If you want to suppress shooting at least until the next Update, pass float.Epsilon. If you want to suppress shooting at least until the next FixedUpdate, pass Time.fixedDeltaTime.

\$\endgroup\$
0
\$\begingroup\$

Event-Driven Design (EED) is a good pattern widely used in the game industry, so yes, that is totally the good and standard approach. How to implement it is entirely your choice. You can do it using Unity Event System, you can do it in pure C# or a mix of the two.

Personally, I always mix EED with a Finite State Machine (FSM) approach to tackle the cases where incompatible events are triggered, but it's just my personal view about EED implementation: you can do it in a stateless way, but it will be more complex to test and fix unwanted behaviours.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.