Money Updated In Shop Menu Only When Scene Is Started With Menu Open? Helmet Not Buying?

Question

For some reason I need to start the scene with the menu open, then close it, then grab the coin and go to the shop menu in order for the shop ui to update my moneyAmount. If i start the scene with the shop menu closed and pick up the coin then go to my shop menu it doesnt update. And when i buy my helmet it says reference not set to an object even though all im doing is getting my player health component and adding 50 to it so why do i need to reference any kind of object? here are the scripts with my GameControl script first.

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using UnityEngine.UI;
using UnityEngine.SceneManagement;

public class GameControl : MonoBehaviour
{
public Text moneyText;
public static int moneyAmount;
int isHelmetSold;
int isBeltSold;
int isPantsSold;
int isShirtSold;
int isBootsSold;


// Start is called before the first frame update
void Start()
{
    moneyAmount = PlayerPrefs.GetInt("MoneyAmount");
    isHelmetSold = PlayerPrefs.GetInt("IsHelmetSold");
    isBeltSold = PlayerPrefs.GetInt("IsBeltSold");
    isPantsSold = PlayerPrefs.GetInt("IsPantsSold");
    isShirtSold = PlayerPrefs.GetInt("IsShirtSold");
    isBootsSold = PlayerPrefs.GetInt("IsBootsSold");
}

// Update is called once per frame
void Update()
{
    moneyText.text = moneyAmount.ToString();
}

Here is my PageOneShop script which makes my item buyable when amount is reached (for the time being ive only finished my helmet not the rest so if you can ignore all of the public texts and buttons as i know i havent added them yet)

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using UnityEngine.UI;
using UnityEngine.SceneManagement;

public class PageOneShop : MonoBehaviour
{

public static int moneyAmount;
int isHelmetSold;
int isBeltSold;
int isPantsSold;
int isShirtSold;
int isBootsSold;

public Text moneyText;

public Text helmetPrice;
public Text beltPrice;
public Text pantsPrice;
public Text shirtPrice;
public Text bootsPrice;

public Button buyHelmetButton;
public Button buyBeltButton;
public Button buyPantsButton;
public Button buyShirtButton;
public Button buyBootsButton;

void Start()
{
    moneyAmount = PlayerPrefs.GetInt("MoneyAmount");
    isHelmetSold = PlayerPrefs.GetInt("IsHelmetSold");
    isBeltSold = PlayerPrefs.GetInt("IsBeltSold");
    isPantsSold = PlayerPrefs.GetInt("IsPantsSold");
    isShirtSold = PlayerPrefs.GetInt("IsShirtSold");
    isBootsSold = PlayerPrefs.GetInt("IsBootsSold");
}

// Update is called once per frame
void FixedUpdate()
{
    moneyText.text = moneyAmount.ToString();

    isHelmetSold = PlayerPrefs.GetInt("IsHelmetSold");

    if (moneyAmount >= 150 && isHelmetSold == 0)
        buyHelmetButton.interactable = true;
    else
        buyHelmetButton.interactable = false;

    isBeltSold = PlayerPrefs.GetInt("IsBeltSold");

    if (moneyAmount >= 120 && isBeltSold == 0)
        buyBeltButton.interactable = true;
    else
        buyBeltButton.interactable = false;

    isPantsSold = PlayerPrefs.GetInt("IsPantsSold");

    if (moneyAmount >= 100 && isPantsSold == 0)
        buyPantsButton.interactable = true;
    else
        buyPantsButton.interactable = false;

    isShirtSold = PlayerPrefs.GetInt("IsShirtSold");

    if (moneyAmount >= 100 && isShirtSold == 0)
        buyShirtButton.interactable = true;
    else
        buyShirtButton.interactable = false;

    isBootsSold = PlayerPrefs.GetInt("IsBootsSold");

    if (moneyAmount >= 80 && isBootsSold == 0)
        buyBootsButton.interactable = true;
    else
        buyBootsButton.interactable = false;
}

public void buyHelmet()
{
    moneyAmount -= 150;
    GetComponent<PlayerHealth>().maxHealth += 50;
    PlayerPrefs.SetInt("IsHelmetSold", 1);
    helmetPrice.text = "Sold!";
    buyHelmetButton.gameObject.SetActive(false);
}

public void buyBelt()
{
    moneyAmount -= 120;
    GetComponent<PlayerHealth>().maxHealth += 50;
    PlayerPrefs.SetInt("IsBeltSold", 1);
    helmetPrice.text = "Sold!";
    buyBeltButton.gameObject.SetActive(false);
}

public void buyShirt()
{
    moneyAmount -= 100;
    GetComponent<PlayerHealth>().maxHealth += 50;
    PlayerPrefs.SetInt("IsShirtSold", 1);
    helmetPrice.text = "Sold!";
    buyShirtButton.gameObject.SetActive(false);
}

public void buyPants()
{
    moneyAmount -= 100;
    GetComponent<PlayerHealth>().maxHealth += 50;
    PlayerPrefs.SetInt("IsPantsSold", 1);
    helmetPrice.text = "Sold!";
    buyPantsButton.gameObject.SetActive(false);
}

public void buyBoots()
{
    moneyAmount -= 80;
    GetComponent<PlayerHealth>().maxHealth += 50;
    PlayerPrefs.SetInt("IsBootsSold", 1);
    helmetPrice.text = "Sold!";
    buyBootsButton.gameObject.SetActive(false);
}

And here is my coin script on my coins that i pick up.

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class Coin : MonoBehaviour
{
void OnTriggerEnter2D (Collider2D col)
{
    PageOneShop.moneyAmount += 200;
    GameControl.moneyAmount += 200;
    Destroy(gameObject);
}

Answer 1

It's important to figure out the pieces involved. Starting from the minimum amount of code, I think this is a good start:

"GameControl"

using UnityEngine;
using UnityEngine.UI;

public class GameControl : MonoBehaviour
{
    public Text moneyText;
    public static int moneyAmount;

    // Start is called before the first frame update
    // Start is only called while this is active and enabled
    void Start()
    {
        moneyAmount = PlayerPrefs.GetInt("MoneyAmount");
    }

    void Update()
    {
        moneyText.text = moneyAmount.ToString();
    }
}

"PageOneShop"

using UnityEngine;
using UnityEngine.UI;

public class PageOneShop : MonoBehaviour
{
    public static int moneyAmount;

    public Text moneyAmountText;

    public Text helmetPrice;

    // Start is called before the first frame update
    // Start is only called while this is active and enabled
    void Start()
    {
        moneyAmount = PlayerPrefs.GetInt("MoneyAmount");
    }

    // Update is called once per frame
    void Update()
    {
        moneyAmountText.text = moneyAmount.ToString();
    }

    public void buyHelmet()
    {
        moneyAmount -= 150;
        helmetPrice.text = "Sold!";
    }
}

"Coin"

using UnityEngine;

public class Coin : MonoBehaviour
{
    void OnTriggerEnter2D(Collider2D col)
    {
        PageOneShop.moneyAmount += 200;
        GameControl.moneyAmount += 200;
        Destroy(gameObject);
    }
}

Everything in these files is used, and we can replicate our issue. Before this point, my previous answer was correct and you weren't updating the moneyAmount instance inside PageOneShop. Now there is a new issue and it's more subtle, because it's strictly speaking not in your code. Start is a Unity called method, and is called "before the first frame update". Start is also only called if the component is enabled and the object it's attached to is active. If the object it's attached to is de-activated (like I'm guessing your menu is when you load your scene) Start will be called the first time you activate the object (open the menu). Since nowhere are you updating the "MoneyAmount" in PlayerPrefs, that value is still 0. If you pick up the coin, then open the menu for the first time, the value would be 200 but instead is re-set to 0 when this runs:

