Some values not saving into PlayerPrefs

Still working on my Crosshair customizer, heres the problem:
I made two functions which change the color of the crosshair

    public void SliderColorChange()
    {
        PlayerPrefs.SetFloat("crosshairred", RedSlider.value);
        PlayerPrefs.SetFloat("crosshairgreen", GreenSlider.value);
        PlayerPrefs.SetFloat("crosshairblue", BlueSlider.value);

        Color crosshaircolor = new Color(RedSlider.value, GreenSlider.value, BlueSlider.value);

        RedInput.text = (RedSlider.value * 255).ToString();
        GreenInput.text = (GreenSlider.value * 255).ToString();
        BlueInput.text = (BlueSlider.value * 255).ToString();

        upper.color = crosshaircolor;
        lower.color = crosshaircolor;
        left.color = crosshaircolor;
        right.color = crosshaircolor;
    }

    public void InputColorChange()
    {
        PlayerPrefs.SetFloat("crosshairred", float.Parse(RedInput.text) / 255);
        PlayerPrefs.SetFloat("crosshairgreen", float.Parse(GreenInput.text) / 255);
        PlayerPrefs.SetFloat("crosshairblue", float.Parse(BlueInput.text) / 255);

        Color crosshaircolor = new Color(float.Parse(RedInput.text) / 255, float.Parse(GreenInput.text) / 255, float.Parse(BlueInput.text) / 255);

        RedSlider.value = float.Parse(RedInput.text) / 255;
        GreenSlider.value = float.Parse(GreenInput.text) / 255;
        BlueSlider.value = float.Parse(BlueInput.text) / 255;

        upper.color = crosshaircolor;
        lower.color = crosshaircolor;
        left.color = crosshaircolor;
        right.color = crosshaircolor;
    }

The first one uses sliders to change the color, of which I have three (for every color channel) using this exact function. The second one uses Input fields. Now, whenever I use the sliders or the Input fields, the color of the crosshair does change, but the green and blue values don’t save into PlayerPrefs (when I read it after exiting and reentering play mode both of them are zero, even though I changed them to another value), however, the value for the red channel does save.

Anyone know what the issue could be? Should I maybe split up both functions into three different ones to change every color channel with a seperate function?

I think you should look at the code that that reads the player pref values, As you’re using magic strings (bad practice) here, a simple typo in the code that reads these player prefs could be the issue.

At the very least use a constant string so you aren’t at risk of a simple typo causing the issue:

public const string CROSSHAIR_RED_KEY = "crosshairred";

// etc etc

That said, once you have player prefs working, I would immediately get out of the habit of using player prefs and learn to serialize data, and write/read it from disk.

A lot of this code could be boiled down into a single serializable class, alongside using JsonUtility to serialize it, before writing it to disk, and then reading it later: Unity - Scripting API: JsonUtility.ToJson

Right now I’m using PlayerPrefs for quite a lot of things, like saving the sensitivity and the other values of the crosshair like width and length of the lines; do you think I should generally avoid using PlayerPrefs or are there some cases where it can be better than saving to and reading from disk?

Edit: Still not working with constant strings

I mean as you’re discovering player prefs can be a bit of a pain to debug. Whereas just writing a simple text file to disk is painfully easy to debug. The file is either there or it isn’t, and the data is either in the file, or it isn’t!

In any case it’s hard to say without seeing the code that reads the player prefs. You should also debug whether the values going into player prefs are valid/what you expect them to be. Perhaps use Registry Editor to see if the values saved are what you expect as well.

Also worth pointing out those two methods are effectively doing the same thing and repeating a lot of code. Repeated code == more chance for bugs! A good way to avoid bugs is to identify where you’re repeating the same thing over and over, and change your code to avoid that.

For example you don’t need to parse the same text dozens of times. Simply the one time:

public const string CROSSHAIR_RED_KEY = "crosshairred";
public const string CROSSHAIR_BLUE_KEY = "crosshairblue";
public const string CROSSHAIR_GREEN_KEY = "crosshairgreen";

public void SliderColorChange()
{
	float r = RedSlider.value;
	float g = GreenSlider.value;
	float b = BlueSlider.value;
	
	RedInput.text = (r * 255).ToString();
	GreenInput.text = (g * 255).ToString();
	BlueInput.text = (b * 255).ToString();
	
	UpdateCrossHairColor(r, g, b);
}

