I need some critique and advice about inheritance and my system

Unity Engine
1

I’m afraid I’m getting myself into a real ugly mess…

I’m a hobby coder who’s still learning. The main issue is that I’ve known about features like abstract, inheritance, static and such but I’ve never really had a use for it until now. Which in turn makes it difficult to plan ahead when making a more complicated system. Personally I do prefer easy to manage code even tho it means I have to do a bit more typing. I got this great idea of a system I’d create and it’s been working for a while…

My weapon swap logic:
I’ve created a system for a player to swap and shoot different types of guns in a 2D game. The structure is as follows:

// Main class handling shooting related input, swaps guns (sprites, prefabs, etc. It also contains separate Update() functions for each weapon to more easily separate their unique functions)
PlayerShoot.cs

// Is an abstract class containing functions and variables shared among all weapons
WeaponCollection.cs

// All Weapon<Gun> classes contains mostly the same things.
// A reference to their unique bullet prefab for instance.
// The shotgun has it's own function for shooting several bullets at the same time, etc.
WeaponPistol : WeaponCollection
WeaponRifle : WeaponCollection
WeaponShotgun: WeaponCollection

// This is as standard class acting as a parent for all the unique bullets. It does all the necessary calculations, checks for collisions, etc.
BulletCollection.cs

// These classes are even more identical than the Weapon<Gun> classes, where BulletShotgun.cs is the exception.
// Every weapon has a reference to their own bullet class, This works fine.
BulletPistol : BulletCollection
BulletRifle : BulletCollection
BulletShotgun : BulletCollection

I do have a lot more weapons following the same structure.

Reasoning:

  • To easily keep track of all these weapon specific objects I created a tiny Wpn struct in PlayerShoot.cs that contains information about the current weapon in use. In this struct I cache components of the weapon currently in use to easily get the correct components for the current weapon. Since every weapon has it’s own script I made a parent class (WeaponCollection), this way I could cache all of the weapon scripts in the same variable.

  • Since the WeaponCollection.cs contains almost all of the variables related to using these weapons I can collect and find every variable under each weapon in the inspector. As an exception, since PlayerShoot.cs controls when I’m able to shoot (fire rate) I made a simple getter function in all the weapon scripts which I fetch at the beginning of a weapon swap. (I know, why not move the getter function to the parent class WeaponCollection.cs. I do adjustments to my system when I see it can be improved, I’ve just not changed this yet to avoid unnecessary confusion from constantly moving things around)

  • Why having a separate Bullet.cs class for every bullet even tho the majority if them are identical? I don’t quite know what kind of changes I might do in the future and already having an individual class makes it easy to hop in and make some changes. Yes I will eventually make a BulletCommon.cs class which can be shared amongst the majority of the weapons.

The problem:
In my PlayerShoot.cs I had temporarily made some variables for the Pistol only to get in some unique logic for that gun. When everything was working I wanted to move these to the WeaponPistol.cs class and store them there to follow the same structure as the other guns and keep things consistent. I’m also able to have all Pistol values visible in the inspector, even the new ones.
I made some public getter functions for my PlayerShoot.cs to fetch when needed.

//WeaponPistol.cs
public float GetValueA() { return valueA; }
public float GetValueB() { return valueB; }

Some of you might already see the problem. Since my cached script is of type WeaponCollection (the parent of all weapons) it doesn’t know about the WeaponPistol.cs specific functions.

Afterthought:
I see several solutions to this “simple” problem, yet all of them leads me to worry about it just getting worse in the future.
Yes, I can…

  • move the Pistol specific values back into PlayerShoot.cs. What if I get 30 other values like this. It will get more confusing and my system will eventually be pointless.
  • add the getter functions to my abstract WeaponCollection and force all my other weapons to have the exact same functions. This is even more pointless as I’d have to return a random float. My PlayerShoot.cs logic should prevent me from ever calling these functions from any other Weapon classes.

Hopefully you managed to understand my current system and its logic even tho this was a highly abstract explanation. It would be too much code to share and frankly the code is not the problem, my current system is. I got the feeling the best solution here would be to just rework the entire system (which I will do if necessary), in that case, would someone explain how I would do things differently?
Thanks for reading.

Tags: organization, inheritance, polymorphism, structure.

2

Hi,

Inheritance is handy, certainly for adding future weapons etc.

You have to build your general public interface into the baseclass only.

As a hack around (for funcs specific to derived classes) you may try

DerivedClass myDerivedClass = myBaseClass.gameObject.GetComponent<DerivedClass>();
if(myDerivedClass != null)
{
   myDerivedClass.foo();
}

But to save you from future spaghetti: I recommend designing your baseclass public interface to be complete for all derived weapons.

Your derived classes may override baseclass functions in unique ways.

Your derived classes don’t have to override all baseclass functions, by not overriding it: when the baseclass is called it will run it’s own virtual func which may return null or false etc. to inform caller that this weapon does not do this thing.

All state specific to a weapon type should be black boxed into the derived class. If you’re reaching into weapons to read stuff unique to one weapon then you may need to rework your code if you want to leverage full benefits of OOP.

3

Also OOP is not always the best choice, also look into composition over inheritance

4

Just to clarify possible mixed messages.

Composition is part of OOP. You may have meant to say “inheritance is not always the best choice”.

5

You solve it on two different ways, in OOP by uing subclasses that adds features, in composition you add behaviour by adding new components instead. Those components can use but not required to use inheritance.

6

As for OP.

Sounds to me like PlayerShoot is doing way more than it aught to be doing.

Like this here:

Why does PlayerShoot control the fire rate? Guns control that. A shotgun can only be fired at a specific rate versus pistols.

Sure you might have a rate modifier on ‘PlayerShoot’ and that’s the player’s proficiency. If you know, you wanted to have a levelable proficiency.

This is actually why you should move them to the Pistol class. Because all of these values pertain ONLY to the pistol.

Furthermore… why do you have special classes for each kind of gun? Most guns behave primarily the same way, only with stats being the variance of that way. Rate of fire, damage, what graphics are displayed, sfx, etc. Even the use of ‘bullet’ is specific to the gun.

This is actually where @AndersMalmgren suggestion of ‘composition’ comes in. You composite these properties into the Gun, and the ‘PlayerShoot’ script deals with it.

I’ll give you an example… I’ll show you my gun script. Though it’s a little long, and I’m in the middle of eating right now. So I’ll be back with that.

7

Yes. I agree composition is often way better than inheritance.

But composition is still a part of OOP.

OOP consists of:
Abstraction
Encapsulation
Polymorphism
Inheritance - which has a point of contention in that now a days it’s often referred to as “inheritance and/or composition”, or “composition over inheritance”.

Hence more modern OOP langauges like golang who implement their inheritance via a purely composition system.

Composition historically began as a OOP design pattern, and slowly grew to become a replacement for inheritance in a lot of situations.

Hence:
https://en.wikipedia.org/wiki/Composition_over_inheritance

8

Thanks for these replies, I’ll look into it tomorrow!
I’ll show you in a little bit more detail how things are working.

// Class for keeping weapon stuff collected
class Wpn {
    public WeaponCollection script;
    public GameObject prefab;
    public Animator animator;

    // Constructor
    public Wpn(GameObject a_prefab, WeaponCollection a_script, Animator a_animator) {
        prefab = a_prefab;
        script = a_script;
        animator = a_animator;
    }
}


void Start() {
    // Create a dictionary of Wpn classes
    weaponDict = new Dictionary<WeaponList, Wpn> {
        { WeaponList.Pistol, new Wpn(prefabPistol, prefabPistol.GetComponent<WeaponPistol>(), prefabPistol.GetComponent<Animator>()) },
        { WeaponList.Rifle, new Wpn(prefabRifle, prefabRifle.GetComponent<WeaponRifle>(), prefabRifle.GetComponent<Animator>()) },
        // List keeps going
    };
}

