Avoiding duplicate code between the player and npc classes deriving from Unity's MonoBehaviour

Question

These would be the base classes like Npc and NpcTask.

public abstract class NpcTask
{
    public Npc Npc { get; private set; }

    public NpcTask(Npc npc)
    {
        Npc = npc;
    }

    public abstract void Update();
}

public abstract class Npc : MonoBehaviour
{
    [HideInInspector]
    public NavMeshAgent NavMeshAgent;

    public NpcTask CurrentTask;

    private void InitializeComponents()
    {
        NavMeshAgent = GetComponent<NavMeshAgent>();
    }

    protected virtual void Initialize()
    {
        InitializeComponents();
    }

    protected abstract void SetDefaultCurrentTask();

    public void Log(object text)
    {
        Debug.Log("[" + name + "] " + text);
    }

    private void Start()
    {
        Initialize();
    }

    private void Update()
    {
        CurrentTask?.Update();
    }
}

The next classes would be concrete and abstract implementations of the above to provide more flexibility to Npc's classes.

public class NpcEnemy : Npc
{
    protected override void SetDefaultCurrentTask()
    {
        CurrentTask = new NpcEnemyTaskWander(this);
    }
}

public abstract class NpcEnemyTask : NpcTask
{
    public new NpcEnemy Npc { get { return base.Npc as NpcEnemy; } }

    public NpcEnemyTask(Npc npc) : base(npc)
    {
    }
}

public class NpcEnemyTaskWander : NpcEnemyTask
{
    public NpcEnemyTaskWander(Npc npc) : base(npc)
    {
    }

    public override void Update()
    {
        // Wandering logic.
        Debug.Log("NpcEnemyTaskWander.Update()");
    }
}

public class NpcAllie : Npc
{
    public List<NpcEnemy> Enemies;

    private void Awake()
    {
        Enemies = new List<NpcEnemy>();
    }

    protected override void SetDefaultCurrentTask()
    {
        CurrentTask = new NpcAllieTaskFightEnemies(this);
    }
}

public abstract class NpcAllieTask : NpcTask
{
    public new NpcAllie Npc { get { return base.Npc as NpcAllie; } }

    public NpcAllieTask(Npc npc) : base(npc)
    {
    }
}

public class NpcAllieTaskFightEnemies : NpcAllieTask
{
    public NpcAllieTaskFightEnemies(Npc npc) : base(npc)
    {
    }

    public override void Update()
    {
        // Fighting logic.
        Npc.NavMeshAgent.SetDestination(Npc.Enemies[0].transform.position);
    }
}

But then there would be the player which will not need pre-coded fighting logic as the fighting would be done by the person playing the game, but still would need a list of enemies to later know which enemies the player can attack or something like that.

public class Player : MonoBehaviour
{
    public List<NpcEnemy> Enemies;

    private void Awake()
    {
        Enemies = new List<NpcEnemy>();
    }

    private void Move()
    {
        // Whatever used to listen for input to move (just an example).
    }

    private void Attack()
    {
        // Whatever used to listen for input to attack (just an example).
    }

    private void Update()
    {
        Move();
        Attack();
    }
}

So as you can see both NpcAllies and the Player have list of NpcEnemies. I think the most feasible/simple solution would be to centralize the List in a singleton placed on the scene?

Lets say also each enemy could have a target which could be either the Player or a NpcAllie(i guess i would need to use GameObject class here to be more general?), so the Player and NpcAllie classes would need to know wheter they are a target or not to attack the right NpcEnemies.

public class NpcEnemy : Npc
{
    public GameObject Target;

    protected override void SetDefaultCurrentTask()
    {
        CurrentTask = new NpcEnemyTaskWander(this);
    }
}

Dirk Boer · Answer 1 · 2020-05-10T23:46:34.503

2

You would keep these two classes seperate. You should think about this like a “has behaviour” relationship instead of an “is” relationship.

Make a class Npc and a class FightingBehaviour. You add them both to Fighting Npcs. If you need access to methods in Npc from Fighter you’ll use GetComponent<Npc>()

FightingBehaviour doesn't even have to inherit from MonoBehaviour.

So you get something similar like this:

public interface IFighter
{
    GameObject gameObject;
    void Shoot();
}


public class PlayerFighter : MonoBehaviour, IFighter
{
    FightingBehaviour _FightingBehaviour;

    private void Awake()
    {
        _FightingBehaviour = new FighterBehaviour(this);
    }
}



public class FightingBehaviour
{
    IFighter _Fighter;

    public List<Enemy> Enemies { get; private set; }


    public FightingBehaviour(IFighter fighter)
    {
        _Fighter = fighter;
    }


    public Enemy GetClosestEnemy()
    {
        // implement using _Fighter
    }


    public Enemy GetEnemiesInView()
    {
        // implement using _Fighter
    }


    // etc


    public Enemy AttackClosestEnemy()
    {
        var enemy = GetClosestEnemy();

        var direction = _Fighter.gameObject.transform.position - enemy.gameObject.transform.position; // get direction to enemy

        _Fighter.RotateTo(direction);
        _Fighter.Shoot();
    }
}

edited May 10 '20 at 23:46

answered May 10 '20 at 21:18

Dirk Boer

381
2
9

So if i understand correctly in my NpcFighter gameobject i would have a Npc component and a Fighter component? What i had in mind was each Npc would be running a NpcTask class instance, not derived from monobehaviour, but would contain an Update method which the Npc class would use to call from its own Update since it derives from monobehaviour – user2640145 May 10 '20 at 21:28
would also work! The important thing is should think like a has-a relationship instead of a hierarchy. Lot of good articles about that. – Dirk Boer May 10 '20 at 21:34
Could you consider my edit? – user2640145 May 10 '20 at 21:39
What do you mean exactly? – Dirk Boer May 10 '20 at 23:05
I updated the original quesiton with more details, added the NpcTask that each Npc has access to and the NpcTask itself has access to the Npc for example when you need a task to wander around you could do Npc.NavMeshAgent.SetDestination(...) on the NpcTask,. I still find it kinda strange to have a monobehaviour only containing data like a List of enemies? – user2640145 May 10 '20 at 23:13
Hi @user2640145, you have to think not only about the list of enemies - but the behaviour around it. What part of NpcFighter will care about the list of enemies and why? You should try to see if you can encapsulate all that behaviour into the class. I'll try to add an example. – Dirk Boer May 10 '20 at 23:40
Hi @user2640145, can you mark this answer as correct if it helped you? Thanks! – Dirk Boer May 11 '20 at 12:11
Do you mind taking a look at a more precise example in my edit? – user2640145 May 12 '20 at 15:15
Hi @user2640145, I think you're question becomes a bit too broad. I would just look up articles of has-a and is-a relationship. – Dirk Boer May 12 '20 at 15:34

score 0 · Answer 2 · answered Jun 07 '21 at 01:37

0

Alternate approach:

public class Character : MonoBehavior
{
// put the common behavior here
}

public class NPC : Character
{
}

public class NPCEnemy : NPC
{
}

public class NPCAlly : NPC
{
}

public class Player : Character
{
}

answered Jun 07 '21 at 01:37

Loren Pechtel

3,371
24
19

It's not a bad approach but could do with some elaboration as opposed to only answering with a dry code snippet. – Flater Jun 07 '21 at 11:24

Avoiding duplicate code between the player and npc classes deriving from Unity's MonoBehaviour

2 Answers2