[DONE][Suggestion] Compact save file formatting

[Delian]

- Save file formatting less verbose in selected cases (functionally same, no performance gain or loss, smaller file size, easier to read by humans)

I really like the recent change of making .sav files more compact. So I'd like to suggest a few more ideas to achieve better formatting in the .sav files.

There are two types of ideas here.
The first is to not save properties that have default values. If the game knows what the default value of an object property is, then there's no need to save this property in a file.
The second is to increase readability by formatting certain very common objects/arrays into single lines.

There can be thousands of items in a battlescape game, so it would be a good idea to both format these into a single line and remove default value properties

Code: [Select]

    - id: 961
      type: STR_SMG_CLIP
      owner: 1000043
      inventoryslot: STR_QD_SLOT
      inventoryX: 0 # default value
      inventoryY: 0 # default value
      ammoqty: 36
      ammoItemSlots: ~ # default value
      tags: ~ # default valuecan be reduced to:

Code: [Select]

    - { id: 961, type: STR_SMG_CLIP, owner: 1000043, inventoryslot: STR_QD_SLOT, ammoqty: 36 }Reducing 187 to 94 bytes.

Battlescape units have a lot of default values saved and could also use some compacting

Code: [Select]

    - id: 1000043
#create a new object "genUnit", add the next 2 properties to it, and format it to a single line
      genUnitType: STR_BANDIT_RATMAN_LEADER
      genUnitArmor: BANDIT_ARMOR_P5
      faction: 1
      status: 0
      wantsToSurrender: false #default value
      isSurrendering: false #default value
      position: [24, 13, 0]
      direction: 3
      directionTurret: 3 #default value - should only be saved if it differs from direction
#create a new object "battleStats", add the next 6 properties to it, and format it to a single line
      tu: 74
      health: 36
      mana: 145
      stunlevel: 0
      energy: 70
      morale: 100
      kneeled: false #default value
      floating: false #default value
      armor: [6, 6, 6, 6, 6]
      fatalWounds: [0, 0, 0, 0, 0, 0] #default value
      fire: 0 #default value
#create a new object "experience" with the next 8 properties and format it to single line. If all the property values are 0, then we probably don't need to save this object either
      expBravery: 0
      expReactions: 0
      expFiring: 0
      expThrowing: 0
      expPsiSkill: 0
      expPsiStrength: 0
      expMana: 0
      expMelee: 0
      currStats: #format to single line
        tu: 74
        stamina: 70
        health: 36
        bravery: 80
        reactions: 109
        firing: 79
        throwing: 55
        strength: 33
        psiStrength: 67
        psiSkill: 0
        melee: 72
        mana: 145
      turretType: -1 #default value
      visible: false #default value
      turnsSinceSpotted: 255 #default value
      turnsLeftSpottedForSnipers: 0 #default value
      turnsSinceStunned: 255 #default value
      rankInt: 7
      moraleRestored: 0 #default value
      AI: #format to single line
        fromNode: 16
        toNode: -1
        AIMode: 0
        wasHitBy:
          []
      motionPoints: 0 #default value
      alreadyRespawned: false #default value
      activeHand: STR_RIGHT_HAND #default value?
      tempUnitStatistics:
        wasUnconcious: false #default value
#create new object "murderer" with the next 7 properties and format it to single line.
      killedBy: 1 #default value
      murdererId: 0 #default value
      fatalShotSide: 0 #default value
      fatalShotBodyPart: 0 #default value
      murdererWeapon: "" #default value
      murdererWeaponAmmo: "" #default value
      mindControllerID: 0 #default value
      summonedPlayerUnit: false #default value
      pickUpWeaponsMoreActively: true #default value -> it's default if it matches ai.pickUpWeaponsMoreActively
      tags:
        UNIT_RECOLOR_DESYNC: 25
to

Code: [Select]

    - id: 1000043
      genUnit: { type: STR_BANDIT_RATMAN_LEADER, armor: BANDIT_ARMOR_P5 }
      faction: 1
      status: 0
      position: [24, 13, 0]
      direction: 3
      battleStats: { tu: 74, health: 36, mana: 145, stunlevel: 0, energy: 70, morale: 100 }
      armor: [6, 6, 6, 6, 6]
      experience: { bravery: 0, reactions: 0, firing: 0, throwing: 0, psiSkill: 0, psiStrength: 0, mana: 0, melee: 0 }
      currStats: { tu: 74, stamina: 70, health: 36, bravery: 80, reactions: 109, firing: 79, throwing: 55, strength: 33, psiStrength: 67, psiSkill: 0, melee: 72, mana: 145 }
      rankInt: 7
      AI: { fromNode: 16, toNode: -1, AIMode: 0, wasHitBy: [] }
      tempUnitStatistics: {}
      murderer: {}
      tags:
        UNIT_RECOLOR_DESYNC: 25