This way I can do the following

// A function has been called to swap weapons
// Go through a switch to check which weapon we're changing to.
// Swap the weapon
void SwapWeapon(WeaponList changeTo) {
    // Disable previous weapon
    weaponDict[currentWeapon].prefab.SetActive(false);

    // Enable next weapon
    weaponDict[changeTo].prefab.SetActive(true);
    // Store script
    currentWeaponScript = weaponDict[changeTo].script;
    // Store animator
    currentWeaponAnimator = weaponDict[changeTo].animator;
    // Fetch shooting cooldown
    shootWaitTime = weaponDict[changeTo].script.GetShootWaitTime();
    nextShootTime = Time.time + shootWaitTime;
    // Current weapon
    currentWeapon = changeTo;
}

Not knowhing how things will be, this at least keeps me from calling GetComponent and searching through a Dictionary every loop.

My “hack” in the end was to make a static class containing all the separate scripts.

// Static class containing all weapon scripts
static class WpnScripts {
    public static WeaponPistol scriptPistol;
    public static WeaponRifle scriptRifle;
    // List keeps going
}

var value = WpnScripts.scriptPistol.GetSomeValue();
9

Yeah, but classic OOP do favor inheritance but you are right composition is OOP

10

Yes.

I’m commenting on the fact you said:

You didn’t say classic, you didn’t say any of that.

I was just correcting the error in your statement. So as to avoid mixed messages about OOP in general. As you have it worded it implies to avoid OOP outright. Which is counter to the use of composition which is part of OOP.

11

My game is a VR game so it has alot more meat to its weapons than desktop games. But we have a basic Firearm component, it can’t do almost anything on its own. It needs atleast a Action component. The action component is what makes the actual operation. When you press the trigger and there is a bullet in the chamber the firearm component tells the action component it has been fired. If it’s a bolt action nothing happens at this point. If it’s a automatic action it now starts to index a new bullet if one is present in the mag.

You can also add a fire selector to the firearm if you want to be able to change firemode. But it’s the action component that will determine rate of fire.

This might be overkill for a desktop game that does not let you operate the different components by interacting with them in a 3D space like you do in VR for examle by operating the bolt action to index a new bullet

12

I’m not used to managing these bigger challenges where it’s important to get a good foundation. I also have a tendency to focus on the wrong aspects as times. So every input from you guys helps, thanks.

@lordofduct Good questions. I’ve actually asked myself these things on the way as well, not taking it too seriously as everything seemed fine.

As of now I’m using MonsterLove.StateMachine to keep track of which guns are in use. This is happening in PlayerShoot.cs. Right now PlayerShoot.cs has evolved far beyond the shooting aspect and should be renamed.
By the power of StateMachine I have separate Update() functions for each weapon. This is one of the reasons I’ve kept everything in one place.

Whenever a weapon swap occurs it calls a GetShootWaitTime() from the relevant weapon script. This is used to keep a steady fire rate even tho the fire button is held down. When the wait time is over and a bullet is actually fired, it calls a Shoot() from the relevant weapon script which takes care of initializing a bullet prefab with positions, directions, speed, an offset angle (in terms of spray) etc. I guess my reason for doing it this way was becaus I didn’t see any point having an Update() just to constantly relay it to the current weapon.

void Update()
{
currentWeapon.Shoot()
}

class CurrentWeapon
{
    public void Shoot()
    {
        //Can we shoot, then do stuff
    }
}

tl;dr
With my inexperience of bigger and more complex managing, letting PlayerShoot.cs take care of the heavy lifting seemed like a good idea at the time. I now see this definitely has to change.

Also, if interested, I could upload all of these files to a repo where it can be criticized in more detail.

13

All right, so I’m done eating. Here is what I said I would show. Note that this is very specific to our game, so a lot of the particulars pertain directly to that.

So in our system we can have multiple weapons. Melee, Ranged, and some others (like a mine that can be placed on the ground). As a result the way we ‘swap’ weapons is abstracted into its own class.

You can see this here:

using UnityEngine;
using System.Collections.Generic;

using com.spacepuppy;
using com.spacepuppy.Anim;
using com.spacepuppy.SPInput;
using com.spacepuppy.Utils;

using com.mansion.Entities.Inventory;
using com.mansion.Entities.Weapons;
using com.mansion.UserInput;

namespace com.mansion.Entities.Actors.Player
{

    public class PlayerSwapWeaponActionStyle : SPComponent, IPlayerActionStyle, PlayerWeaponPouch.ICurrentWeaponChangedHandler
    {
      
        #region Fields

        [Header("References")]
        [SerializeField]
        [DefaultFromSelf(Relativity = EntityRelativity.Entity)]
        private PlayerWeaponPouch _weaponPouch;
        [SerializeField]
        [DefaultFromSelf(Relativity = EntityRelativity.Entity)]
        private AmmoPouch _ammoPouch;
        [SerializeField]
        [DefaultFromSelf]
        private PlayerMeleeWeaponStyle _melee;
        [SerializeField]
        [DefaultFromSelf]
        private PlayerFreeFocusMeleeWeaponStyle _freeFocusMelee;
        [SerializeField]
        [DefaultFromSelf]
        private PlayerRangedWeaponStyle _ranged;
        [SerializeField]
        [DefaultFromSelf]
        private PlayerLockOn _lockon;
      


        [System.NonSerialized()]
        private PlayerEntity _entity;
      
        [System.NonSerialized]
        private IPlayerWeaponStyle _currentWeaponStyle;

        [System.NonSerialized]
        private RadicalCoroutine _swapRoutine;

        #endregion

        #region CONSTRUCTOR

        protected override void Awake()
        {
            base.Awake();

            _entity = SPEntity.Pool.GetFromSource<PlayerEntity>(this);
            if (_melee != null) _melee.IsActiveWeapon = false;
            if (_freeFocusMelee != null) _freeFocusMelee.IsActiveWeapon = false;
            if (_ranged != null) _ranged.IsActiveWeapon = false;
            _currentWeaponStyle = null;
        }

        protected override void OnEnable()
        {
            base.OnEnable();

            if (_currentWeaponStyle == null && !this.IsSwapping && _weaponPouch.CurrentWeapon != null)
            {
                _swapRoutine = this.StartRadicalCoroutine(this.SetWeapon(_weaponPouch.CurrentWeapon));
                if (_swapRoutine != null && _swapRoutine.Finished) _swapRoutine = null;
            }
        }

        protected override void Start()
        {
            base.Start();
        }

        #endregion

        #region Properties

        public IEntity Entity
        {
            get { return _entity; }
        }
      
        public AmmoPouch AmmoPouch
        {
            get { return _ammoPouch; }
        }

        public PlayerWeaponPouch WeaponPouch
        {
            get { return _weaponPouch; }
        }

        public bool IsSwapping
        {
            get { return _swapRoutine != null && !_swapRoutine.Finished; }
        }
      
        #endregion

        #region Methods
      
