realistic limits for combine #897
Conversation
combine.lua
Outdated
| local function unlimited_stacks(types) | ||
| log(4, 'Unlimited stacks\n') | ||
|
|
||
| if opts.unlimited_stack then |
There was a problem hiding this comment.
It might be cleaner/clearer if this if statement guarded the call to this function in populate_stacks instead of the body here
combine.lua
Outdated
| @@ -738,6 +758,7 @@ local function parse_commandline(opts, args) | |||
| {'t', 'types', hasArg=true, handler=function(optarg) opts.types=parse_types_opts(optarg) end}, | |||
| {'d', 'dry-run', handler=function() opts.dry_run = true end}, | |||
| {'q', 'quiet', handler=function() opts.quiet = true end}, | |||
| {'u','unlimited-stack',handler=function() opts.unlimited_stack = true end}, | |||
There was a problem hiding this comment.
Needs documenting in the rst file
…yone/dfhack-scripts into combine_capacity_limits
…yone/dfhack-scripts into combine_capacity_limits
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
…yone/dfhack-scripts into combine_capacity_limits
…yone/dfhack-scripts into combine_capacity_limits
|
@myk002 I've yet to complete the container volume allocations, but have put in the capture of the container volumes, as well as the item volumes. Yet to update the update log file. I've done the other review comments. Let me know if there are other changes needed. |
|
I saw that about the container volumes. I figured that would be completed in a later PR. The stack size limits in this PR is already good progress. |
There was a problem hiding this comment.
I'm a bit concerned about the -u flag. As currently written, combine all -u will have negative consequences.
I suggest one of the following:
- Change unlimited to a custom maximum
- Change seeds to only act on non-plantable seeds and change default size to 5
- Change body parts to teeth and change default size to 20
OR
Remove -u, corpse parts, and seeds from combine for now and implement the first option later
I'm thinking the second option would be best, given the short timeframe for -r4 (and I really want to get the max stack size changes at least into -r4)
docs/combine.rst
Outdated
|
|
||
| ``parts``: CORPSEPIECE | ||
| ``parts``: CORPSEPIECE. Max 1. |
There was a problem hiding this comment.
Can corpsepiece be removed from this list if the max stack size is 1?
Edit: oh I see, it's still useful for -u.
Would this be more useful in general if it just handled teeth and the default stack size was large?
docs/combine.rst
Outdated
|
|
||
| ``seed``: SEEDS | ||
| ``seed``: SEEDS. Max 1. |
There was a problem hiding this comment.
This can be 5 if we filter for only non-plantable seeds. Also, we really should filter for non-plantable seeds or else -u could destroy farming
|
Okay, I'll pull those out into another later branch. What is the timing for the next release? I'll take out corpse and seeds until I can figure out how to do non plantable seeds and teeth. |
|
Next release maybe this coming Thursday. Cuz DFHack/dfhack#4040 |
|
Yeah that's close. I'll have a look at it tonight. |
…yone/dfhack-scripts into combine_capacity_limits
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
|
@myk002 I've reverted back to the realistic combine. Not sure why the pre-commit.ci isn't working. I'm heading to bed now. can have another look tomorrow evening. |
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
Refer to #852.
Implemented reduced stack sizes.