Correctly predict craftability of recipes with overlapping item requirements

[jbytheway]

Summary

SUMMARY: Bugfixes "Correctly predict craftability of recipes with overlapping item requirements"

Purpose of change

Fixes #32311.
Fixes #32554.

When recipes have overlapping requirements the current implementation for checking for availability of those requirements does not properly take into account the fact that a single item cannot be used to satisfy more than one.

(Actually, for overlaps between components and tools, there is some code to check for this, but not between different component requirements.)

This causes problems with some recipes. In particular, survivor telescope is discussed in #32311. The most severe example of this phenomenon is chainmail_suit_faraday which has 6 requirements each of which can be satisfied either by some chainmail sheets or by a specific piece of chainmail armour.

As things stand, the crafting GUI will display these items as being craftable, but if you try to craft them then various strange things can happen, including debugmsg errors, and you end up with an in-progress craft object with 0% progress which you can't continue nor disassemble to recover the ingredients. This is clearly unsatisfactory.

Describe the solution

(Description of the solution adapted from the discussion in #32311).

The basic idea is to detect recipes with overlapping requirements and refactor them into a set of alternative requirements, each of which is internally non-overlapping, then try each of these in turn.

So, for example, the survivor telescope recipe currently has the requirements:

1 high-quality lens
AND
1 high-quality lens OR 1 small high-quality lens

These would internally be changed to have two sets of requirements:

2 high-quality lens
OR
1 high-quality lens AND 1 small high-quality lens

No change to the json is required, this is calculated as part of recipe finalization.

Implementation-wise, this is done via a new class deduped_requirement_data, which contains a vector of requirement_data objects.

Recipes now expose both representations of their requirements, because code can want either the original or the deduped version depending on its needs. The original version is typically smaller and easier to understand, so that's used for example for displaying the requirements to the player.

As another example, the vegetable soup recipe (as discussed in #32554) was a bit more complex (looks like it's since been edited to not have any overlap, but it's still a good example). It used to look essentially like:

eggs OR wild veggies
AND
other veggies OR wild veggies

and is now transformed into

eggs AND (other veggies OR wild veggies)
OR
wild veggies AND other veggies
OR
2 wild veggies

With more overlapping requirements or more items in the overlap the number of alternatives rises fast, but nothing in the core game gets too absurd. The worst case is the aforementioned chainmail_suit_faraday, which requires 63 alternative requirement_data objects. I've set a limit of 100 as a sanity-check to prevent mods performing a denial-of-service attack on the algorithm.

The crafting GUI now correctly displays when items are craftable.

As a side-effect, I have reworked how player::can_start_craft operates. This seeks to detect whether a craft can be started (but maybe not finished), which requires only 5% of the tool charges. It used to do this by creating a whole new requirements object with different tools requirements, but this interacted poorly with this new feature. So, instead, I've added a new flag that can be passed to the various test-craftability functions, which ultimately gets used at the very innermost function to tweak the number of tool charges sought. I'm a little unhappy at how the "5%" arbitrary constant is scattered all over the codebase; I think it might be good to refactor that further, but not in this PR.

TODO:

• When an item is in this strange corner case all the requirements in the crafting menu are green. Need to add a message to help the player understand what's going on.
• When an item is actually crafted, the player needs to decide how to allocate their resources (in the event there are multiple options). This requires a new choice interface that is more holistic than the existing one for choosing ingredients for a specific requirement within the recipe. @ifreund has suggested one approach which would fit well with the existing approach, but would be tricky to implement. I plan to add something simpler initially. If it proves unwieldy then we can think about how to improve it.
• Test that partial crafting (starting a craft that you can't finish, and resuming later) hasn't been broken.

Describe alternatives you've considered

Various options are discussed in the comments for #32311.

This PR only tackles overlapping item requirements. There could in principle be a similar issue with overlapping tools requirements, which could be solved in the same way. I haven't seen that be an issue in practice, though, so I'm hoping YAGNI. In particular, because tool charges are consumed gradually you will probably always at least be able to start the craft.

Testing

Added several new unit tests to test the requirement refactoring algorithm.

Experiments in-game to test the crafting GUI.

Additional context

The crafting code is a complex beast that deals with many semi-orthogonal but interconnected issues. This is a sufficiently invasive change that I'm fairly sure I must have broken something. In particular, I'm not confident I've used the correct inventory in all cases.

Recipe finalization is already the slowest step in game data finalization, and this is making it even slower again. It's not too bad, even on a debug build as I'm using, but it's something we need to keep in mind.

(My apologies for adding a new function to player...)

The new error message that appears in the crafting GUI for this situation:

Crafting a survivor telescope is possible with sufficient lenses:

When you craft and have all the lenses, you must now choose between these options:


Things can get more complicated for recipes like the faraday suit. Here are three options when you have a reasonable selection of chainmail items on hand:



Here's the most crazy possibility, when all 63 options are possible and you must choose from them all. The uilist code still handles it correctly. It doesn't look great, but it does work.

[jbytheway]

While working on the next steps for this I discovered a bug in basecamp upgrades that needs to be fixed before this can be, so this will be on hold while I work on that.

[jbytheway]

The extra features I wanted are now done, although this is now built on top of #36740
I've added a bunch of screenshots to the PR description above showing the new UI elements.

If anyone has any reasonable suggestions for any names to give the (currently blank) uilist entries pictured there, I'd welcome them.

I still want to do some more testing that partial crafting is still working correctly.

[jbytheway]

As I suspected, I had broken partial crafting. But it's fixed now.

[jbytheway]

Now #36740 is merged I'm not aware of any more blockers for this merge. @ifreund did mention that he was hoping to review the changes, though.

[ifreund]

This looks good to me, I couldn't find anything actually wrong so I just made some nitpicking suggestions.

The rework of can_start_craft() seems like a definite improvement to me, I too am not a huge fan of the 5% constant as it is so arbitrary, but it keeps things relatively simple. I've toyed with the idea of trying to make it dynamic based on crafting time but haven't looked into it too deeply yet.

I think the basic UI you have implemented currently is totally fine for this PR. Obviously it can get ugly/hard to use in extreme cases but as far as I can tell such cases will most likely be quite rare given the current recipes. It's also FAR simpler to implement than my decision tree idea, though we could always give that a try if the UI becomes a serious issue.

I'm not very familiar with the basecamp code, but see nothing obvious amiss there after your fix in #36740. The changes to the core crafting code all make sense to me. I played around with it briefly locally and wasn't able to break it.

Congrats on clearing one of the trickier 0.E blockers!