        private System.Collections.IEnumerator SetWeapon(IWeapon weapon)
        {
            _entity.InSwappingWeapon = true;

            /*
             * make sure we drop weapon if it's up...
             * this technically shouldn't happen since we stopped the ability to swap weapon while aiming
             * but we're keeping it here just in case it happens
             */
            IRadicalWaitHandle waitHandle = null;
            if (_currentWeaponStyle != null)
            {
                waitHandle = _currentWeaponStyle.DropWeapon();
                _currentWeaponStyle = null;
            }
            if (_lockon != null) _lockon.DropTarget();
            if (_melee != null) _melee.IsActiveWeapon = false;
            if (_freeFocusMelee != null) _freeFocusMelee.IsActiveWeapon = false;
            if (_ranged != null) _ranged.IsActiveWeapon = false;
            _entity.InStrafe = false;

            if (waitHandle != null && !waitHandle.IsComplete)
            {
                yield return waitHandle;
            }

            //disable weapons
            for (int i = 0; i < _weaponPouch.Weapons.Count; i++)
            {
                _weaponPouch.Weapons[i].gameObject.SetActive(false);
            }

            //set the new weapon
            _currentWeaponStyle = this.GetWeaponStyle(weapon);
            if (_currentWeaponStyle != null && _currentWeaponStyle.SetWeapon(weapon))
            {
                _currentWeaponStyle.IsActiveWeapon = true;

                //play swap in animation
                ISPAnim a;
                if(_currentWeaponStyle.WeaponAnimator != null &&
                    _currentWeaponStyle.WeaponAnimator.PlaySwapIn(out a))
                {
                    yield return a;
                }
            }

            _swapRoutine = null;
            _entity.InSwappingWeapon = false;
        }

        private IPlayerWeaponStyle GetWeaponStyle(IWeapon weapon)
        {
            if (weapon is IMeleeWeapon)
            {
                //TODO - distinguish between focused/free-focus melee weapons
                switch((weapon as IMeleeWeapon).Style)
                {
                    case MeleeWeaponStyle.Undefined:
                    case MeleeWeaponStyle.Focused:
                        return _melee;
                    case MeleeWeaponStyle.FreeFocused:
                        return _freeFocusMelee;
                }
            }
            else if (weapon is GunWeapon)
            {
                return _ranged;
            }

            return null;
        }

        #endregion

        #region IPlayerActionStyle Interface

        bool IPlayerActionStyle.OverridingAction
        {
            get { return false; }
        }

        void IPlayerActionStyle.OnStateEntered(PlayerActionMotor motor, IPlayerActionStyle lastState)
        {

        }

        void IPlayerActionStyle.OnStateExited(PlayerActionMotor motor, IPlayerActionStyle nextState)
        {

        }

        void IPlayerActionStyle.DoUpdate(MansionInputDevice input, bool isActiveAction)
        {
            if (!this.enabled || _entity.Stalled) return;
            if (_weaponPouch == null) return;
            //if (_ranged.WeaponIsDrawn || _melee.WeaponIsDrawn) return;

            if(!this.IsSwapping)
            {
                _entity.InSwappingWeapon = false;

                if (input.GetButtonState(MansionInputs.BumpRight) == ButtonState.Down
                    || input.GetButtonState(MansionInputs.BumpLeft) == ButtonState.Down)
                {
                    var inventory = Services.Get<IInventoryDisplay>();
                    if(inventory != null)
                    {
                        inventory.Show(InventoryDisplayMode.WeaponSwap);
                    }
                }
            }
        }

        #endregion

        #region ICurrentWeaponChangedHandler Interface

        void PlayerWeaponPouch.ICurrentWeaponChangedHandler.OnCurrentWeaponChanged(PlayerWeaponPouch pouch, IWeapon weapon)
        {
            if (_currentWeaponStyle != null && _currentWeaponStyle.Weapon == weapon) return;

            if(_swapRoutine != null)
            {
                _swapRoutine.Cancel();
                _swapRoutine = null;
            }

            _swapRoutine = this.StartRadicalCoroutine(this.SetWeapon(weapon));
            if (_swapRoutine != null && _swapRoutine.Finished) _swapRoutine = null;
        }

        #endregion

    }

}

Basically during update we check for tapping of the L/R bumpers which pull up the quick inventory display. And when a weapon is selected that ‘SetWeapon’ is called which swaps out the weapon. In doing so we clean up what needs to be, determine the appropriate WeaponStyle controller (this would be the ‘PlayerShoot’ in your example’) and then signal to that to do the appropriate and play animations. Note that animations are all played by an abstracted ‘IWeaponAnimator’ interface so that it can integrate with either Legacy or Mecanim.

When swapping to a gun that means we used the ‘PlayerRangedWeaponStyle’. Please note that technically this uses the ‘IPlayerWeaponStyle’ interface, so really if we wanted we could have an abstraction of this, thus allowing us to composite any weapon style we wanted. But for now we really only have the PlayerRangedWeaponStyle in use for ranged weapons.

using UnityEngine;
using System.Collections.Generic;
using System.Linq;

using com.spacepuppy;
using com.spacepuppy.Anim;
using com.spacepuppy.Movement;
using com.spacepuppy.SPInput;
using com.spacepuppy.Utils;

using com.mansion.Entities.Weapons;
using com.mansion.Messages;
using com.mansion.Entities.Proxy;
using com.mansion.UserInput;
using com.mansion.Statistics;

namespace com.mansion.Entities.Actors.Player
{

    [RequireComponentInEntity(typeof(PlayerEntity))]
    public class PlayerRangedWeaponStyle : SPComponent, IPlayerWeaponStyle
    {

        private enum InputCacheState
        {
            None,
            Fire,
            Reload
        }

        private enum ActionState
        {
            None,
            Firing,
            Reloading
        }

        #region Fields

        [Header("Weapon")]
        [SerializeField]
        private GunWeapon _weapon;
        [SerializeField]
        private Transform _gunAimOrigin;
        [SerializeField]
        [DefaultFromSelf(Relativity = EntityRelativity.Entity)]
        private AmmoPouch _ammo;

        [Header("References")]
        [SerializeField]
        [DefaultFromSelf]
        private PlayerActionMotor _actionMotor;
        [SerializeField]
        [DefaultFromSelf]
        private MovementMotor _movementMotor;
        [SerializeField]
        [DefaultFromSelf(Relativity = EntityRelativity.Entity)]
        private MovementAnimatorProxy _movementAnimatorProxy;
        [SerializeField]
        [DefaultFromSelf]
        private PlayerLockOn _lockon;
      


        [System.NonSerialized()]
        private PlayerEntity _entity;

        [System.NonSerialized]
        private IReadiedWeaponAnimator _weaponAnimator;
        [System.NonSerialized]
        private PlayerWeaponAux _weaponAux;

        [System.NonSerialized()]
        private InputCacheState _inputCache;
        [System.NonSerialized]
        private ActionState _state;

        #endregion

        #region CONSTRUCTOR

        protected override void Awake()
        {
            base.Awake();

            _entity = SPEntity.Pool.GetFromSource<PlayerEntity>(this);
            if (_movementMotor == null) _movementMotor = this.GetComponent<MovementMotor>();
            if (_actionMotor == null) _actionMotor = this.GetComponent<PlayerActionMotor>();
            if (_lockon == null) _lockon = this.GetComponent<PlayerLockOn>();
        }

        protected override void Start()
        {
            base.Start();

            this.SetWeapon(_weapon);
        }

        #endregion

        #region Properties
      
        public Transform GunAimOrigin
        {
            get { return _gunAimOrigin; }
            set { _gunAimOrigin = value; }
        }

        public AmmoPouch Ammo
        {
            get { return _ammo; }
        }

        public bool WeaponIsDrawn
        {
            get { return this.enabled && _weaponAnimator != null && _weaponAnimator.State != ReadiedWeaponAnimatorState.None; }
        }

        #endregion

        #region Methods

