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

upgrade reversible recipe property #36872

Closed
wants to merge 1 commit into from
Closed

upgrade reversible recipe property #36872

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 10, 2020

Summary

SUMMARY: Infrastructure "allow reversible to accept time duration"

Purpose of change

Making new uncraft recipes and hassling with json formatting just to adjust disassemble time is rather inefficient way spending ones time - which I reckon is the main reason why almost everything defaults to craft time and uncraft recipes are barely touched unless absolutely required.

Closes #36841

Describe the solution

allow reversible to accept type time_duration and assign it to auto-generated uncraft recipe time
add few examples

Describe alternatives you've considered

Also provide default values based on main skill used

Testing

Observe mp3 and portable_game disassemble time, note that they don't have dedicated uncraft recipes.

@AMurkin
Copy link
Contributor

AMurkin commented Jan 10, 2020

I don't think that mixing value types is the right thing to do, and it should be avoided.

@I-am-Erk I-am-Erk added [C++] Changes (can be) made in C++. Previously named `Code` Crafting / Construction / Recipes Includes: Uncrafting / Disassembling labels Jan 10, 2020
@ZhilkinSerg
Copy link
Contributor

Better introduce a new field uncraft_time to avoid confusion.

@ghost
Copy link
Author

ghost commented Jan 14, 2020

That was my initial approach, but once I ran this for a while I realized that it is unnecessary to add dedicated json property when the parser needs to resolve any latent typing anyway due to nature of language, and that having both types in single value is more efficient and less error prone to update/maintain.

@kevingranade
Copy link
Member

This lacks clarity, another field is a small price to pay for that, or alternately add a new key with a mandatory time string.

@kevingranade
Copy link
Member

This has been sitting with "must-have" feedback for two months, feel free to reopen with those points addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Crafting / Construction / Recipes Includes: Uncrafting / Disassembling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Taking stuff apart
4 participants