Confusing up public/public static/private, and how to use transport data across save states

Unity Engine
,
1

Good day,

I do understand the documented difference. Public is unique and shared only inside the class, while public static IS shared across the project as global. Private is none of the initial two, and serves as protected “hidden” reference, right?. The problem for me is: 1. best practice, and 2. why declare a private, if no declaration in front of a reference does the same job, like so:

private Rect myRect;
Rect myRect; <-- difference to private, I fail to see...

I’m specifically asking because I get all sorts of trouble trying to save game data across sessions, utilizing a mixture of PlayerRef, and external .json procedure. To get even more into detail, I have a hard time to save float coordinates, for a gameObject(Texture2D)'s transform.position and rotation, as well as instantiated prefabs, which I would usually catch with an for(var i = 0 ; i < gameObjects.Length ; i ++) loop.

Trying to reload this data always results in broken coordinates, because I’d also need to take mouse position vs world position data into account. On a 2d game btw.

So what I’m really asking. What is the get to go recommended best practice, and most code effective way to save data between game sessions?

2

It’s not clear to me why you’re supposing these problems have anything to do with whether the data is public, private or static. If you’re having problem with your save/load script you could probably get help fixing it, if you post the code and explain more precisely what the problem is.

1 Like
3

Access Modifiers (the public/private thing) only determine how a member of a class can be accessed.

  • public - The member can be accessed outside of the class, such as from a different script.
  • private - The member can only be accessed inside the class. Other scripts cannot access it.
  • protected - The member can be accessed within the class and by any other classes than inherit from it.

static isn’t an access modifier, but it does essentially do what you described - make the member globally accessable from the project without being dependent on an object existing.

But like @kdgalla mentioned, these things don’t directly affect how code is executed.
int x = 5 will still be int x = 5 regardless if it’s public/private/protected/static/etc.

1 Like
4

In Unity,
private int myInt = 5;
is the same as
int myInt = 5;
“private” is for readability only and is not required. “public”, “protected”, and “static” are not assumed and must be included as needed.

There’s also
[SerializeField]
private int myInt = 5;
This makes a private int (can’t be accessed by other classes), but will show up in the Inspector for fiddling purposes :slight_smile:

5

Thanks for the response guys.

Hmm, so in return it’s best practice to not use any access modifiers, unless you really need them, to save code bloat?

I’m afraid that is not a got idea, posting thousands of lines of code, that cross-connect in between multiple scripts. I was asking if maybe the wrong usage of access modifiers, is causing the issue. But I deem not. I’m trying my luck with coroutine and/or lateUpdate now.

But if you really feel like breaking your head, here you go:

    void logManager(int windowID)
    { 
        toggleLogArea = GUI.Toggle(new Rect(2, 2.5f, 15, 15), toggleLogArea, string.Empty);
        if (toggleLogArea) {
            logToSave = logString;
            homeToSave = homeString;
            constPosXToSave = constPosXFloat;
            constPosYToSave = constPosYFloat;
            constPosZToSave = constPosZFloat;
            if (GUI.Button(new Rect(Screen.width / 2 - 850, 2.5f, 100, 21), "Save Data"))
                saveLog();
            if (GUI.Button(new Rect(Screen.width / 2 - 750, 2.5f, 100, 21), "Load Data")){
                loadConst();
                loadLog();
            }
            logRect.size = new Vector2 (1007, 845);
            logLabelString = GUI.TextField (new Rect (18, 2.5f, 85.5f, 21), logLabelString, 15);
            if(logString != null){
                logString = GUI.TextArea (new Rect (2.5f, 25, 1000.5f, 816.5f), logString, 7000);
            } else if(logString != null){
                logString = GUI.TextArea (new Rect (2.5f, 25, 1000.5f, 816.5f), "", 7000);
            }
        } else {
            logRect.size = new Vector2 (35, 27);
            //GUI.DragWindow(new Rect(15, 0, 92, 27));//dragable window, but removed it for release to prevent bugs
        }
    }
 
    public static void saveLog()
    {
        if(toggleHome == true){
            homeString = main.homeStar.GetComponent<system> ().name;
        }
        if(toggleHome == false){
            homeString = null;
        }
        if(toggleHome == true && main.homeStar.GetComponent<system> ().name == "Sol"){
            homeString = "Sol";
        }
        constPosXFloat = constPos.x;
        constPosYFloat = constPos.y;
        constPosZFloat = constPos.z;
        PlayerPrefs.SetString("SavedLog", logString);
        PlayerPrefs.SetString("SavedHome", homeString);
        PlayerPrefs.SetFloat("ConstPosX", constPosXFloat);
        PlayerPrefs.SetFloat("ConstPosY", constPosYFloat);
        PlayerPrefs.SetFloat("ConstPosZ", constPosZFloat);
        PlayerPrefs.Save();
        Log("Data saved!");
    }
    public static void loadConst()
    {
        constPosXToSave = PlayerPrefs.GetFloat("ConstPosX");
        constPosYToSave = PlayerPrefs.GetFloat("ConstPosY");
        constPosZToSave = PlayerPrefs.GetFloat("ConstPosZ");
        constPos.x = constPosXFloat;
        constPos.y = constPosYFloat;
        constPos.z = constPosZFloat;
        constPosXFloat = constPosXToSave;
        constPosYFloat = constPosYToSave;
        constPosZFloat = constPosZToSave;
    }