        private void UpdateAsActive(MansionInputDevice input)
        {
            if (_lockon != null && _lockon.HasTarget())
            {
                var dir = (_lockon.LockedOnAspect.transform.position - _gunAimOrigin.position).normalized;
                var a = Mathf.Asin(dir.y) * Mathf.Rad2Deg;
                _weaponAnimator.AimWeight = a / 45f;
            }
            else
            {
                _weaponAnimator.AimWeight = 0f;
            }
          
            switch (_weaponAnimator.State)
            {
                case ReadiedWeaponAnimatorState.Holstering:
                case ReadiedWeaponAnimatorState.None:
                    this.ForceHolsterWeapon();
                    return;
                case ReadiedWeaponAnimatorState.Ready:
                    {
                        if (input.GetButtonState(MansionInputs.Aim) <= ButtonState.None)
                        {
                            this.ForceHolsterWeapon();
                            return;
                        }
                        else if(_state > ActionState.None || _entity.AttackNotifier.InAttack)
                        {
                            if (input.GetButtonState(MansionInputs.Action) == ButtonState.Down)
                            {
                                _inputCache = InputCacheState.Fire;
                            }
                            else if (input.GetButtonState(MansionInputs.Reload) == ButtonState.Down)
                            {
                                _inputCache = InputCacheState.Reload;
                            }
                        }
                        else if (_inputCache == InputCacheState.Fire || input.GetButtonState(MansionInputs.Action) == ButtonState.Down)
                        {
                            this.StartRadicalCoroutine(this.DoFire(_weapon));
                        }
                        else if (_inputCache == InputCacheState.Reload || input.GetButtonState(MansionInputs.Reload) == ButtonState.Down)
                        {
                            this.StartRadicalCoroutine(this.DoReload(_weapon));
                        }
                    }
                    break;
                case ReadiedWeaponAnimatorState.Drawing:
                case ReadiedWeaponAnimatorState.Firing:
                case ReadiedWeaponAnimatorState.Reload:
                    {
                        if (input.GetButtonState(MansionInputs.Aim) <= ButtonState.None)
                        {
                            this.ForceHolsterWeapon();
                            return;
                        }
                        else if(input.GetButtonState(MansionInputs.Action) == ButtonState.Down)
                        {
                            _inputCache = InputCacheState.Fire;
                        }
                        else if(input.GetButtonState(MansionInputs.Reload) == ButtonState.Down)
                        {
                            _inputCache = InputCacheState.Reload;
                        }
                    }
                    break;
            }

            if (_weaponAux != null && _weaponAux.ProneStyle > WeaponProneStyle.ProneWhileAiming) _entity.Stun.Begin(this);
            _entity.InStrafe = true;

            if (_weaponAux != null) _actionMotor.MovementSettings = _weaponAux.OverrideAimMovementSettings;

            var h = input.GetDualAxleState(MansionInputs.RStick).x;
            if (_lockon != null)
            {
                if (!_lockon.HasTarget() || h != 0f)
                {
                    _lockon.AttemptFindTarget();
                }
                else
                {
                    _lockon.FaceTarget();

                    if (_lockon.LockedOnEntity.Type == IEntity.EntityType.NPC)
                        _lockon.AttemptFindTarget();
                }
            }

            _actionMotor.ResetIdleActionTicker(false);
        }
      
        private void UpdateAsInactive(MansionInputDevice input)
        {
            if (_actionMotor.IsInactiveUpdateOverriden(this) || _weaponAnimator == null) return;

            if (_weaponAnimator.State <= ReadiedWeaponAnimatorState.None)
            {
                if (_weapon == null) return;

                if (_weaponAnimator.State <= ReadiedWeaponAnimatorState.None)
                {
                    ISPAnim anim;

                    if (input.GetButtonState(MansionInputs.Aim) > ButtonState.None)
                    {
                        //draw weapon
                        _weaponAnimator.Draw(out anim);
                        _actionMotor.ResetIdleActionTicker(true);
                        _actionMotor.States.StackState(this);
                        if (_weaponAux != null) _actionMotor.MovementSettings = _weaponAux.OverrideAimMovementSettings;
                    }
                    else if (input.GetButtonState(MansionInputs.Reload) == ButtonState.Down)
                    {
                        //reload weapon
                        float ammoCnt = _ammo.GetAmmoCount(_weapon.AmmoType) - _weapon.AmmoInMagazine;
                        if (ammoCnt > 0)
                        {
                            if (_weapon.AmmoInMagazine < _weapon.MagazineSize && _weaponAnimator.Reload(out anim))
                            {
                                _weapon.Reload(_ammo.GetAmmoCount(_weapon.AmmoType));
                            }
                        }
                        else
                        {
                            if (_weaponAnimator.ReloadNoAmmo(out anim))
                            {
                                _weapon.ReloadNoAmmo();
                            }
                        }
                        _actionMotor.ResetIdleActionTicker(false);
                    }

                    _inputCache = InputCacheState.None;
                }
            }
            else
            {
                this.ForceHolsterWeapon();
            }
        }





        private System.Collections.IEnumerator DoFire(GunWeapon weapon)
        {
            _state = ActionState.Firing;
            _entity.AttackNotifier.BeginAttack(weapon);
            if (_weaponAux != null && _weaponAux.ProneStyle == WeaponProneStyle.ProneWhileAttacking) _entity.Stun.Begin(this);

            _inputCache = InputCacheState.None;
            float ammoCnt = _ammo.GetAmmoCount(_weapon.AmmoType);

            ISPAnim anim = null;
            if (ammoCnt <= 0f)
            {
                _weapon.AmmoInMagazine = 0f;
                if (_weaponAnimator.FireEmptyShot(out anim))
                {
                    _inputCache = InputCacheState.None;
                    _weapon.OnFireEmptyShot.ActivateTrigger(_weapon, null);
                    Messaging.Broadcast<IPlayerFiredGunGlobalHandler>((o) => { o.OnWeaponFiredEmptyShot(_entity, _weapon); });
                }
                else
                {
                    _inputCache = InputCacheState.Fire;
                }
            }
            else if (_weapon.AmmoInMagazine <= 0f)
            {
                if (_weapon.QuickReload)
                {
                    _weapon.Reload(ammoCnt);
                }
                else
                {
                    if (_weaponAnimator.Reload(out anim))
                    {
                        _weapon.Reload(ammoCnt);
                    }
                    else
                    {
                        _inputCache = InputCacheState.Fire;
                    }
                }
            }
            else if (_weaponAnimator.Fire(out anim))
            {
                _ammo.RemoveAmmo(_weapon.AmmoType, 1f);
                _entity.AttackNotifier.BeginAttack(_weapon);
                bool hit = false;

                if (_lockon != null && _lockon.HasTarget())
                    hit = _weapon.Fire(_entity, _gunAimOrigin.position, (_lockon.LockedOnAspect.transform.position - _gunAimOrigin.position).normalized);
                else
                    hit = _weapon.Fire(_entity, _gunAimOrigin);
              
                if (hit && _lockon != null)
                    _lockon.DropTarget();

                _entity.AttackNotifier.EndAttack(_weapon);

                var stats = Services.Get<GameStatistics>();
                if (stats != null)
                    stats.AdjustStat(GameStatistics.STAT_ROUNDSFIRED, 1);

                Messaging.Broadcast<IPlayerFiredGunGlobalHandler>((o) => { o.OnWeaponFired(_entity, _weapon); });
            }
            else
            {
                _inputCache = InputCacheState.Fire;
            }

            yield return anim;

            if (_weaponAux == null || _weaponAux.ProneStyle < WeaponProneStyle.ProneWhileAiming) _entity.Stun.End(this);
            _entity.AttackNotifier.EndAttack(weapon);
            _state = ActionState.None;
        }

        private System.Collections.IEnumerator DoReload(GunWeapon weapon)
        {
            _state = ActionState.Reloading;
            _entity.AttackNotifier.BeginAttack(weapon);

            ISPAnim anim = null;
            float ammoCnt = _ammo.GetAmmoCount(weapon.AmmoType) - weapon.AmmoInMagazine;
            if (ammoCnt > 0)
            {
                if (weapon.AmmoInMagazine < weapon.MagazineSize && _weaponAnimator.Reload(out anim))
                {
                    weapon.Reload(_ammo.GetAmmoCount(weapon.AmmoType));
                }
            }
            else
            {
                if (_weaponAnimator.ReloadNoAmmo(out anim))
                {
                    weapon.ReloadNoAmmo();
                }
            }
            yield return anim;

            _inputCache = InputCacheState.None;
            _entity.AttackNotifier.EndAttack(weapon);
            _state = ActionState.None;
        }

