dfhack icon indicating copy to clipboard operation
dfhack copied to clipboard

Feature request: Make stockpiles save format human-readable/editable?

Open callym opened this issue 2 years ago • 4 comments

It'd be nice if the .dfstock files generated from saving stockpiles could somehow be human-readable/editable instead of a protobuf dump - like how the professions files that are generated are just plain text files?

callym avatar May 14 '22 07:05 callym

It probably would need a little more structure then just text lines. We'd probably be looking at using JSON, like the exported manager orders.

Protobuf is better for when we're sending information across the network, but I don't think that applies here. I think protobuf was chosen because it was available at the time the code was written and JSON wasn't.

Whereas the transition from protobuf to JSON should be relatively straightforward, there is a lot of code involved here.

We'd also need to plan a migration path so players aren't left with stored settings that they cannot read back.

For those interested in implementing this, the relevant code is in https://github.com/DFHack/dfhack/tree/develop/plugins/stockpiles

The orders code is a good model to follow for serialization/deserialization: https://github.com/DFHack/dfhack/blob/develop/plugins/orders.cpp

myk002 avatar May 14 '22 14:05 myk002

Looks like that's about right - there was some discussion on IRC about using JSON/YAML back when #433 was in development, but we settled on protobuf because we didn't have support for those.

As for why it doesn't use a "human-readable" protobuf format:

[2014-11-25 20:00:40] <lethosor> angavrilov||: do you remember if there was a reason why https://github.com/DFHack/dfhack/commit/edf77cf270e3ff7c4a08cd85fcaf537b254f6273 links to protobuf-lite instead of protobuf? (Ramblurr's plugin requires the full protobuf library, but I'm not sure if that would cause other problems.)
...
[2014-11-26 04:21:23] <angavrilov> lethosor: there is a very big reason - you can either have reload, or full protobuf
[2014-11-26 04:23:31] <angavrilov> reloading plugins
...
[2014-11-26 04:38:16] <Ramblurr> angavrilov: oh crap really?
[2014-11-26 04:38:28] <Ramblurr> herm
[2014-11-26 04:38:56] <Ramblurr> the reason my plugin depends on full protobuf is to access the text_format serialization format in addition to binary
[2014-11-26 04:39:45] <Ramblurr> text_format serialization produces human grokable settings, example stone stockpile http://hastebin.com/fojodofeku.sm
[2014-11-26 04:40:19] <Ramblurr> but if deping on full protobuf is a no go, I could just support the binary format
[2014-11-26 04:41:03] <Ramblurr> (that explains why reloading stopped working for me during the development of the plugin)
...
[2014-11-26 08:24:09] <lethosor> angavrilov: why is that?
[2014-11-26 08:24:41] <lethosor> (reload has never worked very well for me in the first place)
[2014-11-26 08:29:32] <angavrilov> because the non-lite version has some global table of all protobufs and doesn't ever remove anything from it
...
[2014-11-26 10:41:41] <expwnent> Interesting there's no way to remove things from protobuf. That might be fixable but it's probably easier to just remove the text format for now.
[2014-11-26 10:51:56] <angavrilov> it's obviously not designed for reloading or unloading stuff and assumes every protobuf type once loaded will be loaded forever
[2014-11-26 11:09:55] <Ramblurr> supporting text serializations would be a nice feature.. people could edit them outside DF, but its not a deal breaker to lack it.
[2014-11-26 11:10:11] <Ramblurr> could just the stockpiles plugin depend on full protobuf?
[2014-11-26 11:10:24] <Ramblurr> dfhack is already building the full lib, its just not being installed
[2014-11-26 11:10:39] <Ramblurr> if not, i'll just drop text format and stick to binary only
[2014-11-26 11:10:45] <Ramblurr> angavrilov: lethosor ^
...
[2014-11-26 14:32:40] <Ramblurr> lethosor: expwnent shall i refactor the stockpiles plugin to support binary only then? so we don't lose reloading ?
[2014-11-26 14:33:23] <Ramblurr> i don't think human readable/editable stockpile settings are crucial, nice to have, but not a deal breaker
[2014-11-26 14:34:08] <expwnent> Ramblurr: Yes, I agree.
[2014-11-26 14:34:31] <expwnent> It does cross-platform support right? That'd be nice if people are doing succession forts.
[2014-11-26 14:36:48] <Ramblurr> i haven't tested it yet (need to setup a dev env in a windows vm, egh)
[2014-11-26 14:36:57] <Ramblurr> but it should be, that was the reason i used protobuf in the first place :)
[2014-11-26 14:37:16] <Ramblurr> and I'm not storing any floating points, which could be problematic i suppose, just strings, ints and bools
[2014-11-26 14:38:15] <lethosor> I think DFHack could use a human-readable config format, but it sounds like protobuf wouldn't work for that
[2014-11-26 14:39:22] ⇐ X-log__ quit ([email protected]): Read error: Connection reset by peer
[2014-11-26 14:39:25] <Ramblurr> expwnent: cross platform support is definitely a feature, as i would like people to share there settings, not just in succession forts
[2014-11-26 14:39:46] <Ramblurr> lethosor: adding a yaml or json library would make that possible
[2014-11-26 14:40:06] → X-log__ joined ([email protected])
[2014-11-26 14:40:17] <lethosor> I did look into yaml, although it would be fairly complicated to make it available to Lua
[2014-11-26 14:41:40] <Ramblurr> surely there's a json solution?
[2014-11-26 14:43:01] <lethosor> There are Lua bindings for libyaml, although that's a C library. There are C++ libraries as well, but no Lua bindings that I've found. JSON might work.

lethosor avatar May 14 '22 19:05 lethosor

Could an alternative be to load protobuf in the core library so it doesn't need to be reloaded with the plugin? We could potentially store non-reloadables in PkuginStatics.cpp. if that works out could significantly reduce dev effort for this.

myk002 avatar May 15 '22 15:05 myk002

The protobuf library is already loaded by core. The issue is with protobufs, i.e. the message types themselves that are compiled into plugins, like rename.proto.

I'd prefer to avoid bundling plugin components into core whenever possible, as it breaks modularity of plugins. In this case, my understanding is that we would have to link the generated .pb.cc file into the core library, instead of into the plugin, which would prevent self-contained changes to the plugin. If you wanted to develop on the plugin, you'd still be unable to reload it safely, just for a different reason (the protobuf type wouldn't be updated by just reloading the plugin).

At any rate, the lite runtime supports our needs, including the "standard" binary protobuf serialization protocol. For something human-readable, I would move towards JSON at this point, especially if we want it to be human-editable too. Also, this could all change if we upgrade protobuf, especially to protobuf 3+.

lethosor avatar May 15 '22 21:05 lethosor