Better save/load system.

[hhaslam11]

Something way cleaner. Current one is a pain to add things to.. (Use XML?)

[BrendonButler]

Not sure if you've seen my file management system or my configuration system.

But feel free to take a look at JSON or YAML. Yaml is a more simple, cleaner looking configuration type as opposed to JSON, and that's probably what I would recommend for your saving system.

If you want to assign me to this issue I'd love to help. And I'd completely understand if you would rather do this yourself!

Just lemme know which configuration system you prefer and I can set it all up for you. Even a conversion system to convert the old saves to the new system.

[hhaslam11]

@BrendonButler Sounds great! If you want,I can give you write-access to the repo so you can commit to it as you wish; getting rid of the need of forking and pull requests :)

[hhaslam11]

Just let me know ^

[BrendonButler]

Whichever way works for you. I don't mind the extra step. Probably prefer it, just in case there's something I try to merge that you would prefer done another way or something. But if you ever need help with any of the issues, just let me know.

[hhaslam11]

I don't think I can even assign you to to an issue unless you're part of the repo, so I'll just add you :) Don't worry about my preferences, I'm not really picky. Especially when it comes to saving and loading, because any way is better than my current way, honestly. Thanks a ton! I've been procrastinating fixing this for a while.

[hhaslam11]

I added you as a collaborator. You just need to accept. Commit as you wish :) And if you still prefer the whole idea of merge requests and stuff, you can always just make different branches while you work on whatever, then do a merge request back into master. Big thanks once again!

[BrendonButler]

@hhaslam11, So I've been working pretty hard on this and I'm almost done (I think). New save files and converting old save files seems to work just fine.

The only issue that I'm having is that some settings aren't saving to file (notated by a "// not saving" comment), and the ones that are, are very out of order. (They're formatted, but not put in the order they were set in).

I think the issue with the saving has to do with the way my saving method builds trees (maps inside of maps). Shouldn't be a difficult fix. And the out-of-order-ness might just be something small I'm overlooking.

Feel free to have a look at the progress: Saves.java

This is what a save file currently looks like, (even managed to keep your custom extension): Text-Fighter_Bren.TFsave

[hhaslam11]

Looks great so far! I do have one question though, in the method set(String key, Object object) (Line 720 in saves, what exactly is it setting? I'll have to look through your code more thoroughly when I have the time.

[hhaslam11]

I love how clean it is now. Even little things like:

private static boolean readBoolean(){
        return Boolean.parseBoolean(input.nextLine());
}

instead of doing Boolean.parseBoolean(input.nextLine()); on a bunch of lines repeatedly.
👍

[BrendonButler]

So basically, it sets a key and a value for the data map. The only reason that the set method is so large is that I have it so that you can have child nodes.

For example, in Text-Fighter_Bren.TFsave, to get the formatting so that "Health" is indented and directly under "User" you have to have User set to a Map of some sort. Then to access the "Health" variable. You would use the key "User.Health."

What that method does is it breaks down the key into an array. Each string in the array is called a node ["User", "Health"], then it iterates through each node in the array and checks to see if the data map contains the current node. If it does, check to see if it is a Map type, if it is, search that map for the next node. If it is not, create all the rest of the nodes and add them to the data map.

It's very simple, but very complicated at the same time. If you don't understand or have more questions, feel free to ask!

Also, the readBoolean() method was just left in from when I started editing the SaveAndLoad class. xD

private static boolean readBoolean(){
        return Boolean.parseBoolean(input.nextLine());
}

[hhaslam11]

Ah, I see!
I'll probably have more questions as a look through the code more in-depth. I always planned on reading up on all this stuff (I actually have a book on it somewhere, I think..) but never got around to it. This is all new to me! All very interesting too.

[BrendonButler]

I just made a new commit, there's a weird ArrayIndexOutOfBoundException being thrown no matter how I change that line of code on line 770. Feel free to take a look, I'm very tired, maybe I'm just overlooking something simple. I'll take a look tomorrow if you don't get a chance to.

[BrendonButler]

Also, one thing I forgot to mention is that instead of having to worry about what order you set data in. As long as the data is in the data map when the game is saving, it will be formatted and saved to the file.

To retrieve data, when the save file is being loaded, it will automatically take all the data from the .TFsave file and store it in the data map.

[hhaslam11]

Found the problem, don't know how to fix it because I still don't really understand the code.
The error is in line 171.
prevParents.get(i - 1).put(nodes[i], prevParents.get(i));

the size of nodes is only 2 (so the only valid options are nodes[0] and nodes[1]. However, i is 2, trying to do nodes[2], causing the outofbounds exception.

Here's some debug screenshots if it helps:

I need to sleep now. :)

[BrendonButler]

I deleted the whole "else" block that contained that so that I can restart from scratch. The block above that works fine. It will save anything that hasnt been saved yet.

[BrendonButler]

I'm pretty certain that I'm almost done.

Saving:
There are a few booleans and integers that wont save, I'm guessing that it has to do with the data either not being set at runtime or something like that.

Loading:
Whenever loading a save game it reverts the player back to level 0 because the "Stats.XP.Level" key isn't saved in the file for some reason.

Converting:
Converting seems to be working just fine. Once in a while you will get a "No Line Found" error. Seems to occur 25-50% of the time you try to convert a file. I made another change, and haven't done much testing after that, this issue may already be resolved.

If you would like, I can create a pull request and we can both try to figure out the remaining issues. Or I can finish it on my own and create a pull request when it's done. It would just be a few more days before it's finished. Unless I run into another issue that I can't wrap my brain around. xD

[hhaslam11]

sure, go ahead and create a pull request, thanks! I'll merge it on a separate branch so I can continue testing other things while this is still being worked on

[hhaslam11]

What does for (int i = nodes.length - 2; i > 0; i--) { do? More specifically, why the - 2 from nodes.length?

[hhaslam11]

Also, I'm "fixing" things I assume are typos, but I could be wrong because I still don't really understand the code. So I might be breaking more than I'm fixing........

[BrendonButler]

Feel free to change any of the keys (ex: "Stats.XP" could be changed to "User.XP" or just "XP", really anything you want. Just make sure that if you change it in one location, you change it in all locations. So just do a search for the key and replace all instances of it. (Should be 2-3 max).

Also, feel free to change the order of the "set()" method calls. That way you can organize them and group them however you like.

[BrendonButler]

Basically what that loop is doing, is looping through each node and setting the previous parent to the current parent (example below).

The reason it is -2:
If you have a key that's ("first.second.third"), when you split that into an array of nodes it would be ["first", "second", "third"]. Which has a ".length" of 3, arrays are base zero so "first" = 0, "second" = 1, and "third" = 2.

To access "third" you would have to subtract 1 from the length nodes[i - 1]. This is already done for us in the loop itself, so we can just call nodes[i].

Okay, so why -2 instead of -1? To set the "third" node, since each node is a map, you would access the "second" node and put the "third" node in it. nodes[i-1].put(nodes[i], mapObject);

If that doesnt make sense I will explain later. I'm at work right now. But feel free to keep leaving questions and I will answer when I get out! :)

[hhaslam11]

Alright, almost a week later I'm coming back to this haha...
I still don't really understand the -2 part though..
Specifically;

Okay, so why -2 instead of -1? To set the "third" node, since each node is a map, you would access the > "second" node and put the "third" node in it. nodes[i-1].put(nodes[i], mapObject);

For now though, I'm going to do usually helps debug. Copy the class in an entire new project, and minify it so im only working with ~5 variables instead of... a lot.

[hhaslam11]

There are a few booleans and integers that wont save, I'm guessing that it has to do with the data
either not being set at runtime or something like that.

not being set at runtime seems to be unrelated.
Even doing

//Health
set("User.Health", "1");
set("User.Max_Health", "2");
set("User.FirstAid.Owns", "3");
set("Stats.FirstAid.Used", "4");
set("User.InstaHealth.Owns", "5");
set("Stats.InstaHealth.Used", "6"); 
set("Stats.TimesDied", "7");

and printing them out

System.out.println(getInteger("User.Health") + getInteger("User.Max_Health"));
System.out.println(getInteger("User.FirstAid.Owns"));
System.out.println(getInteger("Stats.FirstAid.Used"));
System.out.println(getInteger("User.InstaHealth.Owns"));
System.out.println(getInteger("Stats.InstaHealth.Used"));
System.out.println(getInteger("Stats.TimesDied"));

results in

3
-1
4
-1
-1
7

I can't see any patterns with the variables not being saved.. Maybe I'm overlooking something, because there must be something similar about the three thats not working.

[hhaslam11]

Derped that up with System.out.println(getInteger("User.Health") + getInteger("User.Max_Health"));

Let's try this again.

set("User.Health", 1);
set("User.Max_Health", 2); 
set("User.FirstAid.Owns", 3);//not working
set("Stats.FirstAid.Used", 4);
set("User.InstaHealth.Owns", 5); //not working
set("Stats.InstaHealth.Used", 6); // not saving
set("Stats.TimesDied", 7);

System.out.println(getInteger("User.Health"));
System.out.println(getInteger("User.Max_Health"));
System.out.println(getInteger("User.FirstAid.Owns"));
System.out.println(getInteger("Stats.FirstAid.Used"));
System.out.println(getInteger("User.InstaHealth.Owns"));
System.out.println(getInteger("Stats.InstaHealth.Used"));
System.out.println(getInteger("Stats.TimesDied"));

1
2
-1
4
-1
-1
7

[BrendonButler]

Hopefully this makes sense, but this is the really messy breakdown of the -2 reasoning. Now that I look at it this way it seems like there may be flaws. Might be why the data isnt being set properly for those few keys.. Not sure why it works for all the others though..

[BrendonButler]

@hhaslam11 try printing out the data map. should be something like; System.out.println(data.toString()); and add that to your last comment.

[hhaslam11]

output was

1
2
-1
4
-1
-1
7
{User={Health=1, Max_Health=2}, Stats={FirstAid={Used=4}, TimesDied=7}}

[BrendonButler]

The data doesn't exist in the map and that's why it's returning -1 when you print the value of it. I think I know my mistake, but I could be dead wrong. Will do some testing.

[hhaslam11]

It has something to do with having 3 nodes... thats the only times it wont save. But sometimes it will save with 3.. always works with two though...
Once i figure out the pattern of the not-saving-ness , it'll be easier to fix :)

[hhaslam11]

Also thanks for the diagram, makes sense now

[BrendonButler]

Now that I look at it this way it seems like there may be flaws.

That was exactly what I was talking about xD The three nodes must be the issue.

[hhaslam11]

https://gist.github.com/hhaslam11/a30845a543ea1e0abf7e411029b5c033
Here's my file im testing with

[hhaslam11]

Think i found it.

if node[0] has been used before, it wont work.
For example,
if you change set("User.FirstAid.Owns", 3); to set("Test.FirstAid.Owns", 3); itll work.
or if you change

set("User.Health", 1);
set("User.Max_Health", 2);

to

set("Test.Health", 1);
set("Test.Max_Health", 2);

then set("User.FirstAid.Owns", 3); will work

If that makes sense.....

[BrendonButler]

That means that the issue lay somewhere in this else block.

[hhaslam11]

So first thing i did was look for common mistakes.
one by one, changed all > to >= and < to <=, etc.
interestingly enough, changing for (int i = prevNodes.length - 1; i > 1; i--) to for (int i = prevNodes.length - 1; i >= 1; i--) resulted in..

1
2
3
4
5
6
7
{User={Health=1, Max_Health=2, FirstAid={Owns=3}, InstaHealth={Owns=5}}, Stats={FirstAid={Used=4}, InstaHealth={Used=6}, TimesDied=7}}

Now to confirm this was actually the problem..

[hhaslam11]

Seems to work great on TextFighter! Guess it was that simple all along...

[BrendonButler]

AHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHh

[hhaslam11]

Hmm.. It just crashed when loading, maybe it didnt fix anything (or theres something else wrong)

[BrendonButler]

Crashed when loading the save file?

[hhaslam11]

Everything seems to be in the save file though, which wasnt happening before.

[BrendonButler]

Stacktrace?

[hhaslam11]

Yeah, when loading the save file

You've leveled up! You are now level 0!
You have been rewarded 100 coins!


Exception in thread "main" java.lang.NullPointerException
    at com.hotmail.kalebmarc.textfighter.player.Settings.setConstants(Settings.java:156)
    at com.hotmail.kalebmarc.textfighter.player.Settings.setDif(Settings.java:60)
    at com.hotmail.kalebmarc.textfighter.main.Saves.load(Saves.java:136)
    at com.hotmail.kalebmarc.textfighter.main.Saves.savesPrompt(Saves.java:71)
    at com.hotmail.kalebmarc.textfighter.main.Game.start(Game.java:71)
    at com.hotmail.kalebmarc.textfighter.main.Menu.load(Menu.java:31)
    at com.hotmail.kalebmarc.textfighter.main.Start.main(Start.java:19)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)

[BrendonButler]

did you remember to replace all the data you exchanged for this?

set("User.Health", 1);
set("User.Max_Health", 2);
set("User.FirstAid.Owns", 3);//not working
set("Stats.FirstAid.Used", 4);
set("User.InstaHealth.Owns", 5); //not working
set("Stats.InstaHealth.Used", 6); // not saving
set("Stats.TimesDied", 7);

[hhaslam11]

That was all in another project i copy-and-pasted. at this point the only thing i modified in the original file was the >=

[BrendonButler]

Looks to me like "Settings.Difficulty.Level", "Settings.Cheats.Enabled" are not being saved to the file.

Maybe the ">=" fixed some things, but not everything?

I'm too tired to figure out this crazy issue xD I'm gonna get some sleep. Hopefully it's something simple!

[hhaslam11]

Yeah, thats what I'm thinking.. changing Settings.Difficulty.Level to Settings.Dif seemed to work. But that doesnt really solve the problem, just avoids it (Which is just going to cause more problems in the long-run)

[hhaslam11]

back to the test file ;)

        //Settings
        set("Settings.Difficulty.Level", 1);// not saving
        set("Settings.Difficulty.Locked", 2);
        set("Settings.Cheats.Enabled", 3); // not saving
        set("Settings.Cheats.Locked", 4);
        set("Settings.GUI.Enabled", 5);

        System.out.println(getInteger("Settings.Difficulty.Level"));
        System.out.println(getInteger("Settings.Difficulty.Locked"));
        System.out.println(getInteger("Settings.Cheats.Enabled"));
        System.out.println(getInteger("Settings.Cheats.Locked"));
        System.out.println(getInteger("Settings.GUI.Enabled"));
        System.out.println(data.toString());

-1
2
-1
4
5
{Settings={Difficulty={Locked=2}, Cheats={Locked=4}, GUI={Enabled=5}}}

[hhaslam11]

I'm going to get some sleep and continue in the morning (unless you fix it tonight).
Getting so close!
I'll commit the change and update https://gist.github.com/hhaslam11/a30845a543ea1e0abf7e411029b5c033

[hhaslam11]

I'm resorting to stackoverflow.com
I'm sure I'd be able to fix the problem eventually, but it'll just be a lot easier asking on stackoverflow.

http://stackoverflow.com/questions/38946672/java-not-saving-all-data

[hhaslam11]

So, just finishing up. Everything seems to be working okay finally (Said that before.). I noticed one comment though, and was wondering about it

TODO: make a version checker that checks each part of a version ex: 1.4.1DEV
then determine whether or not it's older, current or newer.

Line 578

Was wonder what exactly would be the purpose of doing this?

[BrendonButler]

It was just an idea I had to check whether or not save file is newer, current or older.

And I did notice one thing with the new set method that guy had. And that's that it doesnt allow you to have multiple nodes that branch off eachother.

"First.Second.Third"
"First.Second.Fourth"

If you delete your TFSave file and try saving all the data, it wont save any branches. (At least in my testing. I was working onna fix, but had to go to work) Maybe take a quick test to see if it is or isnt workijg for you.

[hhaslam11]

Not sure what you mean..Seems to work fine for me

set("First.Second.Third", 1);
set("First.Second.Forth", 2);

test.txt

First:
  Second:
    Third: 1
    Forth: 2

Maybe I'm misunderstanding what you meant..

[BrendonButler]

Mis-clicked, whoops. I dunno. Nevermind then. I copied and pasted his new set method into ModestAPI and did some testing and I didnt get that result :/ will have to do more testing!

[hhaslam11]

Seems to work fine on TextFighter too (Even after deleting my TFSave)