2h, Shield visibility and Dual-Handed Weapons

Bahaa · December 2, 2023, 3:18pm

OK so I developed my own system that basically automatically unequips any weapons that need to be unequipped, when the player is swapping armor or systems out, but there is one minor issue that still exists, and it’s that if I was wielding a weapon and a shield side by side, say a normal sword and a wooden shield, and then I replace them with something… idk… 2-handed or right-handed, something that basically forces both previous items off the players’ hand, the shield visibly stays on the players’ hand, although it is no longer on his equipment system, so it looks something like this:

Needless to say, that shield is not visibly supposed to be there… (and this problem does not occur when I am just unequipping a shield for instance. It only happens when my players’ hands are full)

Can we fix this? Apparently using the ‘Destroy()’ function is not permitted by Unity, as per this error (this is what worked for me earlier in this post, but now it’s not working). After further analysis, I realized that on the hierarchy, the shield is still under the players’ left hand transform, and this is kinda bothering me tbh:

Destroying assets is not permitted to avoid data loss.
If you really want to remove an asset use DestroyImmediate (theObject, true);
UnityEngine.Object:Destroy (UnityEngine.Object)
GameDevTV.Inventories.Equipment:UnequipShieldAndEquipWeapon (GameDevTV.Inventories.Inventory,GameDevTV.Inventories.EquipableItem) (at Assets/GameDev.tv Assets/Scripts/Inventories/Equipment.cs:90)
GameDevTV.Inventories.Equipment:MaxAcceptable (GameDevTV.Inventories.EquipLocation,GameDevTV.Inventories.InventoryItem) (at Assets/GameDev.tv Assets/Scripts/Inventories/Equipment.cs:139)
GameDevTV.UI.Inventories.InventorySlotUI:TryHandleRightClick () (at Assets/GameDev.tv Assets/Scripts/UI/Inventories/InventorySlotUI.cs:98)
UnityEngine.EventSystems.EventSystem:Update ()

And here is my developed code so far, in ‘Equipment.cs’ (I’m posting this here because I seek feedback on ways that we can make this a little better, but generally speaking it seems fine to me. Can we shorten this code a little bit in any way shape or form though, or at least is there something repetitive there that I can eliminate? The main reason I got 2 of what looks like the exact same function, is because of one line at the end that makes all the difference):