Reducing 1595 to 758 bytes and making it a lot more readable.

Next, after playing the game for a while, the missionStatistics section of the .sav file can grow to be quite large, so object entries in this list should be formatted to a single line each.

Only slightly effective, but base facilities, transfers, research, and production list entry objects could also be formatted to a single line.
Similarly for graph data numbers of funds, maintenance, researchScores, incomes, expenditures, country funding and country/region activityXcom, activityAlien.

I think most of the .sav files will grow large due to soldier diary killLists. The list object entries are already formatted to a single line there, but I have a couple of questions regarding these entries.

Code: [Select]

killList:
  - {type: STR_ZOMBIE_STERILE_TERRORIST, rank: STR_LIVE_TERRORIST, race: STR_ZOMBIE, weapon: STR_SHOTGUN_LT, weaponAmmo: STR_SHOTGUN_SHELLS, status: 7, faction: 1, mission: 9, turn: 2703, side: 0, bodypart: 1, id: 1000004}Could the property "race" be removed? It seems to me that in most cases, race can be obtained from the unit type itself and there's no need to save it.
Could the properties "side" and "bodypart" be removed? I don't know of any mods using these for commendations. There's an idea that, oh, a soldier could get commended for how many headshots they've made, but then you realize that aiming in the game isn't possible, so any such commendation would be based on random luck.

[Yankes]

I could compress whole save to one line but then readability will suffer.
Probably formatting will stay same.

Overall chanting structure is out of question as it break compatibility between versions,
I could skip some field that have default value as long old version will handle it correctly.

[R1dO]

I am a bit curious what would be the benefit of omitting some save properties (in relation to amount of work needed and backwards compatibility).
* If your goal is reduction of save file size:
  - You'd probably better off extending existing compressed archive support (for mods) to saves.
    Compression algorithms are pretty good with text only files (expect reductions of 90% or more in file size).
* If your goal is to speed up reading/writing of those files
  - Most of the work (=time player has to wait while loading) is not so much getting the info from disk but casting it into internal data structure, and vice versa for saving.
    Default parameters won't really help in that case (one still have to check if those are overridden by the save).


As for the increase in readability:
Normally serialization (= putting as much info on a single line to save on linebreaks) benefits computers more than it does humans.
Shorter lines have the benefit of readability on smaller screens and side by side comparison of files (or sections in current file).
So this is a personal preference i suppose.

For instance my approach when editing/looking at saves is to:
* Open the file in a texteditor which has support for yaml-language and code-folding.
* Fold all
* Search for specific variable or do a manual walk over the different nodes of interest.

[krautbernd]

I am against omitting values - default or otherwise - from the save files. The benefit is at best negligible, but it would make modifying/debugging more tedious. I don't want to have to search the rul files to look up default values.

There can be thousands of items in a battlescape game[...]

What are you loading up the skyranger with "thousandths of items" for? I don't even get close to a thousandth items in a late/end game battlescape save in XCF with a fully kitted out squad.

[Delian]

I could compress whole save to one line but then readability will suffer.

I didn't ask you to do that, so I'm not sure what's your point. I only picked a few most glaring objects/lists which I feel would benefit the most in improving readability

Overall chanting structure is out of question as it break compatibility between versions,

Well, afaik, players are always told to not upgrade their versions during battlescape games. So if a player isn't in a battlescape game, then I think modifying the structure of battlegame objects isn't an issue. But as you wish...

I am a bit curious what would be the benefit of omitting some save properties (in relation to amount of work needed and backwards compatibility).

The benefits are mostly in making the files more readable. There are no problems with backwards compatibility if the existing objects aren't modified.

putting as much info on a single line to save on linebreaks
Shorter lines have the benefit of readability on smaller screens

You may be surprised to learn that I'm not advocating to put everything into a single line, but only in specific places (lists of objects) where there's a noticeable improvement in readability. For instance, in places where having one object per line doesn't make readability suffer, because the objects in question are small enough.
Shorter lines do have benefit in certain scenarios (like programming), but having only 1-2 words per line is about as detrimental as having everything in 1 line. Typography says that optimally you want 66 characters per line. If you have less than that, then readability suffers because of excessive vertical scrolling. If you have more than that, then readability suffers because you'd have to scroll horizontally.

