Fix assorted json errors #34455

jbytheway · 2019-10-04T11:12:25Z

Summary

SUMMARY: Bugfixes "Fix assorted json errors"

Purpose of change

I have written some code to verify that when parsing json files we actually visit every member of every json object. Failing to do so is often a sign of a mistake in the json. For example a typo in a key name, or a misplaced bracket, or a simple misunderstanding of the format.

This happens a lot. See #33144 #33328 #33502 #33613 #33852 for examples of fixes of this type. I'd like to catch them earlier.

Enabling that code surfaced numerous errors in the existing json. I need to fix those errors before I can PR the code that checks for them.

This is the first batch of such fixes, for the dda core game data.

Describe the solution

The vast majority of these changes are to the game data itself. These are:

Removing parts that just don't do anything and probably aren't important.
Fixing typos and misplacements when I was able to do so.
Normalizing the comment format. Now all json 'comments' must have a key starting with //.
I added a new item group allclothes_damaged because it was required for many places which were trying to apply damage in a way that doesn't work.
Some fields which don't work but contained potentially useful information (like freezing points) I 'commented out' by prefixing them with // rather than just deleting them.

I also tweaked a couple of pieces of code:

Expand vehicle part installation requirements to allow for using both using and the 'standard' requirements fields.
Improved some error messages regarding json malformedness.

Describe alternatives you've considered

One of the more concerning changes is that a bunch of magen code is using chance in places where that doesn't work. I considered making it work instead of removing it as I have, but I didn't want to get any further sidetracked in this PR which has already taken a couple of weeks. All those changers are packaged up in one commit so it can be easily reverted in the future if anyone ever adds support for chance in these extra mapgen components. I probably should have don the commenting out trick. I can go back and do that if it seems worthwhile.

Additional context

If reviewing these changes I recommend doing so one commit at a time, rather than reading throught the diff in aggregate; it will be easier to see what's going on.

Next step is to look for similar errors in (some of) the core mods, and then push the verification code that checks for these mistakes to prevent them returning.

ZhilkinSerg · 2019-10-04T13:10:47Z

data/json/items/chemicals_and_resources.json

    "phase": "liquid",
    "container": "bottle_glass",
-    "freezing_point": 25,
+    "//freezing_point": 25,


What's the idea here? Isn't freezing_point a valid node?

freezing_point is only valid for comestibles. This is an ammo.

ZhilkinSerg · 2019-10-04T13:11:28Z

data/json/items/chemicals_and_resources.json

@@ -958,7 +955,7 @@
    "name": "handful of hickory nuts",
    "name_plural": "handfuls of hickory nuts",
    "description": "A handful of hard nuts from a hickory tree, still in their shell.",
-    "spoils_in": "180 days",
+    "//spoils_in": "180 days",


spoils_in should be valid node too.

spoils_in is only valid for comestibles. This is a generic item.

snipercup · 2019-10-04T16:28:32Z

I tested your changes and encountered no problems.

I could build it with msys2 on windows and there were no unusual errors.

In-game:

Browsed trough recipes
Browsed trough debug item spawn list
visited different locations on the map (cities and specials and fields)
Uninstalled and installed a vehicle part

The only errors I got were:

 DEBUG    : invalid furniture id "f_feather_mattress"
 
 FUNCTION : const T& generic_factory<T>::obj(const string_id<T>&) const [with T = furn_t]
 FILE     : src/generic_factory.h
 LINE     : 356



 DEBUG    : Tried to access invalid map position at grid (0,11,0)
 
 FUNCTION : size_t map::get_nonant(const tripoint&) const
 FILE     : src/map.cpp
 LINE     : 8148

But I don't think they are related to your change.

Night-Pryanik · 2019-10-04T16:59:25Z

Damn, so many errors. So many things worked not the way they should.

jbytheway · 2019-10-05T01:14:02Z

The two cases @ZhilkinSerg highlighted are both examples of item fields which are only valid for some types of item. By default, each item only loads basic info and the details for its slot (in this case, the comestible slot). For some slots it is possible to use them for other item types by explicitly including a sub-member for that slot (e.g. "container_data" for the islot_container). However, that doesn't appear to be possible for islot_comestible.

There were many different comment-ish keys being used: "//", "info", "note", "comment", "description". Some had in-game meaning in some contexts, and some did not. Try to rationalize them all so that the "//" prefix is always used for pure-json comments.

This fixes a bunch of places where json was slightly off somehow, either because some key had a typo in it or some chunk of json was misplaced somehow.

This removes a bunch of json fields. All of them do nothing. And largely they're just unnecessary. So I think they can just be discarded.

These are values that don't do anything, but seemed worth keeping for posterity in case they might be used for something in the future.

