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

new tool: immortal-cravings #1301

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

chdoc
Copy link
Member

@chdoc chdoc commented Sep 12, 2024

allow immortals to satisfy their cravings for food and drink

@myk002
Copy link
Member

myk002 commented Sep 13, 2024

this is an awesome name : D

however, I am seeing a parallel with the concept of idle-crafting I think it would be better to combine those tools into one script, and as we add more kinds of self-service autonomy, they can become features of the unified script instead of creating a new script for each one.

Would it make sense to rename idle-crafting to autonomy or i-want-it-i-do-it or something that encompasses these related ideas?

@chdoc
Copy link
Member Author

chdoc commented Sep 15, 2024

I'm not entirely sure. There obviously are parallels, but idle-crafting is (essentially) an automation tool while this one is clearly in the "gameplay" category. From the implementation perspective, I would have to think about where the synergies really are - apart from a shared main loop where units with needs get picked up. I don't see a natural place to configure this tool using an overlay.

From the communication perspective, idle-crafting has just been announced as a feature and received a lot of positive attention. I'm not sure immediately renaming it is a good idea.

From the development perspective, every need that we satisfy using a tool requires some amount of research and discussion. And other needs may require entirely different techniques. So maybe it is good to keep this thing separate for now.

Ultimately, I expect idle-crafting and related tools to be re-implemented as a C++ plugin at some point. idle-crafting currently takes about 0.7% of CPU time in my forts. Not an immediate problem, but still consuming about 7% of the total DFHack allowance. This tools time usage is hardly measurable in forts with one or two resident necromancers.

changelog.txt Outdated
@@ -31,6 +31,7 @@ Template for new versions:
- `idle-crafting`: allow dwarves to independently satisfy their need to craft objects
- `gui/family-affairs`: (reinstated) inspect or meddle with pregnancies, marriages, or lover relationships
- `notes`: manage map-specific notes
- `immortal-cravings`: allow immortals to satisfy their cravings for food and drink
Copy link
Member

Choose a reason for hiding this comment

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

could you move this up into the new "Future" section?

Comment on lines 17 to 18
``enable immortal-cravings``
``disable immortal-cravings``
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
``enable immortal-cravings``
``disable immortal-cravings``
::
enable immortal-cravings

Copy link
Member

Choose a reason for hiding this comment

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

we don't usually individually document the disable command

Copy link
Member

Choose a reason for hiding this comment

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

could you also add this tool to the control panel registry (internal/control-panel/registry.lua)?

---@param p1 df.coord
---@param p2 df.coord
---@return number
function distance(p1, p2)
Copy link
Member

Choose a reason for hiding this comment

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

this seems like a good candidate to move to utils. no need to do it yet, but if/when another Lua tool requires this, we should remember that this is here

Comment on lines 27 to 28
local x, y, z = dfhack.items.getPosition(item)
local pitem = xyz2pos(x, y, z)
Copy link
Member

Choose a reason for hiding this comment

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

you can combine this into

local pitem = xyz2pos(dfhack.items.getPosition(item))

Comment on lines 130 to 132
watched = {}

threshold = -9000
Copy link
Member

Choose a reason for hiding this comment

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

these should be local or "protected" globals (foo = foo or default_foo). otherwise, they'll get reset whenever the player runs (probably mistakenly) immortal-cravings

Copy link
Member

Choose a reason for hiding this comment

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

threshold should definitely be local since it's read only. watched could be either

local kept = {}
for _, unit_id in ipairs(watched) do
local unit = df.unit.find(unit_id)
if unit and not (unit.flags1.caged or unit.flags1.chained) then
Copy link
Member

Choose a reason for hiding this comment

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

opportunity to reduce code nesting here

        if not unit or unit.flags1.caged or units.flags1.chained then
            goto next_unit
        end

Copy link
Member

Choose a reason for hiding this comment

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

also, unit should be skipped if dead or not active (e.g. left map on a raid)

for _, need in ipairs(unit.status.current_soul.personality.needs) do
if need.id == DrinkAlcohol and need.focus_level < threshold then
goDrink(unit)
goto next_unit
Copy link
Member

Choose a reason for hiding this comment

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

these gotos could be replaced with breaks


---main loop: look for citizens with personality needs for food/drink but w/o physiological need
local function main_loop()
print('immortal-cravings watching:')
Copy link
Member

Choose a reason for hiding this comment

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

this prints in this function are probably too spammy. the prints for when actions are actually taken is enough

--- Handles automatic loading
dfhack.onStateChange[GLOBAL_KEY] = function(sc)
if sc == SC_MAP_UNLOADED then
enabled = false
Copy link
Member

Choose a reason for hiding this comment

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

maybe add a comment here explaining that repeat-util will cancel the scheduled callbacks for you

@chdoc
Copy link
Member Author

chdoc commented Oct 2, 2024

It may take until the next weekend for me to get back to this. Busy at work and other plans for the upcoming weekend.

@myk002
Copy link
Member

myk002 commented Oct 2, 2024

no hurry! I have plenty of other PRs to keep me busy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Review In Progress
Development

Successfully merging this pull request may close these issues.

2 participants