Added autowheelbarrow script for stockpile management #1328
Conversation
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
|
problem. If to many wheelbarrows there won't be enough time for the unit finish storing the wheelbarrow causing a never ending cycle of storing wheelbarrows. |
Changed the amount of days it takes to loop the program as when there are to many wheelbarrows it can take a few days to carry the wheelbarrow over to a stockpile resulting in a infinite loop. Changing to 7 days results in even kids reaching their destination with a wheelbarrow before the 7th day.
There was a problem hiding this comment.
I think this needs substantial improvements before it can be considered for inclusion. In particular, all the magic numbers need to be removed. Further, I'm not convinced that the negative approach (i.e. skipping stockpiles that have certain goods enabled) is the right one. It sounds more reasonable to me, to (only) assign wheelbarrows to stockpiles that have goods that use them.
| if stockpile_settings and stockpile_settings.food then | ||
| local food_settings = stockpile_settings.food | ||
| skip_wheelbarrows = #food_settings.meat > 0 or #food_settings.fish > 0 or #food_settings.unprepared_fish > 0 or | ||
| #food_settings.egg > 0 or #food_settings.plants > 0 or #food_settings.drink_plant > 0 or | ||
| #food_settings.drink_animal > 0 or #food_settings.cheese_animal > 0 or #food_settings.cheese_plant > 0 or | ||
| #food_settings.seeds > 0 or #food_settings.leaves > 0 or #food_settings.powder_plant > 0 or | ||
| #food_settings.powder_creature > 0 or #food_settings.glob > 0 or #food_settings.glob_paste > 0 or | ||
| #food_settings.glob_pressed > 0 or #food_settings.liquid_plant > 0 or #food_settings.liquid_animal > 0 or | ||
| #food_settings.liquid_misc > 0 | ||
| --dfhack.println("Skipping stockpile due to food items" .. tostring(building)) |
There was a problem hiding this comment.
I have no idea why stockpiles containing food should not have wheelbarrows. After all, it is quite possible to have a stockpile that has both food and boulders enabled.
There was a problem hiding this comment.
I understand what you're saying, but I don't like it. The main purpose of creating this program was to maintain high efficiency in stockpiles containing wheelbarrows and, of course, have wheelbarrows move freely between stockpiles. Having food and stones together sounds insane to me, and this program will instead be a detriment to users, as those stockpiles will not be set to 1 automatically if stone is found or to x (wheelbarrows in the fort) per the program's functionality. Additionally, it will be a problem if 30 wheelbarrows inhabit a food and stone stockpile, as that is a possibility if I remove the conditions, which will cascade into a problem with food. I'm sorry for being stubborn; I think these conditions are essential. If this program is scrapped or repurposed, that's fine.
There was a problem hiding this comment.
I think you missed the main point of my remark. I wasn't suggesting that this script should fully account for mixed food and stone stockpiles - I agree that those aren't very useful.
My point was that the program logic could be drastically simplified using a positive check for item categories that require wheelbarrows (i.e. boulders). I don't know which other item categories consistently reach the 75Γ threshold for being carried using a wheelbarrow. Platinum and gold bars do, as do filled minecarts.
Another possibility might be to never check the item flags of the stockpile, and instead only modify the number of requested wheelbarrows for stockpiles that request at least one wheelbarrow. By default, that applies only to stone stockpiles, but the user could decide to assign in wheelbarrows to other stockpiles as well.
| -- Check if all flags are set to false (for the none stockpile) | ||
| if stockpile_settings and stockpile_settings.flags then | ||
| local flags = stockpile_settings.flags | ||
| if flags.animals == false and flags.food == false and flags.furniture == false and | ||
| flags.corpses == false and flags.refuse == false and flags.stone == false and flags.ammo == false and | ||
| flags.coins == false and flags.bars_blocks == false and flags.gems == false and flags.finished_goods == false and | ||
| flags.leather == false and flags.cloth == false and flags.wood == false and flags.weapons == false and | ||
| flags.armor == false and flags.sheet == false and | ||
| flags[17] == false and flags[18] == false and flags[19] == false and flags[20] == false and | ||
| flags[21] == false and flags[22] == false and flags[23] == false and flags[24] == false and | ||
| flags[25] == false and flags[26] == false and flags[27] == false and flags[28] == false and | ||
| flags[29] == false and flags[30] == false and flags[31] == false then | ||
| skip_wheelbarrows = true | ||
| --dfhack.println("Skipping NONE stockpile" .. tostring(building)) | ||
| end | ||
| end |
There was a problem hiding this comment.
Checking unnamed flags likely does not make sense at all. If anything, the code should probably check whether the stockpile has any goods enabled that make use of wheelbarrows, instead of checking whether it has (also) some goods enabled that do not make use of them.
There was a problem hiding this comment.
You got me there. idk what they are for, but they exist so I added them.
There was a problem hiding this comment.
they only "exist" in the sense that the flag word has 32 bits. they don't have a (known) use and so predicating any behavior on their value will result in unpredictable behavior because their value is unpredictable
| -- Manage on state change | ||
| dfhack.onStateChange[GLOBAL_KEY] = function(sc) | ||
| if sc == SC_MAP_LOADED and df.global.gamemode == df.game_mode.DWARF then | ||
| event_loop() -- Start the event loop when the game map is loaded | ||
| elseif sc == SC_MAP_UNLOADED then | ||
| repeatutil.cancel(GLOBAL_KEY) -- Stop the event loop when the map is unloaded | ||
| end | ||
| end | ||
|
|
||
| -- Enable the script and start the event loop | ||
| enabled = true | ||
| event_loop() |
There was a problem hiding this comment.
Have a look at how the enable API is supposed to be used:
https://docs.dfhack.org/en/stable/docs/dev/Lua%20API.html#script-enable-api
| local function auto_wheelbarrows() | ||
| for _, unit in ipairs(df.global.world.units.active) do | ||
| if dfhack.units.isCitizen(unit) and unit.job.current_job and unit.job.current_job.job_type == 38 then | ||
| unassign_wheelbarrows() | ||
| set_wheelbarrows_for_all_stockpiles() | ||
| return | ||
| end | ||
| end | ||
| end | ||
|
|
||
| -- Event loop function to call periodically | ||
| local function event_loop() | ||
| if enabled then | ||
| dfhack.println("Running autowheelbarrow.lua ") | ||
| auto_wheelbarrows() | ||
| -- Check again in 7 days | ||
| repeatutil.scheduleUnlessAlreadyScheduled(GLOBAL_KEY, 7, "days", event_loop) | ||
| end | ||
| end |
There was a problem hiding this comment.
I don't understand the rationale for your timing. It looks to me as if you check every 7 days whether there is, at that very moment, a unit that has the StoreItemInStockpile job, and only call auto_wheelbarrows if that is the case.
There was a problem hiding this comment.
The reason for the timing of seven days is children decide to pick up a wheelbarrow and it can take them quite a bit of time to haul the wheelbarrow meaning if the child is hauling for a while this will cause the program to run again and unassign all of the wheelbarrows in the fort resulting in a loop causing less work to be done.
There was a problem hiding this comment.
The 7 days may be fine, even though it may be preferable to make this configurable. My question was more: why should the reassignment only occur if there is an active StoreItemInStockpile job.
| :summary: Automatically manage wheelbarrows in stockpiles based on conditions. | ||
| :tags: fort auto stockpiles | ||
|
|
||
| This script automates the assignment of wheelbarrows to stockpiles based on specific criteria, such as the type of items stored in the stockpile. Stockpiles with food, armor, or all flags set to ``false`` will not have wheelbarrows assigned. |
There was a problem hiding this comment.
That documentation should clearly identify the problem the script is trying to solve. In particular, it should answer the following questions:
- What is the deficiency of the automatic wheelbarrow assignment of DF that the script is addressing?
- How is the script overcoming the aforementioned deficiency?
| local count = unassign_wheelbarrows() | ||
| building.max_wheelbarrows = count |
There was a problem hiding this comment.
Doesn't calling unassign_wheelbarrows inside the loop mean that only the last valid stockpile will ever get wheelbarrows assigned to it?
There was a problem hiding this comment.
Yes you're right I should unassign all wheelbarrows before the loop.
|
Thanks you are right. I like that you took the time to review this. Once I get home I’ll implementa few changes. |
|
I forgot how to make changes in GitHub. So far, I want to make a single change, but there are likely other adjustments needed. I also want to add that I created this program and added it to the pull request because others may benefit as much as I do. The ability not to worry about the number of wheelbarrows assigned to individual stockpiles—since the program dynamically manages that—is a huge advantage. However, it can cause conflicts if the program also removes wheelbarrows from mixed stockpiles, such as food and stone, which I don't use in my world. If someone wishes to delete this pull request, take it over, or leave it in limbo, that's 100% fine with me. |
This part is easy: just push additional commits to the branch from which you created this PR.
Don't get defensive. The purpose of code review is to discuss simplifications and improvements. Except for trivial PRs, changes are almost always necessary, even for experienced developers. If you're not up for this, feel free to just close this pull-request. |
|
I'm not as committed as I thought I was if I have an idea I'll post it in the suggestion box. If I make something workable for the idea I'll post a video in the suggestion box and Sphegetti code. |
This script automatically manages wheelbarrows in Dwarf Fortress by adjusting their assignment to stockpiles based on what items the stockpile contains. Here's how it works:
Unassign Wheelbarrows: It checks all wheelbarrows in the game. If a wheelbarrow is assigned to a stockpile but not in use, it unassigns it.
Skip Certain Stockpiles: The script skips stockpiles that have specific items, like food, animals, furniture, weapons, etc. It also skips stockpiles that don’t contain anything at all.
Reassign Wheelbarrows: If the stockpile isn't skipped, the script assigns the right number of wheelbarrows to it based on the stockpile’s current state.
Automatic Checks: The script runs automatically every in-game day and rechecks wheelbarrows based on what dwarves are doing (specifically if they’re moving items into stockpiles).
Runs When Needed: The script starts automatically when the fortress map is loaded and stops when the map is unloaded.
This simplifies the management of wheelbarrows, ensuring they are assigned where needed and skipped for irrelevant stockpiles.