Reloadable clip corrections

[Cheo]

Hello, I've been working on the reloadable clip system which I think is imperfect and needs three important corrections.

First of all, in order to follow the new parent's (or the reloadable clip attachment's) position and rotation, it is necessary for the clip's position and rotation to be set to 0. At the end of the detach branch from DetachAttachClip I recommend simply adding :

clip.localPosition = Vector3.zero;
clip.localRotation = new Quaternion(0, 0, 0, 0);

Without this, the clip will not be positioned correctly under the new parent.

Next, there is an unnecessary complication between the DropClip function and the reloading : when not adding a drop clip prefab, the character will take the clip in their hand, play the reload animation and the clip will be back in the weapon. However, when using a drop clip prefab, an event will call the DropClip function which drops an instantiated prefab, but also deactivates the clip in the character's hand and instantiate a new prefab to be inserted into the weapon, instead of simply reactivating the clip in the hand a few moments later ! Here's the addition I propose : first, we need to create an event and a bool.

        [SerializeField] protected bool m_InstantiateReloadClip;
        public bool InstantiateReloadClip { get => m_InstantiateReloadClip; set => m_InstantiateReloadClip = value; }

        [SerializeField] protected AnimationSlotEventTrigger m_ReactivateClipEvent = new AnimationSlotEventTrigger(true, 0);
        public AnimationSlotEventTrigger ReactivateClipEvent { get { return m_ReactivateClipEvent; } set { m_ReactivateClipEvent.CopyFrom(value); } }

For the event to be used we need to register it under RegisterEventsWhileEquippedAndEnabledInternal.

            m_ReactivateClipEvent.RegisterUnregisterAnimationEvent(
    register, eventTarget, "OnAnimatorItemReactivateClip", ReactivateClip);

A simple wait value of 0.1 should be enough for the pistol reloading, to give the illusion that the character is dropping a clip and taking another one from his pocket.

Next, make this addition into the DropClip function :

            if (m_ReloadDetachAttachClip) {
                clip.gameObject.SetActive(false);
                ReactivateClipEvent.WaitForEvent(true);
            }

Lastly, let's just create the event.

        void ReactivateClip()
        {
            var clip = GetReloadableClip();
            if (clip == null)
            {
                return;
            }
            if (!m_InstantiateReloadClip)
            {
                clip.gameObject.SetActive(true);
            }
            m_ReactivateClipEvent.CancelWaitForEvent();
        }

But there is one last thing to solve before we're able to use this : 3 instances of #if FIRST_PERSON_CONTROLLER are setting a firstPerson bool to a value of true if you have the FPS side of UCC installed, regardless of whether you're using it or not, which may create an error if using a drop clip prefab, as the script will try looking for non existing first person attachments.
I'm not sure how to check for the perspective used so I just commented the 3 problematic sections, but if anyone has a more correct idea please let me know !

Anyway if I haven't forgotten anything that should be it. Just leave the InstantiateReloadClip bool set to false and assign a drop clip prefab along an ID for the clip attachment, and this should work without a needless and confusing clip spawn !

Hope this is helpful, please let me know if it works on your side and if you have ideas on how to ameliorate all this. I'd especially like someone from the Opsive staff to take a look at this, I truly think this is not a minor matter !

Edit : In hindsight, it is not accurate to qualify this clip spawning as unnecessary, as it really depends on the situation : if the clip is initially catched with the character's hand as it is with the pistol reloading animation, then it is indeed useless as the illusion can be easily created without spawning a second clip. If the clip is directly ejected however, it makes sense to spawn a new one directly in the character's hand. So I'd say adding this option allows us to handle both scenarios

 

[Sangemdoko]

Thank you for the detailed report.

This part doesn't work well when using the drop in the demo scene, my guess is that our setup isn't exactly the same:

clip.localPosition = Vector3.zero;
clip.localRotation = new Quaternion(0, 0, 0, 0);

So I added a boolean to make it optionaly:

[Tooltip("Should the reload clip be reset to its parent position & rotation when detached?")]
        [SerializeField] protected bool m_ResetClipTransformOnDetach;

Apart from that I added all your suggestions, and haven't found any issues from my testing so those improvements will be available in the next update.

As for the null reference error with only using Thirdperson with UCC someone reported the same issue:

[BUG] Reloader and droppable clip error

@Sangemdoko As discussed in the discord (Unity 2021.3.12f1, UCC3.0.4_HDRP): Adding a droppable clip to pistol setup 'reloader module' incudes an error when reload is triggered: The reloader setup, which mimics the demo scene (demo seems fine):

opsive.com

Currently we fixed it with a null check.

I'm sending you the full script so that you can test it out and make sure it works as you would expect. If it does not please do let me know