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

Show geld status and custom profession in animal zone assignment window #4182

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

neumond
Copy link
Contributor

@neumond neumond commented Jan 16, 2024

Screenshot from 2024-01-16 18-38-36

If player doesn't use custom profession field, nothing will change for them. Geld status sorts along with gender as a composite key (gender, is_gelded).

plugins/lua/zone.lua Outdated Show resolved Hide resolved
@@ -562,16 +575,21 @@ function AssignAnimal:cache_choices()
for _, unit in ipairs(df.global.world.units.active) do
if not is_assignable_unit(unit) then goto continue end
local raw = df.creature_raw.find(unit.race)
local desc = dfhack.units.getReadableName(unit)
if type(unit.custom_profession) == 'string' and #unit.custom_profession > 0 then
desc = unit.custom_profession .. ', ' .. desc
Copy link
Member

Choose a reason for hiding this comment

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

adding the custom profession before the name looks a little strange, IMHO. Adding at the end, after the readable name, would be better, but not ideal.

We could make this a feature of all tools that use getReadableName by integrating the custom profession name into there, perhaps added just before the race name. That would have to be accompanied by an audit of all tools that use that call (not too many -- getReadableName was added within the past year) and ensuring that information isn't duplicated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really sure that custom profession should appear everywhere through getReadableName. sortoverlay.lua uses getReadableName + getProfessionName as a search key. Animal stockpiles are a special case, where profession doesn't really bear any meaning, never gets assigned by game and gets used as just another field for tagging. Probably adding an option is better:

string Units::getReadableName(df::unit* unit, bool include_custom_profession = false)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another option is to attempt to split by ", " in lua code, to insert custom profession after the name.

Although I agree this is little strange, that profession goes before the name, but that's the info I care about the most, since I type this manually. Game-generated names of pets are just random gibberish that doesn't help to pick animals for zones.

Copy link
Member

Choose a reason for hiding this comment

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

That is a fair point. For the people who do use professions to categorize their livestock, they'll want it first so it can determine the sort order. Let me load this into my game and see what it looks like with prisoners and citizens and such

plugins/lua/zone.lua Outdated Show resolved Hide resolved
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.

Ok, you convinced me on the string ordering : )

Could you add a note to the changelog about this improvement? https://github.com/DFHack/scripts/blob/master/changelog.txt#L51

@myk002
Copy link
Member

myk002 commented Jan 17, 2024

Thank you for this. In a separate PR, please feel free to add your name to our list of Authors!
https://github.com/DFHack/dfhack/blob/develop/docs/about/Authors.rst

@myk002 myk002 merged commit 675be08 into DFHack:develop Jan 17, 2024
14 checks passed
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