Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CR]Nested crafting requirements #17435

Closed
wants to merge 7 commits into from

Conversation

Projects
None yet
3 participants
@Coolthulhu
Copy link
Contributor

commented Jun 30, 2016

Includes #17434

Currently a bit hacky due to a type id "cast" that is removed at finalization.

Allows extracting crafting components and tools to a non-inlined requirement.
As an example, I modified the granola recipe to use a new "fruit_sweet" requirement instead of a long list of sweet fruit.

Supports multiplication of requirements.

Currently only supports inlining the requirements in the last array of the vector. That is, it only allows alternative components.
For example, you can list a list of fruits to use in granola, have alternative bread types for sandwich, but can't have one inlined requirement contain both "any kind of bread" and "butter or peanut butter" - the recipe would need to list those two separately.

@mugling

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2016

This looks like a duplication of the using syntax I'm working on?

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2016

Couldn't find a way to do it with using, since it is in the topmost context, while we really need it to be closer to the middle.

@mugling

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2016

What would the ideal syntax look like?

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2016

It needs to allow inlining requirements in the very last level. That is THE most important requirement of requirements.

For example,

"tools" : [ [ [ "any_welding", 10 ] ] ]
"components" : [ [ [ "weldable_metal", 10 ] ], [ [ "copper_wire", 30 ] ] ]
@mugling

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2016

That would be "using": [ [ "welding_standard", 10 ], [ "weldable_metal", 10 ] ]

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2016

How about translating:

"tools": [ [ [ "heat", 20 ] ] ],
"components": [
    [
      [ "oatmeal", 4 ],
      [ "dry_rice", 4 ]
    ],
    [
      [ "honey_bottled", 4 ],
      [ "honey_glassed", 4 ]
    ],
    [
      [ "any_kind_of_fruit", 1 ],
      [ "chocolate", 1 ],
      [ "candy", 3 ],
      [ "candy2", 3 ],
      [ "choco_coffee_beans", 5 ]
    ]
  ]

EDIT: Also, in your example above, there is no copper wire.

@mugling

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2016

I entirely agree your version is less limiting but it's also more complexity and we already have support almost implemented for using that covers 95% of the reasonable use cases.

My concern is that this is going to make refactoring crafting to support qualities OR skills etc more difficult.

In your above example what happens if I add a tool quality for example SCREW 1 to any_kind_of_fruit?

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2016

I entirely agree your version is less limiting but it's also more complexity and we already have support almost implemented for using that covers 95% of the reasonable use cases.

I still can't think of how to implement the use cases I listed above with using. The use case with granola is incredibly common - almost all cooking recipes involving fruit look like that.

If this use case can't be handled, then the 95% applies to cases using does NOT cover, not to the ones it does. And if this is the case, it would be better to scrap it and reimplement it in a way that allows inserting lists of components.

My concern is that this is going to make refactoring crafting to support qualities OR skills etc more difficult.

It wouldn't, really. I wrote it with templates where applicable (tool vs component was a problem). In fact, I tried to make it as generic as possible.

In your above example what happens if I add a tool quality for example SCREW 1 to any_kind_of_fruit?

Currently it will complain and then break. I used the requirements class because it was ready and had all the relevant checks. It can be exchanged for some other class, possibly a totally new one (component_list?).

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2016

Do you have a better idea on how to do that?
My approach probably isn't the only one out there, it's just that it allows the most important thing: freely nesting components in relevant sections.

@mugling

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2016

Nesting is important but I think now equally if not more important is to be able to combine for example qualiities OR time.

The crafting code is horrible so it was a good idea to split this PR into a refactor and extension part.

using is suitable for the interim transition as it doesn't mangle existing JSON definitions too much. It also solves the vehicle installation requirements problem and of course allows us to start defining requirement sets.

If we're going to rewrite crafting it should be nested and also support arbitrary combinations.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2016

If we're going to rewrite crafting it should be nested and also support arbitrary combinations.

You mean something like unifying all 3 types of components?
The only real problem here would be tools used as components: an entire soldering iron for welder rig, for example.

@mugling

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2016

You mean something like unifying all 3 types of components?

Yes, it's the last major piece of the puzzle

@mugling

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2016

A simpler solution that doesn't become obsolete with further changes is to allow recipes to accept an item_group. As an added bonus that keeps related items in sync with their recipes.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2016

A simpler solution that doesn't become obsolete with further changes is to allow recipes to accept an item_group.

Item groups are bad with multiplicity.

@mugling

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2016

They could be tagged with a crafting: 4 field? If we already have an item list class it seems a shame not to reuse that especially since it can be used to keep things more in sync?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.