public void UnequipWeaponAndEquipShield(Inventory inventory, EquipableItem itemToEquip) {
            
            EquipLocation weaponSlot = EquipLocation.Weapon;
            EquipLocation shieldSlot = EquipLocation.Shield;
            Equipment playerEquipment = this;

            EquipableItem equippedWeapon = playerEquipment.GetItemInSlot(weaponSlot);
            EquipableItem equippedShield = playerEquipment.GetItemInSlot(shieldSlot);

            if (equippedWeapon != null && inventory.HasSpaceFor(equippedWeapon)) {
            inventory.AddToFirstEmptySlot(equippedWeapon, 1);
            RemoveItem(weaponSlot);
            Destroy(equippedWeapon);
            }
            
            if (equippedShield != null && inventory.HasSpaceFor(equippedShield)) {
            inventory.AddToFirstEmptySlot(equippedShield, 1);
            RemoveItem(shieldSlot);
            Destroy(equippedShield);
            }

            AddItem(shieldSlot, itemToEquip);   // Main difference between both functions is in this line
            inventory.RemoveItem(itemToEquip, 1);
        
        }

        public void UnequipShieldAndEquipWeapon(Inventory inventory, EquipableItem itemToEquip) {

            EquipLocation weaponSlot = EquipLocation.Weapon;
            EquipLocation shieldSlot = EquipLocation.Shield;
            Equipment playerEquipment = this;

            EquipableItem equippedWeapon = playerEquipment.GetItemInSlot(weaponSlot);
            EquipableItem equippedShield = playerEquipment.GetItemInSlot(shieldSlot);

            if (equippedWeapon != null && inventory.HasSpaceFor(equippedWeapon)) {
                inventory.AddToFirstEmptySlot(equippedWeapon, 1);
                RemoveItem(weaponSlot);
                Destroy(equippedWeapon);
            }
            
            if (equippedShield != null && inventory.HasSpaceFor(equippedShield)) {
                inventory.AddToFirstEmptySlot(equippedShield, 1);
                RemoveItem(shieldSlot);
                Destroy(equippedShield);
            }

            AddItem(weaponSlot, itemToEquip);   // Main difference between both functions is in this line
            inventory.RemoveItem(itemToEquip, 1);

        }

        public int MaxAcceptable(EquipLocation equipLocation, InventoryItem item)
        {

            Inventory inventory = GetComponent<Inventory>();

            if (item is EquipableItem equipableItem)
            {

                if (!equipableItem.CanEquip(equipLocation, this)) return 0;

                switch (equipLocation)

                {

                    case EquipLocation.Shield:
                        if (GetItemInSlot(EquipLocation.Weapon) is WeaponConfig weapon)
                        {
                            if (weapon == null) {
                                Debug.Log("No weapon equipped, wearing the shield");
                                return 1;
                            }
                            if (!weapon.isRightHanded) {
                                Debug.Log($"{weapon} is left handed, not going to wear the shield");
                                // Unequip that weapon here, and equip the shield
                                UnequipWeaponAndEquipShield(inventory, equipableItem);
                                return 0;
                            }
                            if (weapon.isTwoHanded) {
                                Debug.Log($"{weapon} is two-handed, not going to wear the shield");
                                UnequipWeaponAndEquipShield(inventory, equipableItem);
                                return 0;
                            }
                            Debug.Log($"{weapon} is right-handed, wearing the shield");
                            return 1;
                        }
                        return 1;

                    case EquipLocation.Weapon:
                        if (item is WeaponConfig weaponConfig)
                        {
                            if (!weaponConfig.isRightHanded) {
                                UnequipShieldAndEquipWeapon(inventory, weaponConfig);
                                return 0;
                            }
                            if (weaponConfig.isTwoHanded) {
                                UnequipShieldAndEquipWeapon(inventory, weaponConfig);
                                // Play the 2h weapon animation here
                                return 0;
                            }
                            return 1;
                        }
                        return 1;

                    default: return 1;

                }

            }

            return 0;

        }

Oh, and one last request for something I noticed here. I noticed that the items go to the first slot, but unlike wielding and unwielding weapons, where the replaced weapon goes to the slot of the weapon it’s replacing, this one goes to the first empty slot (which is a result of my own code), and I used that as a temporary placeholder because I forgot which function sends the weapon to be replaced to the replacer’s slot. Which function do we use to get it to replace whatever we have though…? I know it’s a little thing, but to me it makes a difference

Brian_Trotter · December 2, 2023, 7:52pm

It’s almost as if the Hit() animation event wasn’t being called… perhaps because the new animation doesn’t have a Hit Animation event in it?
Unity - Manual: Animation events on imported clips.

Bahaa · December 2, 2023, 8:11pm

I never would’ve guessed that in a thousand years… This is brand new to me, and a VERY interesting concept to learn, thank you for your guidance Brian . These events can override current ones, right? As in, I can fiddle with previous animation events as well with that, right?

For future documentation purposes, this fix was… new, and easy:

Go to the animation file you just imported into the engine, select “Animation”, scroll down to “Events”, open that, click on the pen with a + icon, time your animation to when you want the effect to happen, and write your function name (and any variables it might expect, if any)

Brian_Trotter · December 2, 2023, 8:13pm

Yep

Bahaa · December 2, 2023, 8:16pm

Got it, thanks Brian (for now I just want to fix the shield render issue as well mentioned above, and we can call part 2 of this done, before I start trying out dual hand weapons (although tbh, I am a little hesitant on that one))

Brian_Trotter · December 2, 2023, 8:43pm

This instruction is telling Unity to destroy the ScriptableObject in the assets database (remember that what Equipment holds is the ScriptableObject, NOT the rendered weapon).
What you really want is to instruct the equipped weapon to destroy the Weapon prefab, and for that, you need a reference to the Player’s weapon transforms. The same thing is happening with the shield. You’re trying to destroy the Shield scriptable Object instead of the item on the player.

Bahaa · December 2, 2023, 8:51pm

I honestly always thought ScriptableObjects were objects that inherited from Unity’s “ScriptableObject”, through code. This makes things a little more… complicated for me

