Feedback Feedback about my simple shop script.

Making shop system and working with buttons was always hard for me. There are probably things in this code that you would definitely say don't use that. But I can't guess what they are, if any, can you tell me?
Or how can I make it more performant and clear without being too complicated.
(Beside playerprefs i know i should not use it but it's simple and currently working enough for me)

Code (CSharp):

1. using System.Collections;
2. using System.Collections.Generic;
3. using UnityEngine;
4. using UnityEngine.UI;
5.  
6. public class GameManager : MonoBehaviour
7. {
8.     [SerializeField] private Button[] gunUseButtons;
9.     [SerializeField] private GameObject[] gunBuyButtons;
10.     [SerializeField] private Gun[] guns;
11.  
12.     private void Start()
13.     {
14.         PlayerPrefs.SetInt("BoughtWeapon" + 0, 1);
15.  
16.         for (int i = 0; i < gunUseButtons.Length; i++)
17.         {
18.             //Deactive use buttons if we didnt bought weapon
19.             if (PlayerPrefs.GetInt("BoughtWeapon" + i, 0) == 1)
20.             {
21.                 gunUseButtons[i].enabled = true;
22.                 Destroy(gunBuyButtons[i]);
23.             }
24.             else
25.             {
26.                 gunUseButtons[i].enabled = false;
27.             }
28.             //USE SAVED WEAPON
29.             UseWeapon(PlayerPrefs.GetInt("UsingWeapon", 0));
30.         }
31.     }
32.     public void BuyWeapon(int ButtonID)
33.     {
34.         float currentMoney = PlayerPrefs.GetInt("playerMoney", 0);
35.         if (guns[ButtonID].price <= currentMoney )
36.         {
37.             currentMoney -= guns[ButtonID].price;
38.             PlayerPrefs.SetFloat("playerMoney", currentMoney);
39.             BuyWeaponAction(ButtonID);
40.         }
41.     }
42.     private void BuyWeaponAction(int ButtonID)
43.     {
44.         PlayerPrefs.SetInt("BoughtWeapon" + ButtonID, 1);
45.         gunUseButtons[ButtonID].enabled = true;
46.         Destroy(gunBuyButtons[ButtonID]);
47.         UseWeapon(ButtonID);
48.  
49.     }
50.     public void UseWeapon(int ButtonID)
51.     {
52.         //Unequip ALL WEAPONS
53.         for (int i = 0; i < gunUseButtons.Length; i++)
54.         {
55.             gunUseButtons[i].interactable = true;
56.         }
57.         //Then Equip Current Weapon
58.         gunUseButtons[ButtonID].interactable = false;
59.         PlayerPrefs.SetInt("UsingWeapon", ButtonID);
60.         //We set WeaponManager Script UseWeapon in Button 2.Action no need to referance here!
61.     }
62. }
63.  

 

Well yes, I definitely wouldn't use PlayerPrefs for this sort of thing. It should only be used for low level settings only (hence the name). Consider ways to have a reference to the player's components, so that you can directly communicate with it rather than needing PlayerPrefs.

Have you considered generating the buttons on the fly based on what guns are available? There might be no need to set them up manually when you can just instantiate a Button prefab n times for whatever is available. Make a component for the button to, that can hold onto the necessary information, and you can just subscribe a method to the button's click delegate.

You also say this is a shop but the script is called 'GameManager'. It should really be named something more relevant/descriptive as to the purpose of this component.

 

Yes you are right im a bit lazy to learn new save system. I plan to learn it when i finish this game.

Something like this?
If so yes i was thinking to do like that but since there is only 7 item in the shop and i use big amount of money to buy stuff like 100M, 100K which i will need to write another script to add that K,M,B to end of the number. Instead of doing it i choose to set prices manuellly because i can add K,M,B without code and its only 7 item so 1minute job to set manuelly. Thats why i didnt make like this. Is there a performance diffrence between this and my code? If not im okey with mine.

Yes you are right i should change it to ShopManager or something like that. Thank you

 

It's not even save system stuff. It's just the core principle in Unity of understanding how to use code to interact with other objects via code. This kind of stuff doesn't need to go through a save system.

There's nothing here that requires writing more repeated code (you should seldom ever have to repeat code).

You can set prices manually. If you have an object that represent these guns (preferably a scriptable object), you can set the price in these objects. The shop can just read this value and just display it. There are lots of ways to automatically format numbers with C# too.

 

Yes in my current system it also reads prices from scriptable objects. The only diffrent i set buttons text manuel. I can make it read from scriptable object and set it auto to button text and use code to format which i already have for showing money

Code (CSharp):

1.     public void UpdateMoneyText()
2.     {
3.         float money = PlayerPrefs.GetFloat("playerMoney", 0);
4.         string[] ScoreNames = new string[] { "", "k", "M", "B", "T", "KK", "MM", "TT", "ad", "ae", "af", "ag", "ah", "ai", "aj", "ak", "al", "am", "an", "ao", "ap", "aq", "ar", "as", "at", "au", "av", "aw", "ax", "ay", "az", "ba", "bb", "bc", "bd", "be", "bf", "bg", "bh", "bi", "bj", "bk", "bl", "bm", "bn", "bo", "bp", "bq", "br", "bs", "bt", "bu", "bv", "bw", "bx", "by", "bz", };
5.         int i;
6.  
7.         for (i = 0; i < ScoreNames.Length; i++)
8.             if (money < 999)
9.                 break;
10.             else money = Mathf.Floor(money / 100f) / 10f;
11.  
12.         if (money == Mathf.Floor(money))
13.             moneyText.text = money.ToString() + ScoreNames[i];
14.         else moneyText.text = money.ToString("F1") + ScoreNames[i];
15.  
16.     }

But I don't know, I just did it because it was easier to do it that way for me.


Then im missing a big part by avoiding learning new save system(or something like that). I will prioritize it now, thanks.

 

It's not that hard, all you need is to make a few serializable classes and save to/load from json. Since you've been using PlayerPrefs you have (almost) nothing to do because you already have the logic of using int, string and bool.