//done the extra loadConst blog, since it would mess up with isnide the original load block
    public static void loadLog()
    {
        SetCursorPos(Screen.width/2, Screen.height/2);
        if (PlayerPrefs.HasKey("SavedLog"))
        {
            logToSave = PlayerPrefs.GetString("SavedLog");
            homeToSave = PlayerPrefs.GetString("SavedHome");
            logString = logToSave;
            GameObject[] star = GameObject.FindGameObjectsWithTag ("star_system");
            for (var i = 0; i < star.Length; i ++) {
                if(star[i] && homeToSave != null){
                    if(star[i].name == homeToSave){
                        homeStar = star[i];
                    } else if(star[i].name != homeToSave){
                        return;
                    }
                    homeStar.gameObject.GetComponent<system>().select(true);
                    homeStar.gameObject.GetComponent<system>().select(false);
                    MouseEvent(main.MouseEventFlags.RightUp | main.MouseEventFlags.RightDown);
                    system.loadSelection = false;
                    homeStar.name = homeString;
                } else if(!star[i]){
                    Log ("No Home Star set!");
                }
            }
            homeString = homeToSave;
            return;
        }
        else
            Log("There is no log data!");
    }

The coordinate problem might happen due to an extremely weird Camera.main setup.

I’m working on an inherited Editor for Stellaris, made with 4.7.1(which I have to stay with, since updating to latest Unity would be a devastating pita). I’m using two cameras, one as top down 2D on a 3D setup -don’t ask me why, the org creator did it for a reason? I don’t know-, and one camera as static background projector for a background sprite. I would utilize code like this, to get correct coordinates for objects placed in the game world:

            mousePos.x = Input.mousePosition.x;
            mousePos.y = cam.pixelHeight - Input.mousePosition.y; //cam.pixelHeight is used to make mouse position 0,0 at the top left corner
            mousePoint = cam.ScreenToWorldPoint(new Vector3(mousePos.x, mousePos.y, 460)); //default 3rd variable ---> cam.nearClipPlane; 460 is used for correct coordinates, see upside!

The problem is, it can’t be utilized when trying to save coordinate data from a dragable Texture2d(the one with the messed up coordinate save date), since I would need dynamic position/rotation code, for an object placed in the game world, moved by a mouse pointer which I catch with a Physics.Raycaster, and then needs to be transformed to that weird 2D/3D setup. So while I get the exact raycaster data on the mouse pointer, I can never exactly pinpoint the coordinate data, for a dragged object after repositioning/rerotating it. And yes, I was using euler, delta, event strategys a lot. None of them worked.

