- Home /
Item pickup add to inventory?
I've spent the entire day trying to figure this out to no avail. I have created a script for a simple inventory and I am trying to make the item I want the player to be able to pick up add to the inventory. Its a really simple text inventory but it doesn't seem to work. Here's the code for the inventory:
#pragma strict
var showGUI : boolean = false;
var woodInv : int;
woodInv = woodPickup.woodInvAmount;
function Update ()
{
if (Input.GetKeyDown(KeyCode.F))
{
if(showGUI == false)
{
showGUI = true;
Debug.Log("showGUI = true");
} else{
showGUI = false;
Debug.Log("showGUI = false");
}
}
}
function OnGUI()
{
if(showGUI == true)
{
GUI.Box (Rect (10,10,100,90), "Inventory");
GUI.Label (Rect (15,35,40,30), "Wood:");
GUI.Label (Rect (75,35,40,30), " " + woodInv);
}
}
And here is the code for the wood pickup item:
#pragma strict
static var woodInvAmount : int = 0;
function OnTriggerEnter (Col : Collider)
{
if(Col.tag == "Player")
{
++woodInvAmount;
Destroy(gameObject);
}
}
Any Help would be very much appreciated!
*P.S. This is in UnityScript
Try putting this
woodInv = woodPickup.woodInvAmount;
into function Update...
function Update ()
{
woodInv = woodPickup.woodInvAmount;
if(...
As it is the player that moves into objects, rather than the other way around, why is it that the trigger collision is not made inside a player script? This would negate the need for what currently appears to be a duplication of data
Answer by Serinx · Nov 03, 2014 at 12:31 AM
It looks like your woodInvAmount is being set initially but never updated so only the initial value of 0 will be used.
A quick fix would be to move the line "woodInv = woodPickup.woodInvAmount;" into the update function.
If it were me I would add a function to the Inventory script called "WoodPickup" or something and pass the woodInvAmount as a parameter, the passed in value could then be added to the private WoodInv integer and displayed.
You could then reference this script from within your Wood Pickup Item script and call the function before destroying the object.
Good Luck!
Answer by Kiwasi · Nov 03, 2014 at 12:20 AM
@richyrich has the problem right. Change line 29 to:
GUI.Label (Rect (75,35,40,30), " " + woodPickup.woodInvAmount);
Note that you are heading down a dangerous path using static variables this way. Some deep thinking about your desired structure is probably warranted.
Thanks a lot everyone for the help! $$anonymous$$oving the woodInv variable into update fixed it! I'm a it of a novice when it comes to program$$anonymous$$g, I'm more of an art guy, so I was surprised to even have gotten that far. Now, Bored$$anonymous$$ormon, do you think you could elaborate on what you mean by heading down a dangerous path? Like I said before I kind of new to program$$anonymous$$g so I was just trying to figure out how to get it working in anyway. Thanks!
Static variables belong to the class. So you can only ever have one instance of them. Your inventory system will fall down as soon as you decide to have more then one player.
You also want to consider ownership of the variable. Does it really make sense for your wood pickup to be script to be controlling your player inventory? Each class should have one, and only one responsibility. (That principle gets stretched a bit, but the idea is sound).
Here is an alternate structure. Its C#, because that's the better language :). But the principle is the same.
public class Inventory : $$anonymous$$onoBehaviour {
public int woodInv;
// Your display code as before
}
public class WoodPickUp : $$anonymous$$onoBehaviour {
void OnTriggerEnter (Collider other) {
Inventory inventory = other.gameObject.GetComponent<Inventory>();
if(inventory){
inventory.woodInv++;
Destroy(gameObject);
}
}
}
The advantage of this structure becomes apparent really quickly if you add a second agent, or if you add another script that needs to add or subtract wood from the inventory. It makes far more sense for your shop to access the players inventory then it does to access the WoodPickUp script.
Edit: Worth noting that GetComponent is more expensive then tag checking. Tag checking makes sense if every GameObject with inventory is tagged the same. Send$$anonymous$$essage can also be used for interscript communication.
I see. I never thought about that, thank you. So it would be better to then create a new script that basically controls all of the inventory related actions, and use classes for everything than to have a separate scripts for it all? Sorry for all the questions.
The two classes are both meant to be different scripts. That's one of the differences between C# and JavaScript. In JavaScript the class declaration is implicit. In general the rule is each class/script should do only one job.
You want one inventory script that controls all of the inventory levels. Depending on how complex it gets you could also have that script do the GUI stuff.
You want a separate script on your pick up items.
Oh okay, I think I understand now. Again, sorry for all the questions. Thanks a lot!
Your answer