Which one is supposed to handle MissingReferenceException? The developer, or the engine?

Unity Engine
1

I got an error message when destroying a game object on a network game using UNET.

MissingReferenceException: The object of type 'NetworkIdentity' has been destroyed but you are still trying to access it.
Your script should either check if it is null or you should not destroy the object.
UnityEngine.Networking.NetworkServer.DestroyObject (UnityEngine.Networking.NetworkIdentity uv) (at C:/buildslave/unity/build/Extensions/Networking/Runtime/NetworkServer.cs:1388)
UnityEngine.Networking.NetworkServer.DestroyPlayersForConnection (UnityEngine.Networking.NetworkConnection conn) (at C:/buildslave/unity/build/Extensions/Networking/Runtime/NetworkServer.cs:1351)
UnityEngine.Networking.NetworkManager.OnServerDisconnect (UnityEngine.Networking.NetworkConnection conn) (at C:/buildslave/unity/build/Extensions/Networking/Runtime/NetworkManager.cs:693)
UnityEngine.Networking.NetworkManager.OnServerDisconnectInternal (UnityEngine.Networking.NetworkMessage netMsg) (at C:/buildslave/unity/build/Extensions/Networking/Runtime/NetworkManager.cs:587)
UnityEngine.Networking.NetworkMessageHandlers.InvokeHandler (Int16 msgType, UnityEngine.Networking.NetworkConnection conn, UnityEngine.Networking.NetworkReader reader, Int32 channelId) (at C:/buildslave/unity/build/Extensions/Networking/Runtime/NetworkMessageHandlers.cs:73)
UnityEngine.Networking.NetworkMessageHandlers.InvokeHandlerNoData (Int16 msgType, UnityEngine.Networking.NetworkConnection conn) (at C:/buildslave/unity/build/Extensions/Networking/Runtime/NetworkMessageHandlers.cs:60)
UnityEngine.Networking.NetworkServer.InternalUpdate () (at C:/buildslave/unity/build/Extensions/Networking/Runtime/NetworkServer.cs:681)
UnityEngine.Networking.NetworkServer.Update () (at C:/buildslave/unity/build/Extensions/Networking/Runtime/NetworkServer.cs:514)
UnityEngine.Networking.NetworkIdentity.UNetStaticUpdate () (at C:/buildslave/unity/build/Extensions/Networking/Runtime/NetworkIdentity.cs:724)

I am wondering who should handle this error, the developer or the engine?

If it’s supposed to be on the developer side, how should I design the code, so that I can prevent the error from happening?

This is the destroy game object code:

using UnityEngine;
using System.Collections.Generic;
using UnityEngine.Networking;

public class CubeUnit : NetworkBehaviour {
    public enum OrderStatus {
        ATTACK, MOVE, SPLIT, MERGE
    }

    public OrderStatus status;
    public CubeUnit targetUnit;
    public float attackRadius;
    public float attackCooldown;
    public int attackPower;
    [SyncVar] public int healthPoints;

    private float attackCooldownTimer;

    private void Start() {
        if (this.healthPoints <= 0) {
            this.healthPoints = 1;
        }
        if (this.attackCooldown <= 3f) {
            this.attackCooldown = 3f;
        }
        this.attackCooldownTimer = this.attackCooldown;
        this.attackPower = 1;
        this.targetUnit = null;
        this.status = OrderStatus.ATTACK;

        Renderer renderer = this.GetComponent<Renderer>();
        if (renderer != null) {
            Vector3 size = renderer.bounds.size;
            this.attackRadius = size.magnitude + 1f;
        }
        else {
            this.attackRadius = 3f;
        }
    }

    //When the unit state is set to attack another unit, it will then begin the counter that counts until the cooldown reaches zero, and then attack.
    private void Update() {
        if (this.attackCooldownTimer < 0f) {
            switch (this.status) {
                default:
                case OrderStatus.ATTACK:
                    if (this.targetUnit == null) {
                        CheckSurroundings();
                    }
                    AttackUnit(this.targetUnit);
                    break;
            }
            this.attackCooldownTimer = this.attackCooldown;
        }
        else {
            this.attackCooldownTimer -= Time.deltaTime;
        }
    }

    // -----------------------------------------------------------------------------

    public void SetAttackTarget(Vector3 target) {
        this.status = OrderStatus.ATTACK;
        this.SetAgentDestination(target);
    }

    public void SetAgentDestination(Vector3 target) {
        NavMeshAgent agent = this.GetComponent<NavMeshAgent>();
        if (agent != null) {
            agent.SetDestination(target);
        }
    }

    public void SetAgentsDestinationFormation(Vector3 target) {
        //Set a new formation pattern for this for more than 2 units.
    }

    public void TakeDamage(int value) {
        if (this.healthPoints <= 0) {
            CmdKillMe(this.gameObject);   //  <----------------------------   THIS KILLS THE GAME OBJECT.
            this.targetUnit = null;
        }
        else {
            this.healthPoints -= value;
        }
    }

    // -----------------------------------------------------------------------------

    private void CheckSurroundings() {
        if (this.targetUnit == null) {
            Transform origin = this.GetComponent<Transform>();
            Collider[] colliders = Physics.OverlapSphere(origin.position, 5.0f);
            foreach (Collider col in colliders) {
                CubeUnit unit = col.GetComponent<CubeUnit>();
                if (unit != null) {
                    this.targetUnit = unit;
                    return;
                }
            }
        }
    }

    private void AttackUnit(CubeUnit other) {
        if (other == null) {
            return;
        }
        Transform transform = this.GetComponent<Transform>();
        Transform otherTransform = other.GetComponent<Transform>();
        if (Vector3.Distance(transform.position, otherTransform.position) < this.attackRadius) {
            other.TakeDamage(this.attackPower);   //<-------------------   CALLS FOR THE ENEMY TO TAKE DAMANGE EVERY ATTACK TICK (3 seconds)
        }
    }

    // -----------------------------------------------------------------------------

    [Command]
    public void CmdKillMe(GameObject obj) {
        NetworkServer.Destroy(obj);
    }
}
2

FYI, this code is insecure. It allows a malicious client to kill another player by passing their game object. Commands are explicitly called on the player that issued them, so adding a “self” to a command is redundant and breaks the security model.

 [Command]
   public void CmdKillMe(GameObject obj) {
        NetworkServer.Destroy(obj);
   }

But anyway, the engine should handle that exception. I will file a bug on this.

3

Which code is insecure? My “destroy game object code”, or the engine code? If it’s me, I never knew I wrote an insecure code. :smile:

Thank you for clarifying.

4

He means you should do this instead

public void CmdKillMe() {
        NetworkServer.Destroy(this.gameObject);
   }

or something along those lines so that a player can’t tell the server to kill someone else instead of “me”

5

Yeah, that’s more reasonable.