Want to move to pickup before acquiring? Do this

If you want to be able to move to your pickup to pick it up instead of the click to acquire design @sampattuzzi implemented in the RPG course, make these changes…

frist make sure your prefab has a collider and it is set to trigger (you should already have the required Rigidbody on your player to make your weapon pickups work)

then make these changes to ClickablePickup.cs

using GameDevTV.Inventories;
using RPG.Movement;
using UnityEngine;

namespace RPG.Control
{
    [RequireComponent(typeof(Pickup))]
    public class ClickablePickup : MonoBehaviour, IRayCastable
    {
        Pickup pickup;

        private void Awake()
        {
            pickup = GetComponent<Pickup>();
        }

        private void OnTriggerEnter(Collider other)
        {
            if (other.gameObject.CompareTag("Player"))
            {
                pickup.PickupItem();
            }
        }

        public CursorType GetCursorType()
        {
            if (pickup.CanBePickedUp())
            {
                return CursorType.Pickup;
            }
            else
            {
                return CursorType.FullPickup;
            }
        }

        public bool HandleRaycast(PlayerController callingController)
        {
            if (Input.GetMouseButtonDown(0))
            {
                callingController.GetComponent<Mover>().StartMovementAction(transform.position);
            }
            return true;
        }
    }
}
5 Likes

Awesome, thanks for sharing this implementation.

2 Likes

How do we call startMovementAction. i copied the same and it shows some errors…says mover doesnt contain a defination for startmovementaction.

In the RPG Core Combat course, StartMoveAction is a method in Mover normally called by AI and PlayerController when the goal is to move the character to a different location. It should look something like this:

    public void StartMoveAction(Vector3 destination, float speedFraction)
    {
        GetComponent<ActionScheduler>().StartAction(this);
        MoveTo(destination, speedFraction);
    }

Thanks for the tip @HeathClose .

But with the OnTriggerEnter as it is, the player pickup every loot while simply running over them, without making a decision about it. For a game with lots of loots (ARPG, Diablo like) this is not a good fit.

So I changed it a little bit with the use of a boolean hasBeenSelectionned :

using GameDevTV.Inventories;
using RPG.Movement;
using UnityEngine;

namespace RPG.Control
{
    [RequireComponent(typeof(Pickup))]
    public class ClickablePickup : MonoBehaviour, IRaycastable
    {
        Pickup pickup;
        private bool hasBeenSelectionned = false;

        private void Awake()
        {
            pickup = GetComponent<Pickup>();
        }
        
        private void OnTriggerEnter(Collider other)
        {
            if (hasBeenSelectionned && other.gameObject.CompareTag("Player"))
            {
                pickup.PickupItem();
            }
        }
        

        public CursorType GetCursorType()
        {
            if (pickup.CanBePickedUp())
            {
                return CursorType.Pickup;
            }
            else
            {
                return CursorType.FullPickup;
            }
        }

        public bool HandleRaycast(PlayerController callingController)
        {
            if (Input.GetMouseButtonDown(0))
            {
                callingController.GetComponent<Mover>().StartMoveAction(transform.position, 1);
                hasBeenSelectionned = true;
            }
            return true;
        }
    }
}
1 Like

I also found it strange you could pickup items despite not being close to them. Can’t think of an ARPG that lets you grab stuff across the screen but I digress.

I modified @HeathClose & @Nono’s code a bit more since there was an interaction I didn’t care for and I’m not sure my solution is a good one but it does work.

So you could click on an item (triggering hasBeenSelectionned) but you could then walk away from the item because of a fight or just exploring or whatever. If you happened to walk over/near the item again without clicking on it, you would just pick it up again since that bool never gets reverted back.

So I called a coroutine after you click on it. You trigger the bool, then have a set time to make it to that item before it reverts back to it’s unclicked state. 5 seconds seems like more than enough time to walk across the screen to grab an item but I haven’t played with it a ton.

using GameDevTV.Inventories;
using RPG.Movement;
using UnityEngine;
using System.Collections;

namespace RPG.Control
{
    [RequireComponent(typeof(Pickup))]
    public class ClickablePickup : MonoBehaviour, IRaycastable
    {
        Pickup pickup;
        float resetDelay = 5f;
        private bool hasBeenClicked = false;

        private void Awake()
        {
            pickup = GetComponent<Pickup>();
        }
        
        private void OnTriggerEnter(Collider other)
        {
            if (hasBeenClicked && other.gameObject.CompareTag("Player"))
            {                
                pickup.PickupItem();
            }
        }
        