        #endregion

        #region IPlayerActionStyle Interface

        bool IPlayerActionStyle.OverridingAction
        {
            get
            {
                return _movementMotor.States.Current is IPlayerMovementStyle && _weaponAnimator != null && _weaponAnimator.State > ReadiedWeaponAnimatorState.None;
            }
        }

        void IPlayerActionStyle.OnStateEntered(PlayerActionMotor motor, IPlayerActionStyle lastState)
        {
        }

        void IPlayerActionStyle.OnStateExited(PlayerActionMotor motor, IPlayerActionStyle nextState)
        {
            if (_weaponAnimator != null && _weaponAnimator.State > ReadiedWeaponAnimatorState.None)
            {
                ISPAnim anim;
                _weaponAnimator.Holster(out anim);
            }

            if (_lockon != null) _lockon.DropTarget();
            _inputCache = InputCacheState.None;
            _entity.InStrafe = false;
        }

        void IPlayerActionStyle.DoUpdate(MansionInputDevice input, bool isActiveAction)
        {
            if (!this.enabled || _entity.Stalled) return;
            if (_weapon == null) return;

            if (isActiveAction)
                this.UpdateAsActive(input);
            else
                this.UpdateAsInactive(input);
        }
      
        #endregion

        #region IPlayerWeaponStyle Interface

        public bool IsActiveWeapon
        {
            get { return this.enabled; }
            set { this.enabled = value; }
        }

        public IWeapon Weapon
        {
            get { return _weapon; }
        }

        public IReadiedWeaponAnimator WeaponAnimator
        {
            get { return _weaponAnimator; }
        }

        public bool SetWeapon(IWeapon weapon)
        {
            this.ForceHolsterWeapon();

            _weapon = weapon as GunWeapon;
            if (_weapon != null) _weapon.gameObject.SetActive(true);

            _weaponAux = null;
            if (_weapon.GetComponentInChildren<PlayerWeaponAux>(out _weaponAux))
            {
                _weaponAnimator = _weaponAux.WeaponAnimator;
                if (_movementAnimatorProxy != null) _movementAnimatorProxy.SetCurrent(_weaponAux.MovementAnimator);
                _actionMotor.MovementSettings = _weaponAux.OverrideMovementSettings;
            }
            else
            {
                _weaponAnimator = null;
                if (_movementAnimatorProxy != null) _movementAnimatorProxy.SetCurrent(null);
                _actionMotor.MovementSettings = null;
            }
            return _weapon != null;
        }

        public IRadicalWaitHandle DropWeapon()
        {
            var result = this.ForceHolsterWeapon();
            _weapon = null;
            _weaponAnimator = null;
            return result;
        }

        public IRadicalWaitHandle ForceHolsterWeapon()
        {
            if (_lockon != null) _lockon.DropTarget();
            if (_weaponAux != null) _actionMotor.MovementSettings = _weaponAux.OverrideMovementSettings;

            _inputCache = InputCacheState.None;
            _state = ActionState.None;
            _entity.AttackNotifier.EndAttack(_weapon);
            _entity.Stun.End(this);

            ISPAnim anim = null;
            if (_weaponAnimator != null && _weaponAnimator.State > ReadiedWeaponAnimatorState.None)
            {
                _weaponAnimator.Holster(out anim);
            }

            if (object.ReferenceEquals(_actionMotor.States.Current, this))
            {
                _actionMotor.States.PopAllStates();
                return null;
            }

            return anim;
        }

        #endregion

    }

}

This handles all the logic about how the player acts while holding a gun. It plays the appropriate animations when necessary, it handles the inputs for when to draw the weapon and the holster it. It tells the weapon when fire was pressed. That sort.

Note, it does not handle the actual firing logic. That’s the guns job. It just handles signaling to the weapon to fire.

…

And finally we have a ‘GunWeapon’. This represents nearly all guns. We can inherit from it if we wanted to do otherwise. Or really if I could refactor this into a ‘IGunWeapon’ interface if need be in the future to allow for even more custom guns.

using UnityEngine;
using System.Collections.Generic;

using com.spacepuppy;
using com.spacepuppy.Scenario;
using com.spacepuppy.Utils;

namespace com.mansion.Entities.Weapons
{

    public class GunWeapon : SPComponent, IAmmoLoadedWeapon
    {

        #region Fields

        [SerializeField]
        private WeaponTag _tag;

        [SerializeField]
        private AmmoType _ammoType;
        [SerializeField()]
        [UnityEngine.Serialization.FormerlySerializedAs("_clipSize")]
        private DiscreteFloat _magazineSize = 6;
        [SerializeField()]
        [UnityEngine.Serialization.FormerlySerializedAs("_ammoInClip")]
        private DiscreteFloat _ammoInMagazine = 6;
        [SerializeField()]
        private float _damage = 10f;
        [SerializeField]
        [EnumFlags]
        private CritType _crit;
        [SerializeField]
        [Range(0f, 1f)]
        private float _critPercent;

        [Header("Event Triggers")]
        [SerializeField()]
        private Trigger _onFireWeapon;
        [SerializeField()]
        private Trigger _onReloadWeapon;
        [SerializeField()]
        private Trigger _onFireEmptyShot;
        [SerializeField()]
        private Trigger _onReloadNoAmmo;
        [SerializeField]
        private Trigger _onStrike;
        [SerializeField]
        private Trigger _onKill;
      
        #endregion

        #region Properties

        public AmmoType AmmoType
        {
            get { return _ammoType; }
            set { _ammoType = value; }
        }
      
        public float MagazineSize
        {
            get { return _magazineSize; }
            set { _magazineSize = value; }
        }

        public float AmmoInMagazine
        {
            get { return _ammoInMagazine; }
            set { _ammoInMagazine = Mathf.Clamp(value, 0f, _magazineSize); }
        }

        public bool QuickReload
        {
            get;
            set;
        }

        public Trigger OnFireWeapon { get { return _onFireWeapon; } }

        public Trigger OnReloadWeapon { get { return _onReloadWeapon; } }

        public Trigger OnFireEmptyShot { get { return _onFireEmptyShot; } }

        public Trigger OnReloadNoAmmo { get { return _onReloadNoAmmo; } }

        #endregion

        #region Methods

        public float Reload(float max)
        {
            float amt = Mathf.Min((float)max, (float)_magazineSize);
            if(amt > 0f)
            {
                _onReloadWeapon.ActivateTrigger(this, null);
                float result = amt - _ammoInMagazine;
                _ammoInMagazine = amt;
                return result;
            }
            else
            {
                _onReloadNoAmmo.ActivateTrigger(this, null);
                return 0f;
            }
        }

        public void ReloadNoAmmo()
        {
            _onReloadNoAmmo.ActivateTrigger(this, null);
        }

        /// <summary>
        /// Fires gun causing damage, Returns true if enemy died from strike
        /// </summary>
        /// <param name="dir">direction of gun fire</param>
        /// <param name="ignoreFilter">A delegate which returns true if the entity in question should be ignored</param>
        /// <returns></returns>
        public bool Fire(IEntity entity, Transform shotFireTransform, WeaponStrikeFilter ignoreFilter = null)
        {
            return this.Fire(entity, shotFireTransform.position, shotFireTransform.forward, ignoreFilter);
        }