* Open the file in a texteditor which has support for yaml-language and code-folding.
* Fold all

If you require a specific tool for opening .sav files, then you just defeated the whole point of having the file readable in the first place. In that case, even if the whole file was in a single line, you could just tell the tool to format it in a standard way.

I don't want to have to search the rul files to look up default values.
XCF

This has nothing to do with rul files. The default values are hardcoded in the game.
And I play XPZ

[Meridian]

1/ We already omit quite a lot of default values (where we see fit).
But we also keep a lot... for convenience during debugging.
I don't remember all the default values and even more importantly the list of all attributes... and a missing attribute simply makes it harder for me to spot the cause of an issue.


2/ Single lines sometimes help readability, sometimes they hurt readability. We'll be changing those that help us (us = Yankes + Meridian). We won't change the ones that hurt; or the ones we don't care about too much.

Could the property "race" be removed? It seems to me that in most cases, race can be obtained from the unit type itself and there's no need to save it.

3/ Nope, it cannot be removed. We need a hard string snapshot of everything that can be used for commendations (and diaries too actually):
- some commendations recalculate everything each time and we need to know the historical value, not the current value (mods change over time...)
- or the unit type might not even exist anymore, in which case we couldn't even fetch the current value

Could the properties "side" and "bodypart" be removed? I don't know of any mods using these for commendations.

4/ For example the standard Commendations mod for UFO uses both.


All in all, thanks for the feedback, I think we already follow these guidelines where possible, suitable and beneficial.
And maybe we'll make more such changes in the future, based on experience.

[R1dO]

...
If you require a specific tool for opening .sav files, then you just defeated the whole point of having the file readable in the first place. In that case, even if the whole file was in a single line, you could just tell the tool to format it in a standard way
...

I am not saying that it is required, a lot of times notepad suffices for my goals. It was merely there to illustrate there are other options to improve readability.

[Solarius Scorch]

Please don't "compact" anything, it serves literally no purpose (you're not going to miss a few MB).

[Yankes]

Please don't "compact" anything, it serves literally no purpose (you're not going to miss a few MB).

I think some improvements could be done, probably best rule of thumb what to change is "changing this value will break game badly".
This mean save format should allow small hacking and cheating (add more hp or remove bleed).
But moving units/items around should be forbidden and because of this format its is stored in save can be harder to use.

I will remove `inventoryX:` and `inventoryY:` if item is not in inventory slot.
Probably same with `ammoItemSlots: ~` and `tags: ~`.

[Delian]

a missing attribute simply makes it harder for me to spot the cause of an issue.

I don't quite understand this argument.
1. Based on my personal experience, showing all the values usually makes it much more difficult to debug and spot any issues because issues usually arise from values that were modified - the non-default values. So by hiding default values, you're able to easily see only the ones that are interesting.
2. Debug mode. If you want to debug in a way that shows all the values and not just the non-default ones, then that should happen when a certain debug setting is turned on. Like in the pedia article details, you have a "defaults" button to show default attributes. It's good that the button exists, because sometimes checking out the default values is useful, but usually it's more useful to filter out the non-modified attributes.

2/ Single lines sometimes help readability, sometimes they hurt readability. We'll be changing those that help us (us = Yankes + Meridian). We won't change the ones that hurt; or the ones we don't care about too much.

Well, when you're deciding on which ones help the readability, a simple guideline that I'd suggest would be:
- if you format an object/array to a single line, and in the majority of times this line doesn't exceed n characters (what n is is debatable; for programmers, most common is 80, sometimes 120), then readability of this object/array would be improved.

For instance

Code: [Select]

  position:
    - 10
    - 20
    - 0versus

Code: [Select]

  position: [10, 20, 0]In this case, the readability is improved in the 2nd case because formatting the array into a single line will never exceed 80 characters.

Please don't "compact" anything, it serves literally no purpose (you're not going to miss a few MB).

I think you misread the topic title. It's not about reducing sav file size (tho that's a byproduct), as it is about making it more readable.

[Meridian]

I don't quite understand this argument.
1. Based on my personal experience, showing all the values usually makes it much more difficult to debug and spot any issues because issues usually arise from values that were modified - the non-default values. So by hiding default values, you're able to easily see only the ones that are interesting.

