Cataclysm-BN icon indicating copy to clipboard operation
Cataclysm-BN copied to clipboard

[WiP] Move constructions to generic factory, fix some blueprint precalc bugs

Open olanti-p opened this issue 2 years ago • 0 comments

Summary

SUMMARY: Infrastructure "Move constructions to generic factory, fix some blueprint precalc bugs"

Purpose of change

Constructions:

  1. Enable copy-from for constructions
  2. Simplify implementation of new string_id<T> and int_id<T> that work in conjunction with single generic_factory<T>
  3. Clarify N/A UI note for constructions that don't require skills
  4. Allow player to build t_resin_hole_c same as similar resin terrain
  5. Enable text style checks for construction descriptions
  6. Reduce inclusions of construction.h header

Blueprint requirements autocalc:

  1. Fix tool duplication in requirements when list already contains replacing type (e.g. if list has fake_welder and welder, fake_welder would show up twice)
  2. Fix autocalc not following recipe chains beyond first 2 links
  3. Fix autocalc producing different results depending on order of definition of constructions in json files
  4. Fix autocalc silently skipping tiles that don't have construction recipes
  5. Fix autocalc producing nonsense requirements for some tiles (e.g. according to precalc you apply a paint chipper to empty tile, and then cover it with white paint, and get t_wall_w)
  6. Update blueprints with fixed requirements

Describe the solution