Brian_Trotter · December 2, 2023, 8:52pm

Rather than going through the insanity of trying to pass transforms to the Equipment component, take a look at what we do when a weapon is unequipped in Fighter.cs. The default weapon is automatically instantiated and set as the current WeaponConfig, which should be Unarmored for the player. This very action automatically destroys the current weapon…

If only there were a defaultShield with no prefab assigned in Fighter that Fighter could then instruct to instantiate, thereby destroying the equipped shield…

Brian_Trotter · December 2, 2023, 8:53pm

InventoryItem (from which all of our equipment inherits!) is a ScriptableObject.

Hint: If it has a [CreateAssetMenu] above it in the declaration, it’s probably a ScriptableObject.

Bahaa · December 2, 2023, 9:00pm

The only function that I can find in ‘Fighter.cs’ that addresses this, is what we created when making the quiver. This one, to be specific:


    private void UnequipRangedWeapon()
    {
            GetComponent<Fighter>().GetComponent<Inventory>().AddToFirstEmptySlot(currentWeaponConfig, 1, true);
            GetComponent<Fighter>().GetComponent<Equipment>().RemoveItem(EquipLocation.Weapon);
    }

For this one, I don’t think I ever created an empty shield without a Prefab, but this probably won’t be hard to make, right?

Brian_Trotter · December 2, 2023, 9:00pm

