- Home /
Bugs in a pick up items script.
I finally go this script to work on a basic level, but there are some weird things going on.
Sometimes the object that I'm trying to pick up doesn't get picked up until I try to pick it up multiple times.
When I try to pick up a certain object it grabs another certain object.
Like 1 but when I try to drop an item it happens.
Sometimes when I drop the item form 2 item places both of the items in the second issue on the drop position.
Any help is appreciated.
This is my script:
using UnityEngine;
using System.Collections;
#pragma warning disable 0414
namespace CarryingItems {
public class PickUp : MonoBehaviour {
private float RayCastLength = 2.15f;
public LayerMask RayMask;
private float OffsetDropPosition = 2;
// The parent.
private GameObject UtilityProps;
// Where to place the item.
private GameObject Hand;
// The object I'm carrying.
public static GameObject CarriedObject;
public bool ItemPickedUp;
public bool LookingAtObject;
private RaycastHit FilterHit;
void Awake() {
UtilityProps = GameObject.Find("UtilityProps");
Hand = GameObject.Find("ItemHand");
}
void Update() {
CarriedObject = GameObject.FindWithTag("PickedUp");
if(ItemPickedUp == false) {
LookingAtObject = Physics.Raycast(Camera.main.transform.position, Camera.main.transform.forward, out FilterHit, RayCastLength, RayMask.value);
}
if (Input.GetKeyDown("e")) {
if(ItemPickedUp == false) {
if(FilterHit.rigidbody.gameObject != null) {
ItemPickUp();
}
}
else if(ItemPickedUp == true){
ItemDrop();
}
}
}
void ItemPickUp() {
if (LookingAtObject == true) {
FilterHit.rigidbody.gameObject.tag = "PickedUp";
CarriedObject.transform.localScale = new Vector3(1F, 1F, 1F);
CarriedObject.transform.SetParent(Hand.transform, false);
CarriedObject.GetComponent<Rigidbody>().useGravity = false;
CarriedObject.GetComponent<Rigidbody>().isKinematic = true;
CarriedObject.transform.position = Hand.transform.position;
CarriedObject.transform.rotation = Hand.transform.rotation;
ItemPickedUp = true;
CarriedObject.gameObject.GetComponent<Collider>().enabled = false;
}
}
void ItemDrop() {
CarriedObject.transform.localScale = new Vector3(1F, 4F, 25F);
CarriedObject.transform.SetParent(UtilityProps.transform, false);
CarriedObject.transform.position = Camera.main.transform.position + Camera.main.transform.forward*OffsetDropPosition;
CarriedObject.transform.rotation = Hand.transform.rotation;
CarriedObject.GetComponent<Rigidbody>().useGravity = true;
CarriedObject.GetComponent<Rigidbody>().isKinematic = false;
ItemPickedUp = false;
CarriedObject.gameObject.GetComponent<Collider>().enabled = true;
CarriedObject.gameObject.tag = "Carryable";
CarriedObject = null;
}
}
}
First off a couple hints and tips for optimization and readability. You dont have to cache the $$anonymous$$ain camera to a variable as Unity has built in suport for Camera.main this will grab the first instance of a Camera marked as $$anonymous$$ain camera. So where the raycast begins could be stated as Physics.Raycast(Camera.main.transform) saves a bit of memory here when you dont have to cache the main camera. Also the DIRECTION of the Raycast should be Vector3.forward...not Camera.main.transform.forward.
Answer by laurG · Jun 18, 2017 at 02:55 PM
A few tips for optmization:
First of all, never use any finding function like FindObject, FindObjectWithTag, GetComponent, GetComponentInChildren in the Update function (or LateUpdate, FixedUpdate) unless you absolutely have to, which I have to say, that never happens. They are very expensive performance wise. Use them whenever you need to find and object and cache the reference and use that reference when you need to modify it.
Use the Physics.Raycast after Input.GetKeyDown(KeyCode.E) (also use KeyCode enumeration, it is cheaper performance wise than using a string). You don`t need to check every frame if the user is looking at the object, only when he presses the key you want. You will get rid of the LookingAtObject bool as well.
Moreover, I would never rely on the tag of an object like you are using it here. Simply set your CarriedObject = FilterHit.gameObject; and you remove the looking for the object with the given tag to find the carried object.
Here is a cleaner way of doing what you need (untested, but should work). I also modified the naming of some variables using industry standards:
public class PickUp : MonoBehaviour
{
// publics
// The parent.
private GameObject UtilityProps;
// Where to place the item.
private GameObject Hand;
// The object I'm carrying.
public static GameObject CarriedObject;
public LayerMask RayMask;
// privates
public bool m_itemPickedUp;
private float m_rayCastLength = 2.15f;
private float m_offsetDropPosition = 2;
private RaycastHit m_filterHit;
void Awake()
{
UtilityProps = GameObject.Find("UtilityProps");
Hand = GameObject.Find("ItemHand");
}
void Update()
{
if (Input.GetKeyDown(KeyCode.E))
if (m_itemPickedUp)
ItemDrop();
else if (Physics.Raycast(Camera.main.transform.position, Camera.main.transform.forward, out m_filterHit, m_rayCastLength, RayMask.value))
ItemPickUp(m_filterHit.collider.gameObject);
}
void ItemPickUp(GameObject objectToPickup)
{
// unless you've set your LayerMask to only hit the objects that can be picked up (you should do this as well, for optimization purposes), you must check if you can pick up this object
if (objectToPickup.tag != "Carryable") // if you can't pick it up, do nothing then leave this function
return;
// from now on we know we are holding an item
m_itemPickedUp = true;
// make the scene modifications
CarriedObject = objectToPickup;
CarriedObject.transform.localScale = new Vector3(1F, 1F, 1F);
CarriedObject.transform.SetParent(Hand.transform, false);
CarriedObject.GetComponent<Rigidbody>().useGravity = false;
CarriedObject.GetComponent<Rigidbody>().isKinematic = true;
CarriedObject.transform.position = Hand.transform.position;
CarriedObject.transform.rotation = Hand.transform.rotation;
CarriedObject.gameObject.GetComponent<Collider>().enabled = false;
}
void ItemDrop()
{
// from now on, we do not have an object picked up
m_itemPickedUp = false;
// make the scene modifications
CarriedObject.transform.localScale = new Vector3(1F, 4F, 25F);
CarriedObject.transform.SetParent(UtilityProps.transform, false);
CarriedObject.transform.position = Camera.main.transform.position + Camera.main.transform.forward * m_offsetDropPosition;
CarriedObject.transform.rotation = Hand.transform.rotation;
CarriedObject.GetComponent<Rigidbody>().useGravity = true;
CarriedObject.GetComponent<Rigidbody>().isKinematic = false;
CarriedObject.gameObject.GetComponent<Collider>().enabled = true;
// clear the reference
CarriedObject = null;
}
}
The script works but for some reason I have to press e thrice to drop an item.
Try doing some debugging on this to figure out what happens.
Use the Debug.Log ("any message here"); function to display a message whenever the program reaches any point.
For example:
void Update()
{
if (Input.Get$$anonymous$$eyDown($$anonymous$$eyCode.E))
{
Debug.Log ("User pressed E");
// other instructions
}
}
Place the Debug instruction around your code and see where it doesn't execute. That is what I would do. After you find the issue, you can create a fix or a workaround. If you don't find anything, let me know and I will execute it myself.
Answer by Eco-Editor · Jun 18, 2017 at 03:00 PM
Hi, I'm not going to address the script you've posted, but address this tutorial that helped me a lot at a time:https://www.youtube.com/watch?v=runW-mf1UH0∈dex=4&list=RDO8is_EikILA
Your answer
Follow this Question
Related Questions
Pick up script goes straight to drop action 0 Answers
Bugs in a pick up script 1 Answer
Having to press E thrice... 0 Answers
Multiple Cars not working 1 Answer
Distribute terrain in zones 3 Answers