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

Efficiency increase for ban-cooking and change to 'seeds' option #1241

Merged
merged 5 commits into from
Jul 31, 2024

Conversation

Droseran
Copy link
Contributor

Increase the efficiency of ban-cooking by breaking the iteration of the material search when it finds the material ID being looked for (specifically 'STRUCTURAL' in this case).

The 'seeds' option now bans cooking for all seed producing plants and growths instead of just items that produce seeds and are brewable. The previous method could cause replantability to be lost for plants with cookable seed producing items, affecting milling in vanilla and other industries in mods. It may be a good idea to split this into two functions in the future, since the documentation only mentions that this function bans cooking seeds and not the items that produce them. This is also the only option that affects two significantly different groups of items, which is a bit inconsistent.

Droseran added 3 commits July 25, 2024 17:37
Stop iterating through materials when the material being searched for (STRUCTURAL) is found since material IDs should be unique.
Removed the check for has_drink from funcs.seeds.

Currently the 'seeds' function bans seed producing items only if the item is also brewable. This allows the possibility that seeds and replanting can be lost for cookable seed producing items that are also using for milling, spinning, etc. reactions.

This maybe should be split into two functions, one for banning seeds themselves and one for banning seed producing items. Currently the documentation describes this function as only banning seeds, which may lead to some confusion.
Added ban-cooking changes to changelog
Copy link
Member

@myk002 myk002 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you also update the description of the seeds option in docs/ban-cooking.rst?

changelog.txt Outdated
@@ -93,6 +93,8 @@ Template for new versions:
- `clear-smoke`: properly tag smoke flows for garbage collection to avoid memory leak
- `warn-stranded`: don't warn for babies carried by mothers who happen to be gathering fruit from trees
- `prioritize`: also boost priority of already-claimed jobs when boosting priority of a job type so those jobs are not interrupted
- `ban-cooking`: stop iterating through plant materials when the searched-for material is found
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is not user-visible and doesn't need a changelog line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I've removed that line from the changelog and updated the documentation file to clarify which items the 'seeds' option affects.

ban-cooking.lua Show resolved Hide resolved
Droseran added 2 commits July 30, 2024 04:26
Remove line from changelog referencing a non-user facing aspect of this update.
Clarify that the 'seeds' option also bans plants and growths which produce seeds in reactions.
@Droseran Droseran changed the title Efficieny increase for ban-cooking and change to 'seeds' option Efficiency increase for ban-cooking and change to 'seeds' option Jul 30, 2024
@myk002 myk002 merged commit 63801db into DFHack:master Jul 31, 2024
10 checks passed
@Droseran Droseran deleted the ban-cooking-patch branch August 3, 2024 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants