Loading Runtime Error

ID:1430250
 
Nov 25 2013, 4:27 pm
(See the best response by Nadrew.)
Code:
mob
    Login()
        ..();

        if(!usr.client.loggedIn)
        {

            usr.client.loggedIn = TRUE;
            LoadMob(usr);
            usr.UpdateVerbs();
            usr.ReloadCommunicationObjs()
            usr.UpdateInventory()
        }
    proc
        SaveMob(var/mob/m)
        {
            if(m)
            {
                var/savefile/Player = new("Savefiles/[m.ckey].sav");
                m.Write(Player);
            }
        }

        LoadMob(var/mob/m)
        {
            if(fexists("Savefiles/[m.ckey].sav"))
            {
                var/savefile/Player = new("Savefiles/[m.ckey].sav");
                m.Read(Player);
                m.AddToPlayersList();
                m.LinkToPoster();
                AdminLogin(m);
            }
            else
            {
                m << "File for [m.ckey] not found.";
                NewMob(m);
                AdminLogin(m);
            }
        }

    Write(var/savefile/Player)
    {
        if(fexists(Player))
        {
            fdel(Player);
        }
        Player["x"] << src.x;
        Player["y"] << src.y;
        Player["z"] << src.z;
        ..();
    }
    Read(var/savefile/Player)
    {
        var/x2,y2,z2;
        Player["x"] >> x2;
        Player["y"] >> y2;
        Player["z"] >> z2;
        src.Move(locate(x2, y2, z2));
        ..();
    }


Problem description: Something is happening on the second load of a character. I can make one fine, and I can log out on it. When I log back in, all of its data is there. If I log out again, that's when the problem happens. Once I log back in, the character is at 1,1,1 (characters won't spawn at this location normally) with no overlays added with its name set to my key name. This problem only occurs for me when I am hosting the game in Dream Daemon.

Any suggestions?

Nov 25 2013, 4:40 pm
Best response
You should check the savefile being generated using ExportText() to get a friendly output for it, to make sure things are actually being saved as expected. You should also probably remove the call to fdel() inside of Write(), that might be mucking things up, BYOND will automatically overwrite existing savefile data if needed.

Furthermore, you shouldn't be calling Read() and Write() directly, this can get goofy in some situations. Instead you should do something like this:

savefile["player"] << src // For saving
if(savefile["player"]) savefile["player"] >> src // For loading.


These methods call Read() and Write() respectively, but tend to be more reliable, especially by giving the data a save key ("player") to look at instead of just stuffing a mob inside of a file with no reference point.

Your problem with loading could be that you're being loaded into a new mob without properly cleaning things up, when saving and loading a whole mob like this the following is the most ideal means of loading the player:

var/mob/loaded_mob
if(savefile["player"]) savefile["player"] >> loaded_mob
if(loaded_mob) // Just in case
    var
        load_x = savefile["x"] // You could use savefile["x"] >> load_x too if you wanted.
        load_y = savefile["y"]
        load_z = savefile["z"]
    if(load_x && load_y && load_z)
        loaded_mob.Move(load_x,load_y,load_z)
    var/mob/oldmob = src
    src.client.mob = loaded_mob
    del(oldmob) // Cleaning up


This makes it possible to split your mob definitions up a bit, making life a wholllle lot easier, for instance you can use a default mob type of /mob/loading which will handle your player before they load their data, and /mob/player which will handle it after they've loaded. Making things like:

mob/loading
    Login()
        // Do your loading and all that jazz here.

mob/player
    Login()
        ..()
        world << "[src.name] has joined!"


Possible, so you can handle events differently before and after a player is loaded up without having to do a bunch of checking to see where they are in the process.
Nov 25 2013, 6:22 pm
All I can think of that would cause this to be a problem would be your hosting on Ultrasafe, or you are getting access pop-ups that require permission.

Do you receive any pop-ups asking to access savefiles?

P.S. You should follow Nadrews advice :p.

Oh, also, your fexists() statement has no effect. Although, idk why you would want to delete a savefile in the middle of accessing it, I figured I'd let you know. It should be fexists("filepath")
Nov 27 2013, 4:08 pm
I used ExportText() but it listed the x y z correctly so I am supposing that they were saved correctly.

I updated the code with most of what Nadrew posted. This is how it looks now.