        /// <summary>
        /// Fires gun causing damage, Returns true if enemy died from strike
        /// </summary>
        /// <param name="dir">direction of gun fire</param>
        /// <param name="positionOffset">Offset from gun position</param>
        /// <param name="ignoreFilter">A delegate which returns true if the entity in question should be ignored</param>
        /// <returns></returns>
        public bool Fire(IEntity entity, Vector3 shotFirePos, Vector3 dir, WeaponStrikeFilter ignoreFilter = null)
        {
            if (_ammoInMagazine <= 0f)
            {
                _onFireEmptyShot.ActivateTrigger(this, null);
                return false;
            }
            _ammoInMagazine--;

            _onFireWeapon.ActivateTrigger(this, null);

            RaycastHit hit;
            if (Physics.Raycast(shotFirePos, dir, out hit, float.PositiveInfinity, Constants.MASK_HITBOX | Constants.MASK_OBSTRUCTION, QueryTriggerInteraction.Collide))
            {
                if (hit.collider.gameObject.layer == Constants.LAYER_HITBOX)
                {
                    var e = SPEntity.Pool.GetFromSource<IEntity>(hit.collider);
                    if (e != null && e.HealthMeter != null)
                    {
                        if (e == entity) return false;
                        if (ignoreFilter != null && ignoreFilter(e, hit.collider)) return false;
                      
                        if (e.HealthMeter.Strike(this))
                        {
                            _onKill.ActivateTrigger(this, e);
                            return true;
                        }
                        else
                        {
                            _onStrike.ActivateTrigger(this, e);
                            return false;
                        }
                    }
                }
            }

            return false;
        }

        #endregion

        #region IWeapon Interface

        public WeaponTag Tag
        {
            get { return _tag; }
            set { _tag = value; }
        }

        public float Damage
        {
            get { return _damage; }
            set { _damage = value; }
        }

        public CritType Crit
        {
            get { return _crit; }
            set { _crit = value; }
        }

        public float CritPercent
        {
            get { return _critPercent; }
            set { _critPercent = Mathf.Clamp01(value); }
        }

        #endregion

    }

}

In it we have things like the magazine size, how much ammo is in the magazine, damage, crit type, crit percent, some triggers (UnityEvents) for the various events so that we can wire up sound FX and particle FX.

And yes we even have the ‘WeaponTag’ which is similar to your ‘WeaponList’. Which has these values:

    public enum WeaponTag
    {
        /*
         * Only ever append new entries. Inserting them in the middle will mess up any already configured weapons.
         */
      
        Undefined = 0,
        Knife,
        NineMM,
        Shotgun,
        Revolver,
        BBGun,
        ElephantGun,
        BombFire,
        BombCryo,
        BombElectric,
        Crowbar,
        Flashlight
    }

Speaking of that.

Ummm…

Why in god’s name did you name this similar enum of yours ‘WeaponList’? That name is very confusing. It implies that the object is a list. But actually it’s not even an object, it’s a value, but it’s called a ‘List’. Like I get that the enum holds the various types of weapons and it’s sort of in list form.

But that’s not a traditional List. It’s not an actual collection. You can say:

WeaponList lst;
lst.Add(...);

Consider the MSDN example enum:

enum Day {Sat, Sun, Mon, Tue, Wed, Thu, Fri};

It’s called ‘Day’. Not ‘DayList’. Sure we listed off the days in the enum. But when you have a value of Day… it’s only a SINGLE Day.

This same thing goes with your abstracted class ‘WeaponCollection’. Which again implies that it is a collection. But it’s not. It’s a Weapon.

Sure, you’ll inherit from WeaponCollection multiple times possibly. And you now have a ‘set of weapons’. But ‘WeaponCollection’ does not represent that ‘set of weapons’. It’s the abstracted form of that weapon.

Think of it like animal hierarchies.

It’s not called a ‘MammalCollection’, it’s called a ‘Mammal’. My pet dog is not a Mammals, or MammalCollection, it’s a Mammal. All of the people in my house as a colleciton are Mammals, but each individual is a Mammal.

So like… if you had a collection of Mammals, you might say:

List<Mammal> mammalCollection = new List<Mammal>();

Or in the case of weapons, lets call it ‘Weapon’, or ‘AbstractWeapon’. And you’d say:

List<Weapon> weaponCollection = new List<Weapon>();

I mean… I’m not saying you’re wrong necessarily. You can name things however you’d like.

But for most people reading your code, they’re going to be confused. When you wrote this:

void SwapWeapon(WeaponList changeTo) {
    // Disable previous weapon
    weaponDict[currentWeapon].prefab.SetActive(false);

    // Enable next weapon
    weaponDict[changeTo].prefab.SetActive(true);
    // Store script
    currentWeaponScript = weaponDict[changeTo].script;
    // Store animator
    currentWeaponAnimator = weaponDict[changeTo].animator;
    // Fetch shooting cooldown
    shootWaitTime = weaponDict[changeTo].script.GetShootWaitTime();
    nextShootTime = Time.time + shootWaitTime;
    // Current weapon
    currentWeapon = changeTo;
}

I was very confused at first. I was like “Wait, how do you ‘changeTo’ a collection? Am I passing in the collection of weapons the Swap method picks from? But then how is the weapon selected? Ohhhhh, that’s an enum. Weird.”

14

Oh and lastly, if you’re wondering about ‘WeaponPouch’ in the swap weapon controller. Here that is:

using UnityEngine;
using System.Linq;

using com.spacepuppy;
using com.spacepuppy.Anim;
using com.spacepuppy.Utils;

namespace com.mansion.Entities.Weapons
{

    public class PlayerWeaponPouch : WeaponPouch
    {

        #region Fields
     
        [SerializeField]
        [DisableOnPlay]
        private int _currentWeaponIndex;

        [SerializeField]
        [DefaultFromSelf(Relativity = EntityRelativity.Entity)]
        private SPAnimationController _animationController;
     
        [SerializeField]
        [Tooltip("If no mask is defined for a given animation on the WeaponAnimator, this will be used as the mask.")]
        private SPAnimMaskSerializedRef _defaultWeaponAnimMask;

        [System.NonSerialized]
        private IWeapon _currentWeapon;


        private System.Action<ICurrentWeaponChangedHandler, IWeapon> _onChangedCallback;
        private System.Action<ICurrentWeaponChangedHandler, IWeapon> OnChangedCallback
        {
            get
            {
                if (_onChangedCallback == null) _onChangedCallback = (o, w) => o.OnCurrentWeaponChanged(this, w);
                return _onChangedCallback;
            }
        }

        #endregion

        #region CONSTRUCTOR

        protected override void Awake()
        {
            base.Awake();

            _currentWeaponIndex = MathUtil.Clamp(_currentWeaponIndex, _weapons.Count - 1);
        }

        #endregion

        #region Properties

        public SPAnimationController AnimationController
        {
            get { return _animationController; }
            set { _animationController = value; }
        }

        public ISPAnimationMask DefaultWeaponAnimMask
        {
            get { return _defaultWeaponAnimMask.Value; }
            set { _defaultWeaponAnimMask.Value = value; }
        }

        public int CurrentWeaponIndex
        {
            get { return _currentWeaponIndex; }
        }

        public IWeapon CurrentWeapon
        {
            get { return (_currentWeaponIndex >= 0 && _currentWeaponIndex < _weapons.Count) ? this.Weapons[_currentWeaponIndex] : null; }
        }
     
        #endregion

        #region Methods

        public IWeapon SetCurrentWeapon(int index)
        {
            if (_currentWeaponIndex == index) return this.CurrentWeapon;

            _currentWeaponIndex = MathUtil.Clamp(index, _weapons.Count);
            _currentWeapon = _weapons.Count > 0 ? this.Weapons[index] : null;
            this.gameObject.Broadcast<ICurrentWeaponChangedHandler, IWeapon>(_currentWeapon, this.OnChangedCallback);
            return _currentWeapon;
        }