        public CursorType GetCursorType()
        {
            if (pickup.CanBePickedUp())
            {
                return CursorType.Pickup;
            }
            else
            {
                return CursorType.FullPickup;
            }
        }

        public bool HandleRaycast(PlayerController callingController)
        {
            if (Input.GetMouseButtonDown(0))
            {
                callingController.GetComponent<Mover>().StartMoveAction(transform.position, 1);
                hasBeenClicked = true;
                StartCoroutine("PickupCountdown");
            }
            return true;
        }
        private IEnumerator PickupCountdown()
        {
            yield return new WaitForSeconds(resetDelay);
            hasBeenClicked = false;
        }
    }
}

Any better ways to implement something like this?

1 Like

I created a class “Collector” which takes a Pickup as a target, and uses the exact same logic Fighter does to bring the character to the target, but instead of attacking when reaching the target, I collect the pickup.

Collector needs to be an IAction, and the Pickup would call the other.GetComponent().StartCollection(pickup);

2 Likes

The Collector class seems a good idea too. But in the end I needed the same functionnality (go walk toward something interactable and do something once in range) for not only pickup but also to talk to npc, to open chests or door.
So I created an abstract class that do mostly ClickablePickup was doing. It has a virtual method InteractionEffects that the child class can override. The pickup item is done in it.

namespace RPG.Control
{

    [RequireComponent(typeof(Collider))]
    public abstract  class ClickableElement : MonoBehaviour, IRaycastable
    {

        // Interaction properties.
        private bool hasBeenSelectionned = false;
        [SerializeField] float interactionRange = 3f;
        GameObject player;
        

        private void Start()
        {
            player = GameObject.FindWithTag("Player");
        }


        public virtual CursorType GetCursorType()
        {
            return CursorType.None;
        }

        public bool HandleRaycast(PlayerController callingController)
        {
            if (Input.GetMouseButtonDown(0))
            {
                callingController.GetComponent<Mover>().StartMoveAction(transform.position, 1);
                hasBeenSelectionned = true;
            }
            return true;
        }
        
        private void Update()
        {
            if (!hasBeenSelectionned) return;

            if (IsCloseEnoughToInteract())
            {
                player.GetComponent<Mover>().Cancel();
                hasBeenSelectionned = false;
                InteractionEffects();
            }
        }

        private bool IsCloseEnoughToInteract()
        {
            return Vector3.Distance(player.transform.position, transform.position) < interactionRange;
        }
        
        public virtual void InteractionEffects() {}
    }
}
namespace RPG.Control
{
    [RequireComponent(typeof(Pickup))]
    public class ClickablePickup : ClickableElement
    {
        Pickup pickup;

        private void Awake()
        {
            pickup = GetComponent<Pickup>();
        }

        public override CursorType GetCursorType()
        {
            if (pickup.CanBePickedUp())
            {
                return CursorType.Pickup;
            }
            else
            {
                return CursorType.FullPickup;
            }
        }
        
        public override void InteractionEffects()
        {
            pickup.PickupItem();
        }
    }
}

But what Rudy say about the boolean never being revert is true.
So I think the logic of tracking that information should be handled by the player, and be cancelled / reset, with the help of ActionScheduler. Pretty much was the Collector class of Brian is doing.
I need to find a way to combine them.

1 Like

So I finished the buy/sell portion of the course but this was bugging so much I had to come back to it now. So it’s pretty rough and I’m still a noob so the implementation could probably be done so much better technically but it works.

It started following Brian’s advice about a Collector class (hence my class name) but it got more generic once I also saw the need (like Nono) for a more generic “move to point and interact” mechanic.

using GameDevTV.Inventories;
using RPG.Movement;
using UnityEngine;
using RPG.Core;

namespace RPG.Control
{    
    public class Collector : MonoBehaviour, IAction
    {
        [SerializeField] float interactRange = 1f;
        
        IClickable target;            
        
        private void Update() 
        { 
            if (target == null) return;  
            else if (target != null && !GetIsInRange(target.GetTransform()))
            {
                GetComponent<Mover>().MoveTo(target.GetTransform().position, 1f);
            }
            else
            {
                GetComponent<Mover>().Cancel();
                target.InteractionEffects();
                target = null;
            }
        }        
        
        private bool GetIsInRange(Transform targetTransform)
        {
            return Vector3.Distance(transform.position, targetTransform.position) < interactRange;
        }        

        public void SetTarget(IClickable target)
        {
            GetComponent<ActionScheduler>().StartAction(this); 
            this.target = target;
        }

        public void Cancel()
        {                        
            target = null;            
        }        
    }
}