Constructions:

  1. Move constructions to generic factory
  2. Extract boilerplate code into macros in a separate header
  3. Replace note with No skills required
  4. Add corresponding construction recipe
  5. Change description to use translation class, and group constructions by a dedicated construction group id (port of https://github.com/CleverRaven/Cataclysm-DDA/pull/44566). Auto-generate construction groups for individual construction recipes to cut down on boilerplate. Use script to replace descriptions with group ids (included in "Additional context" section)
  6. Main offender was submap.h that included construction.h only to get definition of partial_con structure. Moved that structure into separate header, construction_partial.h, and made submap.h include that instead.

Blueprint requirements autocalc:

  1. Check if the list already contains replacing type before adding it.

  2. There were 2 problems here:

    • The function that was supposed to recursively look for construction recipes had a bug in it, so instead of doing recursive lookup it quit after first 2 steps.
    • There are construction recipes that form cycles (e.g. constr_window_taped and constr_revert_window_taped), and construction recipes that allow alternative ways of building object (e.g. constr_table and constr_place_table). As such, they form a directed graph with loops and parallel edges, and autocalc must have some deterministic way to decide on a single path through it.

    Solution here is to introduce "construction sequences". It's basically sequences of construction recipes that tell autocalc how to get from empty tile to desired terrain/furniture. To cut down on boilerplate, a construction sequence of 1 element is automatically generated for each construction recipe that starts with empty tile and results in a terrain/furniture, but only if there is only 1 such recipe that produces that specific terrain/furniture. This automates handling of trivial cases while forcing manual intervention for when it's unclear on which recipe(s) should be used.

  3. Hide accessor for raw construction recipes list, and instead build a sorted list on finalization and use that to iterate. The recipes are sorted by string ids, so iteration order becomes independent from definition order.

  4. Now that we have a concrete list of construction sequences, autocalc can tell for sure when it doesn't know how to build specific terrain/furniture and will show an error. The developer (or modder) then can decide on course of action:

    • Implement needed construction recipe(s) and/or construction sequence
    • Silence the warning by implementing an empty sequence, causing autocalc to skip that terrain/furniture
    • Silence the warning by turning off requirements check for that specific blueprint
  5. Since construction sequences now have concrete definitions, autocalc doesn't have to guess and pick some random solution that may not be a solution at all due to bug N2.

  6. Run tools/update_blueprint_needs.py and feed it debug.log produced after with the fixes.

Describe alternatives you've considered

  1. Some elaborate algorithm that would walk the recipe graph and return all possible options, than collapse them into single requirements list.
    • That'd be hard, and way out of original scope of the PR (moving constructions to generic factory)
    • I'm not sure whether our requirements system supports enough complexity to express all possible combinations
  2. Not porting the construction grouping PR. It's a lot of boilerplate for seemingly little gain. Counter-points:
    • Boilerplate has been somewhat reduced with group autogen
    • copy-from should reduce boilerplate even further
    • It reduces amount of merge conflicts in the future
  3. Getting rid of basecamps

Testing

Not much, working on it

TODO list

  • [ ] Handle blacklisted construction recipes in sequences
  • [ ] Check autogeneration of construction groups for copy-fromed constructions
  • [ ] Get rid of commented out code (leftover from prototyping)
  • [ ] Test some more
  • [ ] Write docs
  • [ ] Take a look over newly precalculated requirements in blueprints, possibly adjust them

Additional context

Yet another rabbit hole...

Also, here's the script to replace descriptions with group ids. It's an altered version from the original PR (https://github.com/CleverRaven/Cataclysm-DDA/pull/44566) that doesn't produce groups that would have 1 element

Spoiler
#!/usr/bin/env python3

import glob
import json
import os
import re
import subprocess
import sys


def dump_json_and_lint(content, path):
    with open(path, 'w', encoding='utf-8') as fs:
        json.dump(content, fs, indent=2)
    json_formatter_name = glob.glob(
        'tools/format/json_formatter.[ec]*')
    assert len(json_formatter_name) == 1
    subprocess.run([json_formatter_name[0], path],
                stdout=subprocess.DEVNULL)


def main(argv):
    mod_dirs = {
        "data/core",
        "data/json",
    }

    with os.scandir("data/mods") as it:
        for entry in it:
            if entry.is_dir():
                mod_dirs.add(os.path.join("data/mods", entry.name))

    json_filename = re.compile("[^.].*\\.json")

    group_to_desc_map = dict()
    mod_to_groups_map = dict()

    for mod_dir in mod_dirs:
        mod_to_groups_map[mod_dir] = set()
        print("walking dir {}".format(mod_dir))
        for root, dirs, files in os.walk(mod_dir):
            for file in files:
                json_path = os.path.join(root, file)
                content = None
                if json_filename.match(file):
                    try:
                        with open(json_path, 'r', encoding='utf-8') as fs:
                            content = json.load(fs)
                    except Exception:
                        sys.stderr.write('Error parsing %r\n' % json_path)
                        raise
                if type(content) is list:
                    for j in range(len(content)):
                        obj = content[j]
                        if not (type(obj) is dict and
                                "type" in obj and obj["type"] == "construction" and
                                "description" in obj):
                            continue
                        group = re.sub(r'[^\w\d]+', '_', obj["description"].lower()).strip('_')
                        assert group not in group_to_desc_map or group_to_desc_map[group]["desc"] == obj["description"]
                        if group in group_to_desc_map:
                            group_to_desc_map[group]["count"] += 1
                        else:
                            new_group = dict()
                            new_group["desc"] = obj["description"]
                            new_group["count"] = 1
                            group_to_desc_map[group] = new_group
                            mod_to_groups_map[mod_dir].add(group)
                
        for root, dirs, files in os.walk(mod_dir):
            for file in files:
                json_path = os.path.join(root, file)
                content = None
                changed = False
                if json_filename.match(file):
                    try:
                        with open(json_path, 'r', encoding='utf-8') as fs:
                            content = json.load(fs)
                    except Exception:
                        sys.stderr.write('Error parsing %r\n' % json_path)
                        raise
                if type(content) is list:
                    for j in range(len(content)):
                        obj = content[j]
                        if not (type(obj) is dict and
                                "type" in obj and obj["type"] == "construction" and
                                "description" in obj):
                            continue
                        group = re.sub(r'[^\w\d]+', '_', obj["description"].lower()).strip('_')
                        assert group in group_to_desc_map
                        if group_to_desc_map[group]["count"] > 1:
                            new_obj = dict()
                            for k, v in obj.items():
                                if k == "description":
                                    new_obj["group"] = group
                                else:
                                    new_obj[k] = v
                            content[j] = new_obj
                            if not changed:
                                changed = True
                                print("updating {}".format(json_path))
                if changed:
                    dump_json_and_lint(content, json_path)

        if mod_to_groups_map[mod_dir]:
            groups_filtered = [ group for group in sorted(mod_to_groups_map[mod_dir]) if group_to_desc_map[group]["count"] > 1 ]

            if len(groups_filtered) > 0:
                mod_groups_json = [
                    {
                        "type": "construction_group",
                        "id": group,
                        "name": re.sub(r'\.$', '', group_to_desc_map[group]["desc"]),
                    } for group in groups_filtered
                ]
                dump_json_and_lint(mod_groups_json, os.path.join(mod_dir, "construction_group.json"))


if __name__ == "__main__":
    main(sys.argv[1:])

olanti-p avatar Jul 17 '22 19:07 olanti-p