        public IWeapon SetCurrentWeapon(IWeapon weapon)
        {
            return this.SetCurrentWeapon(this.Weapons.IndexOf(weapon));
        }

        protected override void OnWeaponAdded(IWeapon weapon)
        {
            base.OnWeaponAdded(weapon);

            //get references
            var aux = weapon.gameObject.GetComponentInChildren<PlayerWeaponAux>();

            //prepare objects
            weapon.gameObject.SetActive(false);
            weapon.transform.parent = this.WeaponBone;
            weapon.transform.ZeroOut(true);
            if (aux != null && !aux.IsInitialized)
            {
                aux.Init(weapon, _animationController, _defaultWeaponAnimMask.Value);
            }

            if(_currentWeapon == null)
            {
                int index = this.Weapons.IndexOf(weapon);
                if(index == _currentWeaponIndex)
                {
                    _currentWeapon = weapon;
                    this.gameObject.Broadcast<ICurrentWeaponChangedHandler, IWeapon>(weapon, this.OnChangedCallback);
                }
            }
        }

        protected override void OnWeaponRemoved(IWeapon weapon)
        {
            base.OnWeaponRemoved(weapon);
         
            if(_currentWeapon == weapon)
            {
                _currentWeaponIndex = MathUtil.Clamp(_currentWeaponIndex, _weapons.Count);
                _currentWeapon = _weapons.Count > 0 ? this.Weapons[_currentWeaponIndex] : null;
                this.gameObject.Broadcast<ICurrentWeaponChangedHandler, IWeapon>(_currentWeapon, this.OnChangedCallback);
            }
        }

        protected override void OnClearingWeapons()
        {
            base.OnClearingWeapons();

            if(_weapons.Count > 0)
            {
                _currentWeaponIndex = 0;
                _currentWeapon = null;
                this.gameObject.Broadcast<ICurrentWeaponChangedHandler, IWeapon>(null, this.OnChangedCallback);
            }
        }

        #endregion

        #region Special Types

        public interface ICurrentWeaponChangedHandler
        {
            void OnCurrentWeaponChanged(PlayerWeaponPouch pouch, IWeapon weapon);
        }
     
        #endregion

    }
}

It acts as the collection where all weapons are stored and pulled from.

And its base class:

using UnityEngine;
using System.Collections.Generic;

using com.spacepuppy;
using com.spacepuppy.Utils;
using System;
using System.Collections;

namespace com.mansion.Entities.Weapons
{

    public class WeaponPouch : SPComponent
    {

        #region Fields

        [SerializeField]
        [ReorderableArray]
        [TypeRestriction(typeof(IWeapon))]
        [DisableOnPlay]
        protected List<Component> _weapons;
        [System.NonSerialized]
        private WeaponCollection _weaponColl;
      
        [SerializeField]
        private Transform _weaponBone;

        #endregion

        #region CONSTRUCTOR

        protected override void Start()
        {
            base.Start();

            if(!this.IsInitialized) this.InitWeapons();
        }

        internal virtual void InitWeapons()
        {
            if (this.IsInitialized) return;
            this.IsInitialized = true;

            for (int i = 0; i < _weapons.Count; i++)
            {
                var weapon = _weapons[i] as IWeapon;
                if (weapon == null)
                {
                    _weapons.RemoveAt(i);
                    i--;
                }
                else
                {
                    this.OnWeaponAdded(weapon);
                }
            }
        }

        #endregion

        #region Properties

        public bool IsInitialized
        {
            get;
            private set;
        }

        public WeaponCollection Weapons
        {
            get
            {
                if (_weaponColl == null) _weaponColl = new WeaponCollection(this);
                return _weaponColl;
            }
        }

        public Transform WeaponBone
        {
            get { return _weaponBone; }
        }

        #endregion

        #region Methods
      
        protected virtual void OnWeaponAdded(IWeapon weapon)
        {
            if (!this.IsInitialized) this.InitWeapons();
            this.gameObject.Broadcast<IWeaponPouchChangedHandler, IWeapon>(weapon, (o, w) => o.OnWeaponAdded(this, w));
        }

        protected virtual void OnWeaponRemoved(IWeapon weapon)
        {
            this.gameObject.Broadcast<IWeaponPouchChangedHandler, IWeapon>(weapon, (o,w) => o.OnWeaponRemoved(this, w));
        }

        protected virtual void OnClearingWeapons()
        {
            this.gameObject.Broadcast<IWeaponPouchChangedHandler>((o) => o.OnClearingWeapons(this));
        }

        #endregion

        #region Special Types

        public class WeaponCollection : IList<IWeapon>
        {

            private WeaponPouch _owner;

            public WeaponCollection(WeaponPouch pouch)
            {
                if (pouch == null) throw new System.ArgumentNullException("pouch");
                _owner = pouch;
            }


            public int Count { get { return (_owner._weapons != null) ? _owner._weapons.Count : 0; } }

            public bool IsReadOnly { get { return false; } }

            public IWeapon this[int index]
            {
                get { return _owner._weapons[index] as IWeapon; }
            }

            public void Add(IWeapon weapon)
            {
                if (!_owner._weapons.Contains(weapon.component))
                {
                    _owner._weapons.Add(weapon.component);
                    _owner.OnWeaponAdded(weapon);
                }
            }

            public bool Remove(IWeapon weapon)
            {
                if (weapon == null) return false;

                if(_owner._weapons.Remove(weapon.component))
                {
                    _owner.OnWeaponRemoved(weapon);
                    return true;
                }
                else
                {
                    return false;
                }
            }

            public void Clear()
            {
                _owner.OnClearingWeapons();
                if (_owner._weapons != null) _owner._weapons.Clear();
            }

            public bool Contains(IWeapon weapon)
            {
                return _owner._weapons.Contains(weapon);
            }

            public void CopyTo(IWeapon[] array, int arrayIndex)
            {
                for (int i = 0; i < _owner._weapons.Count; i++)
                {
                    array[arrayIndex + i] = _owner._weapons[i] as IWeapon;
                }
            }
          
            public int IndexOf(IWeapon item)
            {
                return _owner._weapons.IndexOf(item.component);
            }

            public void RemoveAt(int index)
            {
                if (index < 0 || index >= _owner._weapons.Count) return;

                var weapon = _owner._weapons[index] as IWeapon;
                _owner._weapons.RemoveAt(index);
                _owner.OnWeaponRemoved(weapon);
            }

            void IList<IWeapon>.Insert(int index, IWeapon weapon)
            {
                if (!_owner._weapons.Contains(weapon.component))
                {
                    _owner._weapons.Insert(index, weapon.component);
                    _owner.OnWeaponAdded(weapon);
                }
            }

            IWeapon IList<IWeapon>.this[int index]
            {
                get { return _owner._weapons[index] as IWeapon; }
                set
                {
                    throw new System.NotSupportedException();
                }
            }

            public Enumerator GetEnumerator()
            {
                return new Enumerator(this);
            }

            IEnumerator<IWeapon> IEnumerable<IWeapon>.GetEnumerator()
            {
                return this.GetEnumerator();
            }

            System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
            {
                return this.GetEnumerator();
            }


            public struct Enumerator : IEnumerator<IWeapon>
            {

                private List<Component>.Enumerator _e;

                public Enumerator(WeaponCollection coll)
                {
                    _e = coll._owner._weapons.GetEnumerator();
                }

                public IWeapon Current
                {
                    get
                    {
                        return _e.Current as IWeapon;
                    }
                }

                public bool MoveNext()
                {
                    return _e.MoveNext();
                }

                object IEnumerator.Current { get { return this.Current; } }