For many pieces of mapgen (NPCs, furniture, etc.) the "chance" field doesn't work, even though map authors clearly expect it to. This commit removes uses of "chance" in unsupported places. Alternatively, we could add support for it. That should probably happen at some point.

A bunch of mapgen was trying to spawn damaged clothing in a manner that doesn't work. Rearrange things a bit into a way that might work.

Mapgen in these locations doesn't allow you to specify charges. Remove uses that did nothing.

This mission wasn't connected to anything and served no purpose.

There were two ways to specify vehicle part installation requirements. Either with "using" or with standard requirements ("components", "qualities", and "tools"). Previously the two were mutually exclusive, but some parts tried to use both. There's no reason we should prevent that, so allow both to be used.

Improve context for consistency check errors in item groups. Improve errors for undefined item (group) types in mapgen.

KorGgenT added <Bugfix> This is a fix for a bug (or closes open issue) [JSON] Changes (can be) made in JSON labels Oct 4, 2019

ZhilkinSerg reviewed Oct 4, 2019

View reviewed changes

jbytheway force-pushed the fix_assorted_json_errors branch from 81206d2 to 9bbd6d2 Compare October 5, 2019 01:01

jbytheway force-pushed the fix_assorted_json_errors branch from 9bbd6d2 to 3e737a9 Compare October 6, 2019 18:05

jbytheway mentioned this pull request Oct 7, 2019

Fix Magiclysm json errors #34525

Merged

jbytheway added 10 commits October 6, 2019 21:37

Rationalize json comments

e7c9197

There were many different comment-ish keys being used: "//", "info", "note", "comment", "description". Some had in-game meaning in some contexts, and some did not. Try to rationalize them all so that the "//" prefix is always used for pure-json comments.

Fix many json typos and misplacement of fields

47a5d40

This fixes a bunch of places where json was slightly off somehow, either because some key had a typo in it or some chunk of json was misplaced somehow.

Remove many unused, unnecessary json fields

c2f7284

This removes a bunch of json fields. All of them do nothing. And largely they're just unnecessary. So I think they can just be discarded.

"Comment out" some json fields

68de343

These are values that don't do anything, but seemed worth keeping for posterity in case they might be used for something in the future.

Add allclothes_damaged item group

f83023f

A bunch of mapgen was trying to spawn damaged clothing in a manner that doesn't work. Rearrange things a bit into a way that might work.

Remove unsupported "charges" values

2898ba3

Mapgen in these locations doesn't allow you to specify charges. Remove uses that did nothing.

Remove MISSION_BOOK

5a818d5

This mission wasn't connected to anything and served no purpose.

Better error messages

74feb9d

Improve context for consistency check errors in item groups. Improve errors for undefined item (group) types in mapgen.

jbytheway force-pushed the fix_assorted_json_errors branch from 3e737a9 to 74feb9d Compare October 7, 2019 01:37

ZhilkinSerg merged commit 0733972 into CleverRaven:master Oct 7, 2019

jbytheway deleted the fix_assorted_json_errors branch October 8, 2019 10:40

curstwist mentioned this pull request Oct 11, 2019

hide blacksmith survey #34661

Merged

jbytheway mentioned this pull request Oct 14, 2019

Detect unused json members #34735

Merged

esotericist mentioned this pull request Oct 14, 2019

Un-Swappable Storage Batteries #34741

Closed

jbytheway mentioned this pull request Dec 17, 2019

Many mods have json errors that need to be cleared up #36171

Closed

51 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix assorted json errors #34455

Fix assorted json errors #34455

jbytheway commented Oct 4, 2019

ZhilkinSerg Oct 4, 2019

jbytheway Oct 5, 2019 •

edited

Loading

ZhilkinSerg Oct 4, 2019

jbytheway Oct 5, 2019

snipercup commented Oct 4, 2019

Night-Pryanik commented Oct 4, 2019

jbytheway commented Oct 5, 2019

Fix assorted json errors #34455

Fix assorted json errors #34455

Conversation

jbytheway commented Oct 4, 2019

Summary

Purpose of change

Describe the solution

Describe alternatives you've considered

Additional context

ZhilkinSerg Oct 4, 2019

Choose a reason for hiding this comment

jbytheway Oct 5, 2019 • edited Loading

Choose a reason for hiding this comment

ZhilkinSerg Oct 4, 2019

Choose a reason for hiding this comment

jbytheway Oct 5, 2019

Choose a reason for hiding this comment

snipercup commented Oct 4, 2019

Night-Pryanik commented Oct 4, 2019

jbytheway commented Oct 5, 2019

jbytheway Oct 5, 2019 •

edited

Loading