BattletechModders / ModTek

Mod system for HBS's PC game BattleTech.
GNU Lesser General Public License v2.1
119 stars 34 forks source link

Improving the documentation #41

Closed mlangsdorf closed 6 years ago

mlangsdorf commented 6 years ago

Great project. It took me a while to wrap my head around it, but I've got a mostly working mod now.

The documentation could be better in places. I'm coming into this as a C programmer who sometimes deals with JSON when doing some side projects in python, but with no experience with C#.

First: This link https://www.newtonsoft.com/json/help/html/MergeJson.htm in https://github.com/Mpstark/ModTek/wiki/The-mod.json-Format is completely unhelpful. I can kind of figure it out, but if I were a new mod writer who doesn't do programming but knows he wants to change the HeatGenerated value of the medium laser, that link would make zero sense. I don't have a better link handy but I think it's a net negative right now.

Second: in readme.md, the sample directory structure under 'Anatomy of a ModTek mod' doesn't match the example in 'A brief primer on Developing ModTek mods'. The difference is confusing, and if you're coming to ModTek with the hopes of writing a simple mod to manipulate some json and you want to figure out what to put where, it's very unhelpful. All the examples in the doc should match each other, or should be explicitly labeled as different.

I can write a patch to fix readme.md if you'd like.

Third: The ability to write a dll and inject code into Battletech is very cool, and I'm sure people will get a lot of mileage out of it. However, I suspect a lot of people will be happy to just edit some JSON and adjust the damage values of the weapons or whatever. Rewriting the docs to make it clear what needs to be done to edit the JSONs first, and then treating dlls as an advanced topic, would probably be helpful.

Again, I can write a patch if you think this is a good idea.

mlangsdorf commented 6 years ago

Fourth issue: The first sample mod at https://github.com/Mpstark/ModTek/wiki/Writing-ModTek-JSON-mods includes the line "Since we only want to change these five things and they're all in the first level" which is useful. Unfortunately, it doesn't tell you want to do for second level objects that aren't in JArrays. It's not too tricky once you think about it, but I got hung up on it the first time I tried edit the starting lance.

Fifth issue: The log file created during load is really useful for figuring out if your mod failed to load. I don't think it's mentioned in the docs and it really, really needs to be.

mlangsdorf commented 6 years ago

Sixth issue: in readme.md, you say "There are other implicit directories like \MyModDirectory\MechDefs, a list of which can be found in, you guessed it, the in-depth guide to the mod.json format." There's no list of implicit directories at https://github.com/Mpstark/ModTek/wiki/The-mod.json-format.

mpstark commented 6 years ago

Thanks for doing this. I'll start fixing the issues later on today. I find writing documentation hard -- especially in the middle of a project that I've just had hands in the internals, it's very easy to overexplain simple things and underexplain complicated things.

I would appreciate any more little issues as you find them. I'm particularly interested in clarifying points that I breezed over over and simplifying things that could be better explained by examples.

mlangsdorf commented 6 years ago

I sympathize with the difficulty of writing docs. Outside eyes always help.

Seventh issue: The use of manifests to add completely new files is confusing. I think it's supposed to work as:

Anyway, at this point, I can modify existing json files but not add new ones. I think there's an issue with the Manifests but the docs and the code and the observed behavior are all mismatched, so it's hard to figure out what I'm doing wrong.

mpstark commented 6 years ago
  1. I've removed the confusing link and instead referred to the ModTek JSON article.

  2. I've matched up the directories. Note that the StreamingAssets mirror path doesn't need to be included in manifest.

  3. I'm not really sure how to approach fixing this.

  4. I've added a second level example.

  5. I've added a note in the JSON article about the ModTek.log and the game's output_log.txt

  6. I've removed this note, since it actually isn't true. I decided against these extra implicit directories during development in favor of explicit manifest entries.

mpstark commented 6 years ago

new files in some_directory that exists in both BATTLETECH\Mods\MyModDirectory\StreamingAssets\data\ and \BattleTech_Data\StreamingAssets\data\ will not be loaded implicitly and need to be listed in the manifest.

If it is in the game manifest at launch and is in StreamingAssets, it works exactly like a vanilla file.

new mechdef*.json files in \BATTLETECH\Mods\MyModDirectory\MechDefs will be loaded implicitly. (maybe? the only instance of MechDefs in the source tree is the readme)

there are other \BATTLETECH\Mods\MyModDirectory\ directories that will be loaded implicitly.

This was an old reference to a planned feature that I decided against -- I thought I had caught all references that referred to it. All new entries need to be specified in the mod.json manifest.

mlangsdorf commented 6 years ago

Cleaned up documents are much nicer, thanks!

Regarding point 3: I think the way you've reformated the docs makes it much easier to understand that .dlls are completely optional and a lot of my concerns go away. I think adding a link to a sample mod that makes a moderate, json-only change (the extra salvage change would be a good one) as well as linking to a mod like your 300-ton Leopard dropship limit would be helpful for giving people a properly configured directory to start from.

Regarding points 2, 6, 7: So if I've got the following files: \BATTLETECH\Mods\MyModDirectory\StreamingAssets\data\mech\mechdef_kintaro_KTO-18.json \BATTLETECH\Mods\MyModDirectory\StreamingAssets\data\mech\mechdef_amazing_AMZ-1G.json the first is a rebuild of the kintaro to match my tastes. the second is an entirely different mech. they're both in the the StreamingAssets\data\mech\ directory because that's the easiest thing for my workflow (ie, I have all of data\ in git and I'm generating the json files from that) my manifest needs to look like

    "Manifest": [
        { "Type": "MechDef", "Path": "StreamingAssets\data\mech\" }
    ]

because I'm adding a new file and I need to path that explicitly. It would helpful if that was also explicit in the documentation.

issue 8: under "special json handling" in https://github.com/Mpstark/ModTek/wiki/The-mod.json-Format, there's a json code block that's empty. It's supposed to have the ShouldMergeJSON example but it looks like it got cut in the editing process and didn't get replaced.

mpstark commented 6 years ago

\BATTLETECH\Mods\MyModDirectory\StreamingAssets\data\mech\mechdef_kintaro_KTO-18.json \BATTLETECH\Mods\MyModDirectory\StreamingAssets\data\mech\mechdef_amazing_AMZ-1G.json

This is not a good approach, at least under the current system. The intention is that all files under StreamingAssets mirror a file that is in the VersionManifest.csv in that path. If you loaded this, ModTek would see four entries for the two files, two from the StreamingAssets implicit folder and two from your explicit manifest entry.

The AMZ-1G will be skipped by the implicit handling and be added by the explicit manifest entry. The KTO-18 though, will be added twice, once as a JSONMerge and once as a replacement file for the existing entry.

The intended manner of adding new content is to have a new path in your modfolder for it, not under StreamingAssets and explicitly declare it in the manifest.

EDIT: I will fix the json code block that is empty thanks.

mlangsdorf commented 6 years ago

I think you should make it explicit that new content needs to be in a separate folder outside of StreamingAssets. I'm not arguing with that design decision, just pointing out that it needs to be explicit in the docs. That complicates my workflow somewhat but I can work around it.

mlangsdorf commented 6 years ago

Typo in readme.md: the sample mod.json doesn't have a comma between the two items in Manifest.

mpstark commented 6 years ago

I've added in two additions to the JSON examples that make it clear that StreamingAssets should only be used for modifying files from the base game. And thanks for pointing out the comma error in mod.json in the readme,,,, I've fixed it.

mlangsdorf commented 6 years ago

Looks great. Thanks for responding.