    void Start()
    {
        moneyAmount = PlayerPrefs.GetInt("MoneyAmount");
    }

Probably the quickest way to fix this is to add this line to coin:

public class Coin : MonoBehaviour
{
    void OnTriggerEnter2D(Collider2D col)
    {
        PageOneShop.moneyAmount += 200;
        GameControl.moneyAmount += 200;
        // Set the PlayerPrefs value
        PlayerPrefs.SetInt("MoneyAmount", GameControl.moneyAmount);
        Destroy(gameObject);
    }
}

At this point even if the menu opens after the coin is picked up, the correct value is retrieved from PlayerPrefs so nothing is overwritten. Potentially a new problem has been introduced, which is even in the editor during testing, this write to PlayerPrefs is occurring so money may accumulate as multiple play sessions are run. There are a few other issues as well. As @derHugo accurately points out you should stop polling for changes in update to an event based approach, and the use of two static variables which are kept in sync like this is at best wasteful, and at worst can cause some real serious headaches.

Let's start by addressing the double static variable use. You could replace all the references of one with the other, but I propose introducing a third class to handle the player's "Wallet" where they keep their money. You could write something as simple as:

public static class Wallet
{
    public static int MoneyAmount;
}

All the references to the other two static moneyAmount variables could be replaced with this Wallet.MoneyAmount.

The polling could now be replaced with events pretty quickly by adding an event to the Wallet class, and replacing the field with a property:

using System;

public static class Wallet
{
    public static event Action OnMoneyAmountChanged;