SaveMob(var/mob/m)
        {
            var/savefile/f = new("Savefiles/[m.ckey]");
            f["mob"] << m;
            f["x"] << m.x;
            f["y"] << m.y;
            f["z"] << m.z;
        }

        LoadMob(var/mob/m)
        {
            var/mob/loadedMob;
            var/savefile/f = new("Savefiles/[m.ckey]");
            if(f["mob"])
            {
                f["mob"] >> loadedMob;
                var/loadX, loadY, loadZ;
                f["x"] >> loadX;
                f["y"] >> loadY;
                f["z"] >> loadZ;
                loadedMob.LoadMobAtLoc(loadX, loadY, loadZ);
                var/mob/oldMob = m;
                m.client.mob = loadedMob;
                loadedMob.AddToPlayersList();
                loadedMob.LinkToPoster();
                AdminLogin(loadedMob);
                del oldMob;
            }
            else
            {
                m << "File for [m.ckey] not found.";
                NewMob(m);
                AdminLogin(m);
            }
        }
        LoadMobAtLoc(var/lX, lY, lZ)
        {
            src.loc = locate(lX, lY, lZ);
        }


It still drops the mob at 1,1,1 and gives an error though:

BYOND Error:(Sfile.cpp,1138) failed to open file:
BYOND Error:(Sfile.cpp,1138) C:\Users\YKWN\Dropbox\Bleach Sagas- Source Shared\Bleach Sagas\Savefiles\
proc name: SaveMob (/mob/proc/SaveMob)

I don't really know how SaveMob would be called on logging in and I'm really at a loss here. It loads the mob's name and its overlays but not much else.

Any further advice?
Nov 27 2013, 4:37 pm
You need to show how you're calling LoadMob() in your code.
Nov 27 2013, 6:34 pm
 
mob
    Login()
    {
        ..();
        if(!usr.client.loggedIn)
        {
            usr.client.loggedIn = TRUE;

            global.Admins << "[usr.key] logged in.";
            for(var/obj/BanInfo/b in global.BannedPlayers)
            {
                if(usr.key == b.key || usr.client.address == b.IP || usr.client.computer_id == b.computer)
                {
                    if(!global.AdminKeys.Find(usr.key))
                    {
                        usr << "You are banned.";
                        spawn(5)
                        {
                            del usr.client.mob
                        }
                    }
                }
            }

            LoadMob(usr);
            usr.UpdateVerbs();
            usr.ReloadCommunicationObjs()
            usr.UpdateInventory()
        }
    }


Essentially the same as in the title post.
Nov 27 2013, 7:54 pm
In this instance, you actually shouldn't be calling ..() (go figure, huh?) since you're handling everything that the default would do in your own way. I imagine ..() might be causing things to execute a bit strangely.

Also, Login() is a proc, you should be using src and not usr, although Login() is technically usr-friendly, it's not a great habit to get into.

And, the stuff after LoadMob() in that Login() isn't gonna be called, you're deleting the mob associated with that Login() process inside of LoadMob() which will terminate anything left inside of Login(). You should instead be calling those procs for the mob you create inside of LoadMob().

ANNND, you misunderstood the thing about your fexists(), he didn't mean the check before loading the data, he meant the one where you were also calling fdel() which wasn't needed. You should still be checking if the file exists before trying to access it, otherwise you're gonna create a blank file instead of checking if the file exists by looking for an index inside of it.
Nov 27 2013, 7:59 pm
To add, ideally, instead of using something like client.loggedIn you should probably split your pre-loaded and post-loaded mob types up and handle their Login() separately like I mentioned in my previous post. It's a lot cleaner in the long run instead of trying to keep up with one giant Login() mess for both conditions.
Nov 29 2013, 6:33 pm
Okay, everything's working now.

Thanks for the clarifications on Login(). I wasn't sure on if usr or src was the best way to do Login. If src is the best option for avoiding strange behavior, I'll stick to that usage from now on. (The way I'm interpretting your text, it seems like you're implying to only use usr for verbs and use src for everything else. Is that correct?)

Somewhat disappointed with myself on not catching the tidbits past LoadMob() never being called. I feel like that's something I should have noticed. Apologies on that.

I dunno if the lack of fexists() was why I was getting that save message, but I put the check for it back in and it works properly now.

Thank you for all your help and thank you especially for giving me an alternative to using the client.loggedIn method. It felt pretty hacky to me. Having everything divided up into a loading and loaded class makes the code look really clean to me.