Not what I’m talking about.
Take a look at the method that is linked to the equipment updated event. (I’m not on a code computer atm [in fact, I’m on a computer with multiple broken keys, so I can’t type the 2nd letter in euipment, I have to use autocorrect, LOL] so I can’t point to the exact function.

Bahaa · December 2, 2023, 9:03pm

Let me guess, the ‘UpdateWeapon()’ we updated in ‘Fighter.cs’?

Alls good lel, my laptop’s bottom panel recently had a bad hit as well

Brian_Trotter · December 2, 2023, 9:03pm

Pretty much the same as creating a shield with a prefab. Just create a new Shield and call it something like Unshielded.

That would go in a serialized field in Fighter like defaultShield…
When the euipment is updated, if there is no shield euipped, call the defaultShield’s spawn method, which will destroy the existing shield.

What this means is that your method that was causing you the trouble with trying to destroy assets (don’t destroy assets) can just totally skip that Destroy() method, because Fighter will handle it automagically when you remove the items from Equipment

Bahaa · December 2, 2023, 9:35pm

OK frankly speaking, I’m a little baffled here… I went into fighter and created the default Shield variable, but I’m not exactly sure where to put this line (I’m assuming that’s the line that instantiates the empty shield, but I’m not sure how):

if (currentShield != null) baseShield.SpawnEquipableItem(rightHandTransform, leftHandTransform);

There’s only one ‘UpdateWeapon()’ function in “Fighter.cs” that updates the weapons and shields, but there’s no visible spot of where to assign it tbh

Brian_Trotter · December 2, 2023, 9:41pm

Before instantiating the shield prefab, the ShieldConfig should be destroying any previous shield instances (that’s that whole clearing the child of the shield transform passed in from Fighter)

Paste in that method and we’ll take a look

Bahaa · December 2, 2023, 10:12pm

I think I solved that a minute ago. Here’s what I did (please checkproof it for me ):

In ‘fighter.cs’, create the variable known as ‘Base Shield’, which is of type ‘ShieldConfig’, as follows:

[SerializeField] ShieldConfig baseShield;

Create a Scriptable Object known as ‘ShieldConfig’ in Unity, which contains no pickup, sprite or 3D model of any sort. The whole goal is that it’s something empty to replace our shield when it’s taken off (since all it’s solving is a visual issue, nothing more)
In the ‘Fighter.UpdateWeapon()’, check for whether the shield is null or not. If it is, equip the null ‘BaseShield’ Scriptable Object, as follows:

if (shield != null) EquipShield(shield);
        else if (shield == null) EquipShield(baseShield); // This line
        else DestroyOffHandItem();

So now my ‘UpdateWeapon()’ looks like this:

    private void UpdateWeapon() {

        var weapon = equipment.GetItemInSlot(EquipLocation.Weapon) as WeaponConfig;
        var shield = equipment.GetItemInSlot(EquipLocation.Shield) as ShieldConfig;

        if (weapon == null) EquipWeapon(defaultWeapon);
        else EquipWeapon(weapon);

        if (shield != null) EquipShield(shield);
        else if (shield == null) EquipShield(baseShield);
        else DestroyOffHandItem();

    }

For now, everything seems to be working perfectly fine for this system. Tonight (your time)/tomorrow (my time) we can try the dual-hand weapons (for now though I have a question that I’ve been tempted to ask for 3 days now. I’ll drop that and wait to hear from you xD).

Brian_Trotter · December 3, 2023, 1:59am

Can be greatly simplified…
First of all, shield is null, or shield is not null, so the third else clause will never ever ever ever be called (much like getting back together with Taylor Swift again).
If the first condition fails, if(shield ! null), then by definition, the second clause if(shield is null) (my equals key is also broken, as well as my code key, LOL) must be true

        if (shield != null) EquipShield(shield);
        else EquipShield(baseShield);

Not only that, it can be simplified to exactly one line…

     EquipShield(shield!= null? shield : baseshield);

Bahaa · December 3, 2023, 3:05am

(inputs joke about the leaked picture on the Internet of her boyfriend helping her into her car being labelled as her “next album”)

OK jokes aside lel, guess I’m trying the Dual Handed Weapon System now (when I get out of bed, I just woke up). I’ll fix my Update Weapon according to your code first though

Edit: this line:

Worked beautifully. Thank you Master Brian

Then I did the same for weapon, as follows:

EquipWeapon(weapon != null ? weapon : defaultWeapon);

Bahaa · December 3, 2023, 3:59am

OK so here is my thought approach for dual-wielded weapons:

We create a boolean, something called ‘isDualWield’ for example, that checks whether a weapon is dual wielded or not. If it’s dual-wielded, then we can accept a weapon in the shield spot, side by side with the weapon that has ‘isDualWield’ set to true. If not, we don’t accept anything but a shield (I’m thinking of placing a trigger as well, something like ‘isShield’, but I’m not sure if that’s actually necessary), and we can take off the main hand weapon from the players’ hand if he tries wielding an off hand weapon with a weapon that doesn’t have ‘isDualWield’ activated.

As for the off-hand weapons, we can set their equipLocation to be the Shield slot, run some dual-hand animations, double the damage rate (because now we have two weapons dealing damage, still not sure on how we will implement this one tbh), and be able to only wield their counterparts, if we have them on us, similar to what we did in the arrow and quiver system. Thoughts on this?

I’m still thinking of how to implement it though

Bahaa · December 3, 2023, 5:31am

Before we get into this though, the 2h system is still slightly flawed, when the players’ inventory gets filled to the max. What happens is that, when the inventory is filled up, and I try to equip a 2h weapon, naturally I would expect that this operation is not possible, because there’s no space for the shield.

Instead, what happens here is that not only am I still capable of wielding both a 2h weapon and a shield together (just when I thought I’m done with this one… Anyway, this is not supposed to happen), and this issue actually goes on forever from that point onwards, but sometimes (this one is a bit random) I can just flat out lose a weapon from both my inventory and equipment (data loss) when the player attempts to equip a 2h weapon that replaces a one-handed weapon, but the inventory is full (which is also why earlier I wanted my unequipped weapons to replace the slot of the weapon that will replace it, rather than find a new empty slot). Any ideas on how to solve these two issues?

And one last thing. When we save and quit our game, our normal shield literally gets REPLACED by a ‘Base Shield’, when we reload our game (a white, null item that I created to eliminate the visual shield issue before, which is not even supposed to exist in the inventory). So when I reload the game, my normal shield is literally replaced by a ‘Base Shield’

As for the data loss issue, if I am placing my 1h sword on my player, there’s no data loss. If I am replacing it with my 2-hand sword, I permanently lose my 1h sword, and this is a case that only occurs when my inventory is nearly full, or full, and we are FINDING THE FIRST EMPTY SLOT, RATHER THAN REPLACE THE SLOT like we always did before introducing this system (which is why I urgently need to know how to REPLACE (instead of finding the first empty slot) inventory slot contents, rather than find the empty one first, because this one is causing unwanted issues. We did this before with the equipment, I just don’t know where the code is…)