Destroying weapons we dont need any more

Unity Courses Talk
,
#1

In future parts of this RPG series we’ll implement proper inventory systems, but for now we just need to be able to use various weapons… and of course it helps if we can destroy the old weapon when we pick up a new one.

#3

Is it good way to do this task in Fighter script as below?

image
image794×269 12.2 KB

this is for Weapon script…
image

As for me it is easier to understand and implement

#4

Except that ScriptableObjects are a shared resource… meaning that every character that is using the same weapon uses the same Weapon, and the weaponinstance could be referring to any one of them.

That’s why we have to pass the righthandTransform, leftHandTransform, and animator to the Spawn method. Scriptable Objects should not be storing state that isn’t intended to be a shared state.

1 Like
#5

Thank you for explanation! :slightly_smiling_face:

#6

It feels… complicated… at first.

I can see why we need to do this “destroying” by name in Weapon Scriptable Object script. As you said: Scriptable Objects are shared state.

But Beatabout is proposing to destroy the GameObject reference that is stored in Fighter component, that inherits from Monobehaviour. So I think it will work as well.

Although I see the benefits from having the destroying method in Weapon script.

EDIT: ok, I can see now, that currentWeapon is stored as Weapon Scriptable Object class. So it wouldn’t work unless we store in Fighter script reference to weapon GameObject. My bad.

#8

I feel a little bit confused too about the solution.
Also, having the Weapon script find an destroy a game object linked to the player’s hands seems unnatural.

What about the following solution?

  1. Have the SpawnWeapon method of Weapon script return a GameObject:
public GameObject SpawnWeapon(Transform rightHand, Transform leftHand, Animator animator)
{
    GameObject weaponGameObject = null;
    if (equippedPrefab != null)
    {
        Transform handTransform = GetHandTransform(rightHand, leftHand);
        weaponGameObject = Instantiate(equippedPrefab, handTransform);
    }
    if (animatorOverride != null)
    {
        animator.runtimeAnimatorController = animatorOverride;
    }
    return weaponGameObject;
}
  1. Add a private GameObject equippedWeapon to the Fighter script
  2. Modify the EquipWeapon method of the Fighter script in order to destroy the equippedWeapon GameObject and replace it by the new one (if there is one)
public void EquipWeapon(Weapon weapon)
{
    if (equippedWeapon != null)
        Destroy(equippedWeapon);

    currentWeapon = weapon;
    GameObject weaponGameObject = currentWeapon.SpawnWeapon(rightHand, leftHand, animator);
    if (weaponGameObject != null)
        equippedWeapon = weaponGameObject;
}

Is there any downside to this solution (even for future lectures)?

#9

In my opinion, this part feels a little too complex.

at Spawn, we can save reference to GameObject like:

        GameObject instantiation = null;

        public void Spawn(Transform handLeft, Transform handRight, Animator animator)
        {
            Transform hand = rightHanded ? handRight : handLeft;
            instantiation = Instantiate(weaponPrefab, hand);
            if (animatorOverride)
            {
                animator.runtimeAnimatorController = animatorOverride;
            }
        }

then on a new public method,

 public void DestroyInstantiatedWeapon()
 {
     if(instantiation)
     {
         Destroy(instantiation);
     }
 }

finally, on Fighter:

        public void EquipWeapon(Weapon weapon)
        {
            if (handRight && handLeft && weapon)
            {
                this.equippedWeapon.DestroyInstantiatedWeapon();
                this.equippedWeapon = weapon;
                Animator animator = GetComponent<Animator>();
                this.equippedWeapon.Spawn(handLeft, handRight, animator);
            }
        }
#10

Keeping the spawned weapon in the Weapon SO seems like it will lead to issues, so I would indeed go with the suggestion from Akaki, returning the spawned weapon instance when spawning and having the fighter keep it, until there is a more complete inventory solution in place…

Instead of just destroying the weapon when picking up another one, one could as well spawn a new pickup object, so essentially switching weapons and dropping the old one. That would need some protection against immediately picking it up again and possibly running into a weapon-pickup ping-pong loop, though…

#11

We can’t really do this because the ScriptableObject WeaponConfig is a shared resource. If 10 characters have a basic sword, then each spawned weapon model will be unique so the WeaponConfig will only have a reference to the last copy. In most situations, only the last character to equip a weapon would actually have a weapon model.

Privacy & Terms