public void InputColorChange()
{
	float r = float.Parse(RedInput.text) / 255;
	float g = float.Parse(GreenInput.text) / 255;
	float b = float.Parse(BlueInput.text) / 255;
	
	RedSlider.value = r;
	GreenSlider.value = g;
	BlueSlider.value = b;
	
	UpdateCrossHairColor(r, g, b);
}

public void UpdateCrossHairColor(float r, float g, float b)
{
	PlayerPrefs.SetFloat(CROSSHAIR_RED_KEY, r);
	PlayerPrefs.SetFloat(CROSSHAIR_GREEN_KEY, g);
	PlayerPrefs.SetFloat(CROSSHAIR_RED_KEY, b);

	Color crosshaircolor = new Color(r, g, b, 1.0f);	

	upper.color = crosshaircolor;
	lower.color = crosshaircolor;
	left.color = crosshaircolor;
	right.color = crosshaircolor;
}

But you could simply have a serializable class that stores all your settings and write that to disk as needed:

using UnityEngine;
using System.IO;

[System.Serializable]
public class PlayerSettings
{
	public Color CrosshairColor;
	
	public static string SavePath = Application.persistentDataPath + "/settings.txt";
	
	public void SavePlayerSettings()
	{
		string json = JsonUtility.ToJson(this, true);
		File.WriteAllText(SavePath, json);
	}
	
	public static PlayerSettings GetOrCreatePlayerSettings()
	{
		bool exists = File.Exists(SavePath);
		if (exists == false)
		{
			return new PlayerSettings();
		}
		else
		{
			string json = File.ReadAllText(SavePath);
			return JsonUtility.FromJson<PlayerSettings>();
		}
	}
}

This is a basic example to illustrate a concept rather be fully working code. Ideally you’d want to hold onto an instance of this somewhere, update the values accordingly, and only read/write it when necessary.

Just so I can be sure I understand the second script correctly; In the script I would have my floats which set color, length and width of the crosshair, and when I use the function PlayerSettings, if none have been created yet, the values get set to whatever standard I set before (like in a prefab) and if I they do exist, those floats will automatically get set to the saved values?

I didn’t quite understand most of that, sorry.

It’s a regular C# class. You can instance it anywhere, and even serialize it for use in the inspector. Though in this particular instance you would manage it with another component most likely. Probably a singleton monobehaviour component that instantiates itself when accessed, and it used as your central point for acessing and updating your settings.

So again, you have your settings object:

using System.IO;
using UnityEngine;

[System.Serializable]
public class PlayerSettingsData
{
	public float MouseSensitivity = 1.0f;

	public float CrossHairSize = 1.0f;

	public Color CrosshairColor = Color.white;

	public static string SavePath = Application.persistentDataPath + "/settings.txt";

	public void SavePlayerSettings()
	{
		string json = JsonUtility.ToJson(this, true);
		File.WriteAllText(SavePath, json);
	}

	public static PlayerSettingsData GetOrCreatePlayerSettings()
	{
		bool exists = File.Exists(SavePath);
		if (exists == false)
		{
			return new PlayerSettingsData();
		}
		else
		{
			string json = File.ReadAllText(SavePath);
			var settings = JsonUtility.FromJson<PlayerSettingsData>(json);
			settings.SavePlayerSettings();
			return settings;
		}
	}
}

Fixed a few things I missed when typing it in NotePad++. Namely it will save itself when generated for the first time.

Then you use a monobehaviour to manage it:

using UnityEngine;

public class PlayerSettings : MonoBehaviour
{
	#region Internal Members

	[System.NonSerialized]
	private PlayerSettingsData _playerSettingsData;

	private static PlayerSettings _instance;
	
	#endregion
	
	#region Properties

	private static PlayerSettings Instance
	{
		get
		{
			if (_instance == null)
			{
				_instance = CreateInstance();
			}

			return _instance;
		}
	}

	public static PlayerSettingsData PlayerSettingsData => Instance._playerSettingsData;
	
	#endregion
	
	#region Unity Callbacks
	