I verified I save/load the correct Data via PlayerPrefs, but the Texture2D object will never end up in the position it gets feeded with, unless I didn’t rotated it.

The Camera.main is “sliding” on a CameraAxe, which uses the middle of the game map as anchor point, and it is rotated by x:90, y:180, z:0. It also sits on z:460, hence why you see the number 460 in any of my position definition code.

Ain’t it complicated? Happy new year…:slight_smile:

6

I still don’t understand what it is that your saving. Is it a position of an object that you placing at runtime? Is it a Unity gameObject?

7

It’s an external .png imported at runtime, so users are allowed to define their own background image. I would save the coordinates of the, at runtime created sprite, that houses the Texture2D.

8

Do you actually need to save the position? Is it not something you can recreate? How do you determine the position to begin with?

Is the runtime-created-sprite a GameObject? Can you show the code where you retrieve the position from the objects transform and where you restore the loaded position to the objects transform?

9

That’s what I do. It gets messed up whenever I try to give it a custom position, but only when saving. It works perfectly as long as I stay inside the game, even when saving/reloading. Once I exit the game, and return, the position is not were it is supposed to be, despite Debug.Log perfectly gives me the correct saved position data. It just doesn’t apply it correctly, and I don’t know why?

Code to apply constellation sprite:

    void applyConstellations()
    {
        if (constellationSprite.gameObject.GetComponent<SpriteRenderer>().enabled == false)
        {
            constellationSprite.gameObject.GetComponent<SpriteRenderer>().enabled = true;
            texConst = LoadBackgroundPNG("imageConst.png");
            Sprite sprite = Sprite.Create(texConst, new Rect(0, 0, texConst.width, texConst.height), Vector2.zero);
            constellationSprite.sprite = sprite;
            if(homeStar != null && homeStar.GetComponent<system>().systemName == "Sol"){
                constellationSprite.gameObject.GetComponent<RectTransform>().position = new Vector3(541+homeStar.transform.position.x, homeStar.transform.position.y, 479+homeStar.transform.position.z);
                //Log (homeStar.transform.position.ToString());
            } else if(homeStar == null){
                constellationSprite.gameObject.GetComponent<RectTransform>().position = new Vector3(500, 0, 500);
            }
            Log ("Constellations Applied");
        }
      
        else if (constellationSprite.gameObject.GetComponent<SpriteRenderer>().enabled == true)
        {
            constPos = constellationSprite.gameObject.GetComponent<RectTransform>().position = new Vector3(constPos.x, constPos.y, constPos.z);
            constellationSprite.gameObject.GetComponent<SpriteRenderer>().enabled = false;
            DestroyImmediate(texConst);
            Log ("Constellations Removed");
        }
    }

And for the drag logic:

//in OnGUI()
        //dragBox overlay for Constellation Manager
        dragBox = new Rect (0, 0, Screen.width, Screen.height);
        constellationManager ();