    private static int moneyAmount;
    public static int MoneyAmount
    {
        get
        {
            return moneyAmount;
        }
        set
        {
            moneyAmount = value;
            // maybe null check, or follow a "never null" approach
            OnMoneyAmountChanged();
        }
    }
}

Instead of always checking moneyAmount you can now add some code like:

    void Start()
    {
        // Listen for changes
        Wallet.OnMoneyAmountChanged += handleMoneyAmountChanged;
    }

    private void OnDestroy()
    {
        // Remember to always remove your listeners when you're done with them
        Wallet.OnMoneyAmountChanged -= handleMoneyAmountChanged;
    }

    // set the text to the value held in the wallet whenever the amount changes
    void handleMoneyAmountChanged()
    {
        moneyText.text = Wallet.MoneyAmount.ToString();
    }

Maybe you don't want the PlayerPrefs.SetInt call in Coin any more, and instead you could move it into the Wallet now. If you put it in a [Conditional] method you could also protect yourself from constantly overwriting PlayerPrefs with every play session in the editor.

using System;
using System.Diagnostics;
using UnityEngine;

public static class Wallet
{
    public static event Action OnMoneyAmountChanged;

    private static readonly string moneyAmountKey = "MoneyAmount";

    private static int moneyAmount;
    public static int MoneyAmount
    {
        get
        {
            return moneyAmount;
        }
        set
        {
            moneyAmount = value;
            OnMoneyAmountChanged();
            // this method call is conditional on a defined constant
            saveMoneyAmount();
        }
    }

    // This part will only run if ENABLE_PLAYERSPREFS is defined.
    [Conditional("ENABLE_PLAYERPREFS")]
    private static void saveMoneyAmount()
    {
        PlayerPrefs.SetInt(moneyAmountKey, moneyAmount);
    }
}

You can type whatever you want into the scripting define symbols text box described in the "Platform custom #defines" section on this page. If you define "ENABLE_PLAYERPREFS" that method will call.

You probably now want to initialize your wallet as well, and to keep things symmetrical you could "close" it as well. This would let you cut down on a lot of reads and writes to player prefs.

    public static void Open()
    {
// this is another way to interact with platform define constants
#if ENABLE_PLAYERPREFS
        moneyAmount = PlayerPrefs.GetInt(moneyAmountKey);
#else
        moneyAmount = 0;
#endif
    }

    public static void Close()
    {
        // maybe you want to do other stuff in here
        saveMoneyAmount();
    }

If you called Open at the start of your application, and Close at the end you could just use the variables in memory the rest of the time and remove the saveMoneyAmount call from the MoneyAmount setter.

As for this second question you added later than my original answer, "And when i buy my helmet it says reference not set to an object even though all im doing is getting my player health component and adding 50 to it so why do i need to reference any kind of object?". You didn't post the line number, but I can guess that it originates on any/all the calls that look like this GetComponent<PlayerHealth>().maxHealth += 50; since those are all in methods on PageOneShop so unless you've added your PlayerHealth component to your menu object that GetComponent will be null, at which point you can refer to What is a NullReferenceException, and how do I fix it?