	private void Awake()
	{
		_playerSettingsData = PlayerSettingsData.GetOrCreatePlayerSettings();
	}

	private void OnDestroy()
	{
		_playerSettingsData.SavePlayerSettings();
		_instance = null;
	}

	#endregion

	#region Factory

	private static PlayerSettings CreateInstance()
	{
		var go = new GameObject("PlayerSettings");
		var playerSettings = go.AddComponent<PlayerSettings>();
		Object.DontDestroyOnLoad(go);
		return playerSettings;
	}

	#endregion
}

I named the component PlayerSettings and renamed the data itself to PlayerSettingsData.

And then when it comes to using it, you just access it via the PlayerSettings.PlayerSettingsData property, which causes the component to instantiate itself, gets the settings, and lets you modify them:

using UnityEngine;

public class PlayerSettingsUsageExample : MonoBehaviour
{
	[System.NonSerialized]
	private PlayerSettingsData _settingsData;

	private void Start()
	{
		_settingsData = PlayerSettings.PlayerSettingsData;
		_settingsData.MouseSensitivity = 2.0f;
	}
}

And of course I can see the data gets written to disk:

{
    "MouseSensitivity": 2.0,
    "CrossHairSize": 1.0,
    "CrosshairColor": {
        "r": 1.0,
        "g": 1.0,
        "b": 1.0,
        "a": 1.0
    }
}

You just need to ensure your systems are reading/writing from this central data.

In any case, debug your PlayerPrefs not working first before you dive into this. And if you do dive into this, play around with it in isolation, rather than trying to work it into your project before you understand how it works.

And make sure you’re using version control software before you do try to implement it into your project!

Spiney, you deserve the world

Thank you so much for helping!

1 Like

Fixed by splitting up both functions into three seperate ones, one for every color channel (I love unoptimising code)

public static string CROSSHAIR_RED_KEY = "crosshairred";
public static string CROSSHAIR_GREEN_KEY = "crosshairgreen";
public static string CROSSHAIR_BLUE_KEY  = "crosshairblue";

    public void RedSliderChange()
    {
        float r = RedSlider.value;

        PlayerPrefs.SetFloat(CROSSHAIR_RED_KEY, r);

        RedInput.text = (r * 255).ToString();

        UpdateCrosshairColor(r, GreenSlider.value, BlueSlider.value);
    }

    public void GreenSliderChange()
    {
        float g = GreenSlider.value;

        PlayerPrefs.SetFloat(CROSSHAIR_GREEN_KEY, g);

        GreenInput.text = (g * 255).ToString();

        UpdateCrosshairColor(RedSlider.value, g, BlueSlider.value);
    }

    public void BlueSliderChange()
    {
        float b = BlueSlider.value;

        PlayerPrefs.SetFloat(CROSSHAIR_BLUE_KEY, b);

        BlueInput.text = (b * 255).ToString();

        UpdateCrosshairColor(RedSlider.value, GreenSlider.value, b);
    }

    public void RedInputChange()
    {
        float r = float.Parse(RedInput.text) / 255;

        PlayerPrefs.SetFloat(CROSSHAIR_RED_KEY, r);

        RedSlider.value = r;

        UpdateCrosshairColor(r, GreenSlider.value, BlueSlider.value);
    }

    public void GreenInputChange()
    {
        float g = float.Parse(GreenInput.text) / 255;

        PlayerPrefs.SetFloat(CROSSHAIR_GREEN_KEY, g);

        GreenSlider.value = g;

        UpdateCrosshairColor(RedSlider.value, g, BlueSlider.value);
    }

    public void BlueInputChange()
    {
        float b = float.Parse(BlueInput.text) / 255;

        PlayerPrefs.SetFloat(CROSSHAIR_BLUE_KEY, b);

        BlueSlider.value = b;

        UpdateCrosshairColor(RedSlider.value, GreenSlider.value, b);
    }

    public void UpdateCrosshairColor(float r, float g, float b)
    {
        Color crosshaircolor = new Color(r, g, b, 1.0f);

        upper.color = crosshaircolor;
        lower.color = crosshaircolor;
        left.color = crosshaircolor;
        right.color = crosshaircolor;
    }

If anyone finds a more optimised solution feel free to post