Skip to content

Conversation

@brmley
Copy link

@brmley brmley commented May 15, 2025

The script to show more information about scholars activities, adapted for use with spectate-overlay or dfhack-console

@ab9rf
Copy link
Member

ab9rf commented Aug 16, 2025

I haven't moved forward to review this PR because the pre-commit check hasn't been resolved. Is the OP still interested in pursuing this PR?

Copy link
Member

@chdoc chdoc left a comment

Choose a reason for hiding this comment

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

Since this tool can be invoked on its own from the command-line, it needs documentation. Without having a good understanding what this tool is supposed to do, it is hard to give good feedback. Please have a look at the .rst files in the docs folder. And then, of course, this also needs a changelog entry like every other PR.

goto continue
end

local historical_figure = researchInfo.getHistoricalFigure(unit.id)
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
local historical_figure = researchInfo.getHistoricalFigure(unit.id)
local historical_figure = df.historical_figure.find(unit.hist_figure_id)

There are two observations here: units have a historical figure id field, and the idiomatic way to turn a numeric field into the corresponding structure is to use df.<structure_name>.find(id). This makes functions like your getHistoricalFigure basically useless. Please revisit your code and see whether this same pattern can be applied elsewhere.

Comment on lines +41 to +44
local unit = dfhack.units.getCitizens()
local selected_unit = dfhack.gui.getSelectedUnit()

if unit then
Copy link
Member

Choose a reason for hiding this comment

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

dfhack.units.getCitizens returns a table, and even the empty table (which this will never return in an active fort) is a "truthy" value in lua. So this condition can never evaluate to false.


local parts = {}
for part in str:gmatch("[^_]+") do
-- Первую букву делаем заглавной, остальные - строчными
Copy link
Member

Choose a reason for hiding this comment

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

If you add comments, please add them in English.

Comment on lines 43 to 50
local goal_info = ""
for i = 0, 31 do
if goal[i] then
local global_flag_index = category * 32 + i -- wizards move
goal_info = string.format("%s", df.dfhack_knowledge_scholar_flag[global_flag_index]) or string.format("Unknown Flag: %s", i)
end
end
return goal_info
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand what you're doing here. Please explain, so that we can see whether there is a better way.

Copy link
Author

Choose a reason for hiding this comment

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

This func returns name of researchable topic

it takes data from https://github.com/DFHack/df-structures/blob/e4993ceef7907351fd2ac6f9bd3a9e5ff24699db/df.history_figure.xml#L308 and match value with name from https://github.com/DFHack/df-structures/blob/e4993ceef7907351fd2ac6f9bd3a9e5ff24699db/df.dfhack.xml#L221 there is a trouble in relations between these structures and these is doesnt match directrly. So, i use some magic.

@brmley brmley closed this Sep 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants