.loadmeta garbage

[Fwiffo]

I noticed this in the 1.2.2 changelog:

+++LoadGameDialog
When a game is saved it now creates an associated metadata file - called .loadmeta alongside the .sfs. This file contains a hash of the sfs save. When the loadgame dialog opens it will compare the hash in the .loadmeta file with the hash of the savegame, and if so it will use the data in the .loadmeta file for the load dialog details instead of parsing the whole save file.

Ew.  Gross.  Really?

You take my nice clean Saves directory and clutter it up with a bunch of garbage?

Can I suggest the developers eliminate this disgusting, amateur hack and put the cached information in the sfs files themselves, near the beginning of the file?  The dialog could just read up to the end of the cached data.  That would provide the same speedup, without littering the user's folder with files that are meaningless to them.  It should be trivial to do this in a way that maintains compatibility with old/new saves.

That's my friendly and opinionated two cents.

Edited January 5, 2017 by Fwiffo

[Red Iron Crown]

People always become more receptive when you call their work disgusting and amateurish. 

[diomedea]

Let me be frank, even if what I'm saying could be less than welcome. Those comments in the OP seem to come from somebody who knows very little about IT.

The .loadmeta were introduced to avoid having the load the full sfs, those can be so many and massive loading them all to be able to present the selection of games at start brings to a very serious delay. Using a hash of those files instead is fast, and if that matches using the .loadmeta instead allows to save a lot of time and present the window to choose games a lot faster.

[suicidejunkie]

Just delete all the .loadmeta files, and you will be right back to the previous state of things with slow loading.

 

What I wonder:

Is there any particular reason the meta data needs to be a separate file, rather than being a header before the rest of the .sfs in one file?

I expect it would have been done this way to minimize the scope of the changes, and in any case it is unlikely to be worth the effort of altering at this point, but I'm just curious.

[Snark]

As a professional software engineer myself, one word of caution I'd suggest:  don't just assume that a thing is the way it is because people are stupid or lazy, or that it "obviously" should be a different way.  Software developers, as a general rule, tend to be intelligent, hard-working people, and generally have reasons for what they do.  Those reasons are often not apparent to consumers of the software.  Doesn't mean one can't offer helpful suggestions, just... try not to leap to conclusions.

I assume they had some kind of reason for doing the .loadmeta thing in a separate file, and without knowing more about what those reasons are, I'm not inclined to second-guess them too much.

One idea that does occur to me, though, that they could have done that would be less "cluttery" to the player, without (I suspect) needing much of a code change:  create a "meta" sub-folder in the saves folder, and stick all the .loadmeta files down in there.

There may be code reasons why it was simpler to put the .loadmeta in a separate file from the .sfs, but there's no reason it has to be sitting in the same folder.

7 hours ago, Red Iron Crown said:

People always become more receptive when you call their work disgusting and amateurish. 

^ Also, this. 

 

[Starman4308]

My best guess is that it makes ensuring .loadmeta validity much simpler to implement; the game checks to see if the hash of the save file matches the hash stored in the .loadmeta. If the files were combined into one, then you'd need some slightly complicated logic to ensure it only hashed the savefile contents, and not the meta-data, because otherwise the hash would never match (since you're re-writing it each time with the hash generated from a file with a modified hash generated from a file with a modified hash...).

While one could theoretically try to manage things so only the non-metadata portion of the combined savefile was hashed, it's much simpler and more maintainable coding-wise to say "hash this file and compare to string in this other file" than say "hash only certain parts of this file and compare to a string inside it"; you need to figure out how to exclude certain bytes from what you're feeding into the hashing function. Furthermore, if more metadata needs to be stored, you don't need to add another exception into your "exclude some bytes" mechanism, you just have to write it to the .loadmeta file.

It could be done, but the cost/benefit ratio of making that change is very unfavorable.

Edited January 7, 2017 by Starman4308

[Fwiffo]

Thanks everyone for chiming in.  I've been away from the forums for a bit but was amazed at how much attention my little post got, and want to take the opportunity to respond.

On 1/6/2017 at 5:41 AM, Red Iron Crown said:

People always become more receptive when you call their work disgusting and amateurish.

Point taken.

Yes, I was harsh and critical.  I just called it like I saw it.  It kind of smelled like a bolt-on to make things a little easier for a programmer (or at least, reduce code touchpoints) at the cost of making life a little more annoying for some of their users.  I could have been more tactful.  But hey, it did spark some discussion.  I'll take a free potshot from anyone offended.

I love the community here, and the fact that people jumped in to call me out and defend the guy who, granted, was just trying to make things better.

On 1/6/2017 at 7:30 AM, diomedea said:

Those comments in the OP seem to come from somebody who knows very little about IT.

There's the free potshot :-).

On 1/6/2017 at 7:30 AM, diomedea said:

The .loadmeta were introduced to avoid having the load the full sfs, those can be so many and massive loading them all to be able to present the selection of games at start brings to a very serious delay. Using a hash of those files instead is fast, and if that matches using the .loadmeta instead allows to save a lot of time and present the window to choose games a lot faster.

I get that, and I appreciate you providing some background.  I was just circumspect of the rationale that new files and hashing had to be introduced in order to achieve the performance boost.  Seems like it would be more polished (at least from a user perspective) to go with something along the lines of this:

Spoiler

PREVIEW 
{                                // Put metadata required by Load dialog here, or
    version = 1.2.2              // somewhere early in the GAME section (or even
    vesselCount = 1              // at/near end of file and read out in reverse).
    gameMode = CAREER            //
    gameNull = False             // Create a lightweight LoadPreview() function
    gameCompatible = True        // that reads just this portion and bypasses the
    funds = 1023227.4730251089   // slow reading / parsing of the rest of the file.
    science = 19                 // 
    reputationPercent = 52       // Does not require .loadmeta files or fancy
    ongoingContracts = 3         // hashing.  Backward-compatible; old KSP loads
}                                // the SFS just fine and ignores the new section.
GAME
{
    version = 1.2.2
    Title = ForFun (CAREER)
    Description = No description available.
    linkURL = 
    linkCaption = 
    Mode = CAREER
....

 

It keeps the pertinent information in the SFS where it arguably belongs, and if implemented correctly, has the handy side-effect of being slightly faster by eliminating the need to read all the bytes of the file to compute a hash.  Granted, it doesn't do much for legacy savefiles (and the existing upgrade mechanism would be of limited help there since IIRC it doesn't kick in until the player actually loads the savegame).

Despite accusations to the contrary, I do have some humble experience in this field, and can venture an educated guess (rife with assumptions!) as to how it wound up being implemented the way it did.  e.g. New dev comes on board, doesn't want to dive too deep into existing, working spaghetti code around the savefile format, maybe hits some other snags, comes up with this hack approach instead which only requires wrapping a couple functions.  Time pressure, testing constraints, other priority issues to get to, yadda, yadda.  I concede it has merits - e.g. quick, easy and low-risk to implement; works right away on existing save files; no backward compatibility concerns or impact to third-party utilities, encapsulates changes and keeps them proximate to the Load dialog.  But with some thought and care I'd bet the same ends could have been achieved minus the extra user-facing artifacts.

I realize most users don't mind or care; fair enough.  But some of us do.  That's why I spoke up.  Sorry if I hurt someone's feelings - if the person who coded it is here I'd be happy to apologize.  I don't mean to belittle their efforts, and I'm grateful that they tackled the job of massively speeding up the lethargic load dialog.

On 1/6/2017 at 10:24 AM, suicidejunkie said:

What I wonder:

Is there any particular reason the meta data needs to be a separate file, rather than being a header before the rest of the .sfs in one file?

Exactly, that's what I suggested above and in the OP.

On 1/6/2017 at 5:01 PM, Starman4308 said:

If the files were combined into one, then you'd need some slightly complicated logic to ensure it only hashed the savefile contents, and not the meta-data, because otherwise the hash would never match (since you're re-writing it each time with the hash generated from a file with a modified hash generated from a file with a modified hash...).

I'm not advocating putting the hash in the SFS file, I'm suggesting it be eliminated altogether.

(I can see where you're going about stuffing the file with a partial-coverage hash to detect dirty data, if you start by limiting yourself to making code changes only on the Load dialog side of things.  But it just seems so much simpler to write out the needed information at the time of save).

On 1/6/2017 at 1:04 PM, Snark said:

As a professional software engineer myself, one word of caution I'd suggest:  don't just assume that a thing is the way it is because people are stupid or lazy, or that it "obviously" should be a different way.  Software developers, as a general rule, tend to be intelligent, hard-working people, and generally have reasons for what they do.  Those reasons are often not apparent to consumers of the software.  Doesn't mean one can't offer helpful suggestions, just... try not to leap to conclusions.

Ah, Snark, always there with calming words of wisdom.  I hear what you're saying, and it's true: not working at Squad I'm not privy to all the factors.  As someone who's released a lot of well-groomed code, I know you have fairly high standards in terms of polish and attention to detail.

I might be overly sensitive as I’ve seen how easy it is to let a piece of software fall to patchwork when you start to "work around" existing code for fear of treading into it.  I love KSP and don't want to see that happen.  I hope the devs can appreciate my passion here.  I know they're really invested in the game (and that's part of what makes KSP awesome).

There were very likely practical challenges opaque to me.  And I'm sure many folks will say I'm nitpicking.  And maybe my inferences are ill-justified.  But none of that changes the fact the file clutter ain't pretty.  As someone who takes so much care in the pedigree of work they distribute, tell me you didn’t cringe just a little bit when you saw it?

On 1/6/2017 at 1:04 PM, Snark said:

One idea that does occur to me, though, that they could have done that would be less "cluttery" to the player, without (I suspect) needing much of a code change:  create a "meta" sub-folder in the saves folder, and stick all the .loadmeta files down in there.

That is a fantastic suggestion, and this compromise would 100% alleviate my complaints.

Should I add it as a low priority enhancement request on the bugtracker?

[xEvilReeperx]

I question the use of a hash at all in this case. Putting this information in a header at the start of the sfs would've been a lot better. When is the sfs hash going to be different than the saved one anyway, barring user intervention or corruption? There's no need to read the entirety of every sfs into memory every time MainMenu opens

Edited February 11, 2017 by xEvilReeperx

[Starman4308]

14 hours ago, xEvilReeperx said:

I question the use of a hash at all in this case. Putting this information in a header at the start of the sfs would've been a lot better. When is the sfs hash going to be different than the saved one anyway, barring user intervention or corruption? There's no need to read the entirety of every sfs into memory every time MainMenu opens

There are quite a few people who do manually edit their save files, including me. It's entirely reasonable to use a quick hash function to double-check that it hasn't been changed, so that the .loadmeta always reports valid information regardless of whether the user was mucking with it or not.

Granted, I like Snark's suggestion of putting the .loadmeta (with the hash) in a subfolder, but eliminating the hash function breaks the existing functionality of "always reports the state of the save, even if the user decided he'd rather load a file slightly different from what was saved".

[p1t1o]

Well I mean....extra files in my save folder! ...Oh noes!

Squad increases speed+efficiency for (ok "almost") no drawbacks -> gets complaints

Come on.

[Renegrade]

Sorry for the "massive" necro, but I felt it important to add this to the discussion here, as this thread is the top link in my google results about "KSP .loadmeta".

To expand on Snarks' point about reasons-not-apparent-to-the-consumer, I'd like to say it's likely that the way Unity (or the KSP codebase...or whatever is reading the save files) handles these .sfs files probably involves automatically parsing the whole thing on open in a manner that isn't easy to override.   Putting the data in a separate file could very well then be a simple addition to the existing code, rather than having to rewrite a (potentially) large percentage of the load system.

Furthermore, keep in mind that older versions of KSP won't expect the .sfs to start with a metadata stanza.   It might be possible to embed it in comments at the start, etc, but that is considered poor form as comments aren't supposed to be machine-readable...

BTW, the hash is what brought me here (it's a binary md5sum if anybody else is curious).  I wonder why they didn't use mtimes though instead of a hash.  Wouldn't have to read the whole .sfs file at all then