I use an interface on everything I want to be clickable, like shops, pickups and npc speakers, hence the name of the interface lol. I’m not sure as of right now if there is anything I’ll need to add to it for any other contingencies.

using UnityEngine;

namespace RPG.Core
{
    public interface IClickable
    {
        public void InteractionEffects();
        public Transform GetTransform();                
    }
}

The HandleRaycasts look like this on every instance of IClickable.

public bool HandleRaycast(PlayerController callingController)
        {
            if (Input.GetMouseButtonDown(0))
            {
                callingController.GetComponent<Collector>().SetTarget(this);
            }
            return true;
        }

And the interface implementation looks like this (this one is from pickup) but it’s all the same. Your previous raycast stuff (open shop, pickup item etc) go in InteractionEffects and GetTransform just returns transform of the object (IClickable doesn’t have a transform.)

  public void InteractionEffects()
        {
            pickup.PickupItem();
        }

        public Transform GetTransform()
        {
            return pickup.transform;
        }

Definitely up for hearing some criticism of this bad boy. Hope someone can find it useful. Had a lot of fun figuring it out (go go GDTV!)

Not bad. I’ve seen (and written) a few different ways to handle movement to a target.

Here’s my approach:

MoveableActionBehaviour.cs
using System.Collections.Generic;
using TkrainDesigns.Attributes;
using UnityEngine;

namespace TkrainDesigns.Movement
{
    [RequireComponent(typeof(Mover))]
    [RequireComponent(typeof(ActionScheduler))]
    public abstract class
        MoveableActionBehavior<T> : MonoBehaviour, IAction where T : MonoBehaviour
    {
        protected Health health;
        protected Mover mover;
        protected ActionScheduler actionScheduler;
        protected T target = null;
        protected float acceptanceRadius = 3.0f;
        private Dictionary<string, float> Cooldowns = new Dictionary<string, float>();


        protected virtual void Awake()
        {
            health = GetComponent<Health>();
            mover = GetComponent<Mover>();
            actionScheduler = GetComponent<ActionScheduler>();
        }

        public virtual void StartAction(T newTarget)
        {
            actionScheduler.StartAction(this);
            target = newTarget;
        }

        /// <summary>
        /// In most cases, nothing should be needed to add to Update.  You can override Update (call base.Update())
        /// </summary>
        protected virtual void Update()
        {
            if (!health.IsAlive())
            {
                actionScheduler.CancelCurrentAction();
                enabled = false;
                return;
            }

            UpdateTimers();
            if (target == null) return;
            if (!IsValidAction()) return;
            if (Vector3.Distance(transform.position, target.transform.position) > acceptanceRadius)
            {
                mover.MoveTo(target.transform.position);
                HandleOutOfRange();
            }
            else
            {
                mover.Stop();
                Perform();
            }
        }

        protected abstract void Perform();

        /// <summary>
        /// Override to manage things like canceling an animation state if the character moves out of range.
        /// </summary>
        protected virtual void HandleOutOfRange()
        {
        }


        protected void SetAcceptanceRadius(float value)
        {
            acceptanceRadius = value;
        }

        protected void SetTimer(string key, float value)
        {
            Cooldowns[key] = value;
        }

        protected bool IsCooldownExpired(string key)
        {
            return !Cooldowns.ContainsKey(key);
        }

        void UpdateTimers()
        {
            if (Cooldowns.Count == 0) return;
            List<string> keysToClear = new List<string>();
            foreach (string key in Cooldowns.Keys)
            {
                keysToClear.Add(key);
            }

            foreach (string key in keysToClear)
            {
                Cooldowns[key] -= Time.deltaTime;
                if (Cooldowns[key] < 0) Cooldowns.Remove(key);
            }
        }

        /// <summary>
        /// Override this method to implement custom checks, for example if T is a Health, you may wish to check to see
        /// if the target is alive.
        /// </summary>
        /// <returns></returns>
        protected virtual bool IsValidAction()
        {
            return true;
        }

        public virtual void Cancel()
        {
            //Debug.Log($"{name} cancel()");
            target = null;
        }
    }
}

So you would implement this by inherting from the class with the object that is dealt with… here’s the PickupCollector:

PickupCollector.cs
using GameDevTV.Inventories;
using TkrainDesigns.Movement;
using UnityEngine;

namespace TkrainDesigns.Inventories
{
    [DisallowMultipleComponent]
    public class PickupCollector : MoveableActionBehavior<Pickup>
    {
        protected override void Perform()
        {
            target.PickupItem();
        }
    }
}
1 Like

Privacy & Terms