//drag, position, rotation, zooming logic etc.
    void constellationManager(){
        if (Input.GetKey (KeyCode.F10)) {
            currentMode = 8;
            //GUI.Box (new Rect (dragBoxX, dragBoxY, Screen.width, Screen.height), string.Empty);// visual indicator for debugging
            var e = Event.current;
            if (e.type == EventType.MouseDrag && dragBox.Contains (e.mousePosition) && Input.GetKey (KeyCode.Mouse0)) {
                dragBox.x += e.delta.x;//makes sure the dragBox stays in position
                dragBox.y += e.delta.y;
                constellationSprite.transform.position += new Vector3 (-e.delta.x, 0, e.delta.y);
                //dragBoxX = rect.x += e.delta.x;//makes sure the dragBox indicator stays in position
                //dragBoxY = rect.y += e.delta.y;
            }
            if (e.type == EventType.ScrollWheel && dragBox.Contains (e.mousePosition) && Input.GetKey (KeyCode.Mouse1)) {
                if (Input.GetAxis ("Mouse ScrollWheel") < 0) {
                    dragBox.x += e.delta.x;//makes sure the dragBox stays in position
                    dragBox.y += e.delta.y;
                    if (constellationSprite.transform.position.y <= -1) {
                        constellationSprite.transform.position += new Vector3 (0, 5.0f + moveInc, 0);
                    }
                }
                if (Input.GetAxis ("Mouse ScrollWheel") > 0) {
                    dragBox.x += e.delta.x;//makes sure the dragBox stays in position
                    dragBox.y += e.delta.y;
                    if (constellationSprite.transform.position.y >= -530) {
                        constellationSprite.transform.position += new Vector3 (0, -5.0f - moveInc, 0);
                    }
                }
            }
            if (e.type == EventType.ScrollWheel && dragBox.Contains (e.mousePosition) && !Input.GetKey (KeyCode.Mouse1)) {
                if (Input.GetAxis ("Mouse ScrollWheel") < 0) {
                    float distance_to_screen = Camera.main.WorldToScreenPoint (transform.position).z;
                    transform.position = Camera.main.ScreenToWorldPoint (new Vector3 (Input.mousePosition.x, Input.mousePosition.y, distance_to_screen));
                    constellationSprite.transform.RotateAround (transform.position, Vector3.up, Time.deltaTime * 90f + moveInc);
                }
                if (Input.GetAxis ("Mouse ScrollWheel") > 0) {  
                    screenPoint = Camera.main.WorldToScreenPoint (transform.position);
                    offsetPos = transform.position - Camera.main.ScreenToWorldPoint (new Vector3 (Input.mousePosition.x, Input.mousePosition.y, screenPoint.z));
                    constellationSprite.transform.RotateAround (transform.position - offsetPos, Vector3.down, Time.deltaTime * 90f + moveInc);
                }
            }
        } else if(Input.GetKeyUp (KeyCode.F10)){
            currentMode = 0;
            constPos = constellationSprite.transform.position - offsetPos;
        }
    }
10

It’s not about “bloat”, it’s simply setting the level of access. Use whatever level you need. In my experience public and private are common, static is more rare but certainly used in a number of cases (maintaining values across scenes or creating utility methods accessible anywhere), and protected is quite rare, as you’d be making a class that inherits from another class—more advanced and often not needed for many projects.

11

So position of homeStar is what you’re saving and loading? Is that also the value that’s incorrect? I don’t see how this code connects with the code that you posted earlier. Earlier you are loading the position to something called constPos and constPosXFloat, etc.

12

No, the position of the overlayed constellation image is what I’m saving.

13

Well, one thing that jumps-out to me in the code you posted earlier:

    public static void loadConst()
    {
        constPosXToSave = PlayerPrefs.GetFloat("ConstPosX");
        constPosYToSave = PlayerPrefs.GetFloat("ConstPosY");
        constPosZToSave = PlayerPrefs.GetFloat("ConstPosZ");
        constPos.x = constPosXFloat;
        constPos.y = constPosYFloat;
        constPos.z = constPosZFloat;
        constPosXFloat = constPosXToSave;
        constPosYFloat = constPosYToSave;
        constPosZFloat = constPosZToSave;
    }

Notice that you’re not actually setting the values of constPos to values that you’re loading from PlayerPrefs. Are you sure you didn’t mean to write:

constPos.x = constPosXToSave;

etc?

14

Thanks for your continous patience. I did what you proposed in any constellation possible, just to exclude the options of something being fishy about the code execution sequence. But no luck for me. I got that PlayerRef code from here.