The thing is, what you don't see, you (usually) don't think about.

Let's say, somebody complains that their favorite special demonic weapon doesn't work. They upload a save and say it's soldier John Smith and the weapon in their right hand. I download the save, find John Smith and quickly scan the battleunit fields.

Option 1 (defaults saved too): I go through the fields and almost at the top already see that John Smith's psi skill = 0. I vaguely remember that this demonic weapon has a damage formula directly proportional to psi skill and thus with psi skill = 0, it will always do zero damage. Case closed.

total time spent: about 30 seconds

Option 2 (defaults not saved): Since psi skill = 0 is the default value, I will not see it in the save (and not even think about it... since at this point I have no idea what I'm looking for... I just look for anything suspicious). So, first attempt failed and now I have to go brute force and:
- check out the necessary oxce version (1 minute)
- compile it (5-10 minutes)
- run it with the mod turned on (in debug mode super slow... easily 20 minutes with Piratez or XCF, just to start)
- load the save (in debug mode super slow... easily a few minutes)
- test and reload several times to figure out what's wrong (a report rarely says what's wrong, just that "it doesn't work") - let's say 5 attempts, 5 minutes each
- then set a couple of breakpoints on a few potentially suspicious places, check some results there - let's say 3 places, 5 minutes each
- I managed to find out that damage is zero, set the breakpoint there
- final step by step debugging through the entire damage calculation (10+ minutes.... there's a LOT of steps in damage calculation)
- finally I figure out that we multiply by zero (psi skill)... issue found

total time spent: 85+ minutes

[Yankes]

I make some improvements, but as for now diminish returns hits and size grain in small and majority of size is in nodes that can't be reduce more.

I for now consider this "solved" and I do not plan add any more changes to save format.

[Delian]

total time spent: 85+ minutes

That's quite a... pessimistic set of events. Sorry but, I find it too unrealistic.
Ok, so someone says a weapon doesn't work. What does that mean?
Maybe you can't attack? You trigger a weapon and it does attack.
Maybe you can't hit anything? You attack a hostile unit, and hits just fine.
Maybe you can't kill an enemy. You notice that your attacks do 0 damage (you don't need debug mode for that). So that's likely the problem.
So the first thing you'd check is what the damage is based on (arguing that your mind is blank and you don't know what you're looking for is really not the case here). You check the weapon and see the damage is based on psiSkill. For this you usually wouldn't even need to check the save file, but you'd know that what you'd be looking for. So you look at unit's stats and you don't find the psiSkill attribute, which tells you that its value is default, thus 0. Case solved.

But, let me think a bit about what could be a good example. When we don't see an attribute, we don't assume that it doesn't exist, but simply that it has a default value. Because we're smart. Well, because we know that C++ objects aren't javascript objects with dynamic properties. Heh, fuck javascript. Anyway, the problem would have to be with a default value, but at the same time in a case where we wouldn't know which property we're looking for. I... can't think of a realistic case like that. But let's assume that such a case exists. So we're looking through the object properties and don't notice anything unusual. What now? Well, if I was debugging, and I'd notice that the problem isn't in any non-default value, then through the process of elimination, I'd assume that the problem must lie in one of the default values. Great, problem solved. Of course not. The problem wouldn't be solved, and even if I could turn on display of default values here (or if they were already shown), it wouldn't help me, because seeing all the properties and their default values wouldn't easily tell me which one is the culprit. Still, if I know that the problem likes in a default value, it means that I now am "thinking about what I don't see".

Btw, I never proposed that unit stats should be hidden if default. They're a rare case where you'd want to show an attribute despite it having a default value, because I think they're very commonly searched-for attributes.

Btw², does turning on debug mode really make your game take 20min to start?

I make some improvements

Thanks a lot~

[Meridian]

Sorry but, I find it too unrealistic.

Heh, that's very nice to hear that what I've been doing almost daily for the past several years is too unrealistic.
I guess I don't exist then.
And I cannot prove to you that I exist, so... you won.
Good job.

[Delian]

It looks like I've proven myself to be an ignorant fool yet again.

I apologize, I wasn't trying to downplay your hard work. Please forget I said that.

Your debug process is quite... time-consuming to say the least. There are two off-topic questions which I'd like to ask.
How long does it take for you to start oxce+xpz normally, and how long with debugging enabled? What factor are you seeing in game performance decrease between the two?