                void IDisposable.Dispose()
                {

                }

                void IEnumerator.Reset()
                {

                }
            }

        }

        #endregion

        #region Special Types

        public interface IWeaponPouchChangedHandler
        {
            void OnWeaponAdded(WeaponPouch pouch, IWeapon weapon);
            void OnWeaponRemoved(WeaponPouch pouch, IWeapon weapon);
            void OnClearingWeapons(WeaponPouch pouch);
        }

        #endregion

    }

}

Where PlayerWeaponPouch is the player specific version, and WeaponPouch is the more generic version. It may be directly or indirectly used, such as by enemies.

15

Why use an enum to define the items at all? We just have a scriptable object with all our items references using the prefab reference.

16

Not sure for OP.

For us, it’s more of a ‘tag’ (hence the name WeaponTag). And weapons don’t ‘require’ being tagged (hence the ‘undefined’ option).

It’s a feature for my artist, it allows him to generalize things across multiple weapons of similar tag (there are technically more than one 9mm for instance).

This is also why I have the big text in the enum warning him to not insert entries. He’s not a coder, so I need that there to remind him to NOT do it.

17

What’s the tag used for though? We have a similar tag for our mags and mag entry points for example a M249 SAW can both take a Stanag mag and a box mag. So it has two mag entry points with different tags.

Btw, I would explicit number those enums if order is important

18

We’re talking about an artist here.

An artist who can barely dress himself on a good day.

Numbers are not his strong suit.

We have version control if he breaks things.

And mind you the ‘order’ is just because the Unity serializer stores enums by their integer value. Changing it screws up the Unity serializer.

I’m not sure why you care… but ok.

It’s an off shoot thing that he needed, and had to be designed in a way that his brain can wrap around it.

A large portion of my job is to come up with designs that from the outside of the code come across simple to understand for my developer. Sometimes they arise early in development (we’re currently releasing ep2 of this series this week, so this code base is 2 years old at this point), and over time he just uses it for stuff. I sometimes suggest “oh, we could refactor that to be better”. And he’s just like “Nahhhhh, why fix what ain’t broke!” Which is very true… my time could be better spent elsewhere.

So god knows what he does with it exactly.

But basically it boils down to this. I write the core mechanics, he puts together the ‘flash & pizazz’.

See he used to work for ‘Vicarious Visions’, on titles like Guitar Hero, Marvel Ultimate Alliance, Skylanders, etc. For a portion of time while he was there his job was to make “combustables”. They were the junk in the games that you could smash and break and what not. So the engineers there wrote up a little system that through their game engines editor they could just wire their equivalent of ‘GameObjects/Components’ together and signal FX, play sounds, play animations, and swap out visuals.

Here is one of the easter eggs he snuck into MUA2 with said system:

It’s a fairly simplistic and rudimentary system that can be used and abused for a lot of stuff thusly making it extremely powerful for him.

And well, we created a very similar system (in concept, not in implementation… we didn’t have access to how the system worked at VV and that’d technically be IP theft… he just wanted something he could wrap his head around). It’s something he was very famliar with, so he could wrap his brain around it. Note that we devised this system long before ScriptableObject was even a thing (we’ve been working together for 7 years with Unity).

So when he needs a way to determine what weapon struck something so that he can react accordingly. This enum works.

Shotgun? OK, head goes kablooey.
BBGun? OK, nothing really happens.
Cryo Bomb? OK, enemy stands still and blue FX play.

And the enum/tag comes with benefits. Not only is he unfamiliar with SOs (though he’s growing used to them these days), if he goes an adds a new weapon. Say a new cryo type weapon, he just tags it accordingly and all the various objects that react to cryo weapons just start working with it. If it were an SO we’d have to update each one. This of course could be mitigated with yet another ‘mask’ SO that has a list of the types under some category… but now we have SO’s storing a list of SO’s. And well… this is already something we have to do with our inventory system and he forgets time and time again to put entries in there. Which I ended up having to write yet another tool that scans the project for new inventory items so we can easily pick out the necessary items and create the games inventory list. So yeah… workflows, it’s all about juggling workflows.

Anyways, we result with something like this:
QuerulousVacantFinwhale

Pretty much everything you’re witnessing on screen here aside from the player movement controls themselves are all done via this system.

And the way things react vary based on the weapon used via this tag. (it’s a little glitchy in this video, is sort of old, proof-of-concept period)

But yes, the tank that is shot only explodes if it’s shot by the correct kind of weapon. And the enemies freezing still afterward (that are near enough) only do so if it was hit by a cryo weapon.

19

So I’ve just skimmed through the code you shared @lordofduct . As we’re getting into the details of things I might add that the game I’m talking about is just a simple 2D project between me and a friend which we are working on for fun, and for the learning experience of course. The majority of your “in between” classes and interfaces would not be necessary for us. I’d have to sit down and really look through all of the code to get a good understanding of what exactly is going.

Considering our simple 2D game where the weapon arsenal and their functions is most likely going to be the most complex system as there’s so much to keep track of (in comparison to the other aspects of the game), I got a few questions about another way of doing it.

First of all, let’s pretend I already renamed PlayerShoot.cs to i.e. WeaponManager.cs.
This script would keep track of all the weapons in the game as well as which one is equipped at any time.
Right now I keep all input related code in PlayerInput.cs. Whenever a weapon swap button is pressed it tells WeaponManager.cs this, and it would perform the actual swap, (mainly disable/enable weapon gameobject).
Should I move swap input relay from PlayerInput to WeaponManager?

The input script also tells WeaponManager.cs whenever the shoot button is being pressed or released. This is looped over in the Update() by WeaponManager.cs as well as checking if we can shoot in terms of fire rate. If both of those are true it tells the current weapon to shoot. It also tells the weapon which way the player is facing. To turn the player I flip the X scale. This way all attached components (colliders, etc) and gameobjects (weapons and their bullet spawn transforms) flip as well.
If I were to keep the fire rate locally in the weapon script, I’d have to constantly call weapon.Shoot(facing) in the WeaponManager.cs. This doesn’t feel right to me; constantly calling a function and sending values for no reason.
Should I move the shoot input relay from PlayerInput to every weapon script?
Where should I loop to check if the shoot button is pressed AND the required parameters are fulfilled?

One of the other weapons I got is a minigun. This gun has a dealy from when the shoot button is hold to when the gun is actually shooting, as it waits for the gun animation to speed up. This is the only weapon so far that requires a constant check for input to check if the gun is ready as well as playing the animation.
Considering the minigun situation I’d say moving the shoot input to the weapon class(es) would be the best option. It would actually resolve a lot of issues.

If I could get some input on the examples above, knowing at least that I’m doing the right thing in the beginning, I should be able to do the most suitable thing when it comes to using inheritance or anything else.

I’ll also go around and rename all my wrongly renamed variables to something much more suitable!
WeaponList → WeaponState.
WeaponCollection.cs → Firearms.cs

20

I would argue what you should take away from my example is:

Consider the job each class has to do.

‘PlayerSwapWeaponActionStyle’ handles the swapping of weapons.
‘PlayerRangedWeaponStyle’ handles the player’s inputs to fire a weapon (yes swap and fire ‘could’ be combined…)
‘GunWeapon’ handles the actual logic of a gun, firing it

2D or 3D.

This is the general idea.

Where as in your original description you had logic/data that pertained to a gun inside your ‘PlayerShoot/WeaponManager’ class. Probably not where it belongs.

The use of interfaces and abstraction in my examples. Yeah, that’s to deal with your fears of how “what if in the future I have 30 properties on my pistol”. That’s what abstraction and what not is about. My ‘PlayerRangedWeaponStyle’ doesn’t care about most of what is going on in ‘GunWeapon’. All it really cares about is getting access to the method called ‘Fire’.