I have to be honest, the usage of multiple floats replacing each other was confusing to me in the beginning as well, but it turned out it actually’s to happen or won’t work at all. This is where the “PlayerRef manager” starts in OnGUI():

        if (toggleLogArea && !isModKey) {
            logRect = GUI.Window (0, logRect, logManager, "");//prevents double text in write mode
        } else if (!toggleLogArea && !isModKey) {
            logRect = GUI.Window (0, logRect, logManager, "");
            GUILayout.BeginArea(new Rect (logRect.position.x+35, logRect.position.y+2.5f, 85.5f, 21));
            GUILayout.Label (new GUIContent(logLabelString));
            if(/*Input.GetKey(KeyCode.Mouse1)*/Input.GetButtonDown("Unselect") && GUILayoutUtility.GetLastRect ().Contains (Event.current.mousePosition))
            {
                logRect.position = new Vector2 (430, 40);
            }
            GUILayout.EndArea();
        }

    void logManager(int windowID)
    {
        toggleLogArea = GUI.Toggle(new Rect(2, 2.5f, 15, 15), toggleLogArea, string.Empty);
        if (toggleLogArea) {
            logToSave = logString;
            homeToSave = homeString;
            constPosXToSave = constPosXFloat;
            constPosYToSave = constPosYFloat;
            constPosZToSave = constPosZFloat;
            if (GUI.Button(new Rect(Screen.width / 2 - 850, 2.5f, 100, 21), "Save Data"))
                saveLog();
            if (GUI.Button(new Rect(Screen.width / 2 - 750, 2.5f, 100, 21), "Load Data")){
                loadConst();
                loadLog();
            }
            logRect.size = new Vector2 (1007, 845);
            logLabelString = GUI.TextField (new Rect (18, 2.5f, 85.5f, 21), logLabelString, 15);
            if(logString != null){
                logString = GUI.TextArea (new Rect (2.5f, 25, 1000.5f, 816.5f), logString, 7000);
            } else if(logString != null){
                logString = GUI.TextArea (new Rect (2.5f, 25, 1000.5f, 816.5f), "", 7000);
            }
        } else {
            logRect.size = new Vector2 (35, 27);
            //GUI.DragWindow(new Rect(15, 0, 92, 27));//dragable window, but removed it for release to prevent bugs
        }
    }

This is the save data:

    public static void saveLog()
    {
        if(toggleHome == true){
            homeString = main.homeStar.GetComponent<system> ().name;
        }
        if(toggleHome == false){
            homeString = null;
        }
        if(toggleHome == true && main.homeStar.GetComponent<system> ().name == "Sol"){
            homeString = "Sol";
        }
        constPosXFloat = constPos.x;
        constPosYFloat = constPos.y;
        constPosZFloat = constPos.z;
        PlayerPrefs.SetString("SavedLog", logString);
        PlayerPrefs.SetString("SavedHome", homeString);
        PlayerPrefs.SetFloat("ConstPosX", constPosXFloat);
        PlayerPrefs.SetFloat("ConstPosY", constPosYFloat);
        PlayerPrefs.SetFloat("ConstPosZ", constPosZFloat);
        PlayerPrefs.Save();
        Log("Data saved!");
    }

Then I’d divide the constellation load data from the rest, because I’d figure at one point, the coordinates would entirely be messed up, becaause of how OnGUI() and Update() interact. Notice that using LateUpdate() nor using a coroutine changed anything for the better:

    public static void loadConst()
    {
        constPosXToSave = PlayerPrefs.GetFloat("ConstPosX");
        constPosYToSave = PlayerPrefs.GetFloat("ConstPosY");
        constPosZToSave = PlayerPrefs.GetFloat("ConstPosZ");
        constPosXFloat = constPosXToSave;
        constPosYFloat = constPosYToSave;
        constPosZFloat = constPosZToSave;
        constPos.x = constPosXFloat;
        constPos.y = constPosYFloat;
        constPos.z = constPosZFloat;
    }

And the rest:

    public static void loadLog()
    {
        SetCursorPos(Screen.width/2, Screen.height/2);
        if (PlayerPrefs.HasKey("SavedLog"))
        {
            logToSave = PlayerPrefs.GetString("SavedLog");
            homeToSave = PlayerPrefs.GetString("SavedHome");
            logString = logToSave;
            GameObject[] star = GameObject.FindGameObjectsWithTag ("star_system");
            for (var i = 0; i < star.Length; i ++) {
                if(star[i] && homeToSave != null){
                    if(star[i].name == homeToSave){
                        homeStar = star[i];
                    } else if(star[i].name != homeToSave){
                        return;
                    }
                    homeStar.gameObject.GetComponent<system>().select(true);
                    homeStar.gameObject.GetComponent<system>().select(false);
                    MouseEvent(main.MouseEventFlags.RightUp | main.MouseEventFlags.RightDown);
                    homeStar.name = homeString;
                    system.loadSelection = false;
                } else if(!star[i]){
                    Log ("No Home Star set!");
                    return;
                }
            }
            homeString = homeToSave;
            return;
        }
        else
            Log("There is no log data!");
    }

On more detail: The whole load/save procedure is initiated from a dedicated “Project Manager” script, handling the whole PlayerRef/.Json based saving/loading. The Texture2D is imported from an external source, placed in the root folder. This is the code I’m using to toggle it on and off:

    void applyConstellations()
    {
        if (constellationSprite.gameObject.GetComponent<SpriteRenderer>().enabled == false)
        {
            constellationSprite.gameObject.GetComponent<SpriteRenderer>().enabled = true;
            texConst = LoadBackgroundPNG("imageConst.png");
            Sprite sprite = Sprite.Create(texConst, new Rect(0, 0, texConst.width, texConst.height), Vector2.zero);
            constellationSprite.sprite = sprite;
            if(homeStar != null && homeStar.GetComponent<system>().systemName == "Sol"){
                constellationSprite.gameObject.GetComponent<RectTransform>().position = new Vector3(541+homeStar.transform.position.x, homeStar.transform.position.y, 479+homeStar.transform.position.z);
                //Log (homeStar.transform.position.ToString());
            } else if(homeStar == null){
                //constellationSprite.gameObject.GetComponent<RectTransform>().position = new Vector3(500, 0, 500);
                constellationSprite.gameObject.GetComponent<RectTransform>().position = new Vector3(constPos.x, constPos.y, constPos.z);
            }
            Log ("Constellations Applied");
        }
    
        else if (constellationSprite.gameObject.GetComponent<SpriteRenderer>().enabled == true)
        {
            constPos = constellationSprite.gameObject.GetComponent<RectTransform>().position = new Vector3(constPos.x, constPos.y, constPos.z);
            constellationSprite.gameObject.GetComponent<SpriteRenderer>().enabled = false;
            DestroyImmediate(texConst);
            Log ("Constellations Removed");
        }
    }

So what happens whenever I toggle the constellation is, it will bake the data to its public static reference-which I’m using because I’d call for it from another script as well-. And when I’m actually saving the game, it will save the last baken coordinate to the PlayerRef. Staying with the current game session it all works fine and dandy. Once I exit the game and reload, the coordinates are messed up, like if it would load random coordinates any time I’m starting a new game.

15

That was going to be the next thing I wanted to bring up. There appears to be a lot of redundant an unnecessary code in there. You generally don’t want to go storing data in multiple duplicate variables because updating one set of variables will not update the others. You end-up with data that is out-of-sync. Having variables in method is ok, but duplicate members like constPosXFloat is just causing confusion. It’s not really necessary, but also the name is not descriptive. It doesn’t really tell what the purpose of the variable is.

For example, I don’t understand why loadConst

public static void loadConst()
    {
        constPosXToSave = PlayerPrefs.GetFloat("ConstPosX");
        constPosYToSave = PlayerPrefs.GetFloat("ConstPosY");
        constPosZToSave = PlayerPrefs.GetFloat("ConstPosZ");
        constPosXFloat = constPosXToSave;
        constPosYFloat = constPosYToSave;
        constPosZFloat = constPosZToSave;
        constPos.x = constPosXFloat;
        constPos.y = constPosYFloat;
        constPos.z = constPosZFloat;
    }

can’t just be:

public static void loadConst()
    {
        constPos.x = PlayerPrefs.GetFloat("ConstPosX");
        constPos.y = PlayerPrefs.GetFloat("ConstPosY");
        constPos.z = PlayerPrefs.GetFloat("ConstPosZ");
    }

What do you need constPosXFloat and constPosXToSave, etc for?