Skip to content

Conversation

@pajawojciech
Copy link

@pajawojciech pajawojciech commented Dec 2, 2025

Add search to manager orders

Adds search box to find orders by job name + arrow indicators to show which one you're looking at.

What it does

  • Search box appears on orders screen (Alt+S to focus)
  • Type keywords to filter (e.g. "iron picks" finds "forge iron picks")
  • Multi-word search works in any order ("picks iron" and "pi ir" also matches)
  • Alt+P/N to jump between matches. Also Enter and Shift+Enter works when focused
  • Shows "3 of 12" style counter
  • Arrow point at current match
  • Arrows hide when you: scroll view, edit search or add / remove from orders list

Implementation

  • Uses df::interface_button_building_new_jobst to get order name
  • Two overlays: OrdersSearchOverlay for search UI, OrderHighlightOverlay for arrows
  • Both enabled by default
  • Moved importexport overlay position to make room for search (and make new version of config)

What didn't work

  • Tried to clear search input when exiting orders window - couldn't find reliable way to detect window exit

Related issues

Dwarf.Fortress.2025-12-30.12-33-18.mp4

Adds search overlay to find and navigate manager orders with arrow indicators showing current search result. Search uses Alt+S to focus, Alt+P/N for prev/next navigation. Overlays are disabled by default.
Copy link
Contributor

@ChrisJohnsen ChrisJohnsen left a comment

Choose a reason for hiding this comment

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

This is a nice workaround for the inability to filter the view like in the other info panel search widgets.

I like the labeled magic numbers in OrderHighlightOverlay. DF's layout behavior is not fun thing to have to replicate, but I think this would be fairly clear to someone that hasn't looked at this stuff as much (at least as long as they have DF at hand to try out different window sizes).

Re: not being able to detect exiting the window: yeah, I have also wanted something
like a callback for "this overlay is no longer active (not going to be rendered)".


  • Would utils.search_text be useful here? The main SortOverlay uses it for searching.
  • Could the first match automatically be highlighted on Enter?
    I can see how you might not want to highlight (thus likely move the scroll position) for each change in search input, but Enter seems like a nice place to jump to the first match.
  • The highlight should probably be cleared when the search text changes (especially when the highlighted order does not match the new search input).
  • It might just be my color vision being flaky, but I completely did not notice the highlight arrow at first. Now that I know what to look for, it isn't hard to spot. But my first cycling through the matches was confusing because it wasn't obvious which order was being indicated.

@pajawojciech pajawojciech marked this pull request as draft December 6, 2025 13:06
@pajawojciech
Copy link
Author

* Would utils.search_text be useful here? The main SortOverlay uses it for searching.

I switched to using utils.search_text instead of the custom search logic

* Could the first match automatically be highlighted on Enter?

Added Enter/Shift+Enter to cycle through matches using the default submit/submit2 methods.

* The highlight should probably be cleared when the search text changes (especially when the highlighted order does not match the new search input).

Fixed, now the highlight gets cleared whenever the search text changes.

* It might just be my color vision being flaky, but I completely did not notice the highlight arrow at first. Now that I know what to look for, it isn't hard to spot. But my first cycling through the matches was confusing because it wasn't obvious which order was being indicated.

I reshaped the arrow, moved it to the right side of icon and used more contrasting colors (black on white) to make it more visible.
obraz


@ChrisJohnsen Thanks for all the detailed feedback! I made each change in a separate commit to make the review easier. Let me know if there's anything else that needs adjusting.

@pajawojciech pajawojciech marked this pull request as ready for review December 7, 2025 11:18
Copy link
Contributor

@ChrisJohnsen ChrisJohnsen left a comment

Choose a reason for hiding this comment

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

Good spotting of the need to hide when job_details.open. The other functionality changes all seem good (with a new note about order deletion handling).

I've put some more thoughts on the specifics of the code in this review.

@pajawojciech pajawojciech marked this pull request as draft December 8, 2025 19:15
@ChrisJohnsen
Copy link
Contributor

So, I hadn't looked into the details of collect_reactions from internal/quickfort/stockflow until now... I had assumed (yeah, my mistake) that its order fields were Lua tables that mimicked df::manager_order objects enough that (e.g.) make_order_key could use either a real order or a "mocked up" one.

But those order fields are actual df::manager_order objects (~3500 of them) that are allocated and filled out in (functions called by) collect_reactions . So if callers don't free them, they end up leaked. Since these objects aren't needed by this code after keys are derived from them, they should probably just be freed right away.

The other caller is internal/quickfort/orders; it accepts the leak and only calls collect_reactions once (storing it in a module global; disclaiming any need for world-varying or modded reactions).

@pajawojciech
Copy link
Author

pajawojciech commented Dec 13, 2025

So, I hadn't looked into the details of collect_reactions from internal/quickfort/stockflow until now... I had assumed (yeah, my mistake) that its order fields were Lua tables that mimicked df::manager_order objects enough that (e.g.) make_order_key could use either a real order or a "mocked up" one.

But those order fields are actual df::manager_order objects (~3500 of them) that are allocated and filled out in (functions called by) collect_reactions . So if callers don't free them, they end up leaked. Since these objects aren't needed by this code after keys are derived from them, they should probably just be freed right away.

The other caller is internal/quickfort/orders; it accepts the leak and only calls collect_reactions once (storing it in a module global; disclaiming any need for world-varying or modded reactions).

I added df.delete(reaction.order) to free the C++ objects and set reactions = nil afterward.

I'm not very familiar with memory leak issues, so please let me know if this fix looks correct or if there's anything else I should do.

1bdd533

@pajawojciech pajawojciech marked this pull request as ready for review December 13, 2025 12:24
@pajawojciech
Copy link
Author

pajawojciech commented Dec 13, 2025

I need advice on where to put stockflow collect_reactions():

Option 1: Use plugins\lua\stockflow.lua (needs changes like in pr 1524)

  • Used by the stockflow plugin
  • The plugin is currently disabled (commented out in CMakeLists.txt since v50)
  • I believe this plugin was designed to keep order names consistent with the game UI, so my changes will benefit stockflow plugin in future

Option 2: Use scripts\internal\quickfort\stockflow.lua (changes prepared in pr 1524)

  • Used by internal/quickfort/orders, but throws exceptions after pr 1524, because has its own translation logic. So internal/quickfort/orders needs fixes in this option

Option 3: Create a new stockflow.lua file

  • Problem: Where should I create it?

What approach would you recommend to get reactions?

EDIT: used option 4: make DF instead of stockflow create order names

Copy link
Contributor

@ChrisJohnsen ChrisJohnsen left a comment

Choose a reason for hiding this comment

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

I found couple of functionality things that I didn't notice before:

  • cache invalidation between world loads,
  • searchability after setting material for an order.

While looking at the details of customized orders I wondered if the overall size of the cache can be reduced.

Some functions can be made local (they don't access self, or won't once the methods the call are themselves made local):

  • perform_search
  • getListStartY
  • getViewportSize
  • calculateSelectedOrderY

When adding/removing an order there is still a small UI bug: the "Search: X of Y" title remains unchanged even if a matching order is added or removed. You could refresh the search when the number of orders changes, or just revert to the plain "Search" title so that the (possibly) stale count isn't displayed.

@ChrisJohnsen
Copy link
Contributor

I need advice on where to put stockflow collect_reactions():

Option 1: Use plugins\lua\stockflow.lua (needs changes like in pr 1524)

I don't think there is a reason to try to use this version. Like you noted, the plugin is disabled. Its Lua code includes stuff that you don't need here.

Option 2: Use scripts\internal\quickfort\stockflow.lua (changes prepared in pr 1524)

* Used by internal/quickfort/orders, but throws exceptions after pr 1524, because has its own translation logic. So internal/quickfort/orders needs fixes in this option

I haven't really looked at scripts#1524 much yet. If it is causing problems for Quickfort, that will surely need to be addressed though. Quickfort is the only existing user of internals/quickfort/stockflow it wouldn't be appropriate to break it to make the new order search work.

I don't know which exceptions you have seen from Quickfort, but it might be possible to make fewer changes to stockflow (keeping Quickfort working without changes) and make more adaptations in the order searching code (like extending "make earring" to "make earring willow" instead of making stockflow generate an additional "Make willow Earring" order).

Option 3: Create a new stockflow.lua file

Another copy of the code (e.g., with the scripts#1524 changes) probably isn't be best approach, if that's what you mean.

It does seem the approach here of matching pre-described orders is kind of brittle. I suppose quickfort only needs to concern itself with the list of items/orders that it supports placing, so it doesn't need to support all possible user-configurable, mod-enabled, world-varying orders.

If there was some other approach for recreating the UI text DF would display for any given order, that might work better than trying to match order attributes against a predefined, cached list. That would probably take some reverse engineering work to suss out what all DF considers when rendering the order names.

@pajawojciech pajawojciech marked this pull request as draft December 17, 2025 17:26
@pajawojciech
Copy link
Author

pajawojciech commented Dec 30, 2025

If there was some other approach for recreating the UI text DF would display for any given order, that might work better than trying to match order attributes against a predefined, cached list. That would probably take some reverse engineering work to suss out what all DF considers when rendering the order names.

Thanks for the suggestion, I used workshop button trick to make this works. Looks good and fast 👍

@pajawojciech pajawojciech marked this pull request as ready for review December 30, 2025 11:51
@ChrisJohnsen
Copy link
Contributor

Interesting.

I see that the similar DFHack::Job::getName is already used in scripts/do-job-now.lua and plugins/lua/spectate.lua. It does seem to work well. It produced the matching text for everything I tried (existing orders of a couple of forts, plus "assemble instrument" (for some non-ASCII characters), new-fangled dye mixing, gem encrusting, specified-wood/bone/hair/yarn, differently-sized clothing). It does seem fast enough (3.5µs per query when I benched 2 million calls).

Despite the similarity with getName I wonder if it belongs in DFHack::Job. Though I don't see a better place in the existing code (nothing that specializes in manager_order)… I don't know if a new namespace (DFHack::ManagerOrder?) would be appropriate for a single function.

For fun I also benched a Lua-only implementation. It is slower but not so slow that I noticed any problems in interactive use (55µs per query in a bench of a quarter million; ~35µs after some tweaking). So it could conceivably live on the Lua side if there isn't a good place for it on the C++ side.

I noticed the addition of setting button->art_specifier as compared to getName. Was that required for a particular kind of order or just added for completeness in matching up fields between manager_order and interface_button_building_new_jobst? I wonder if getName should adopt that, too.

This last niggle also exists in getName, but I wonder about copying order->specdata.hist_figure_id and then also order->specdata as a whole… It looks like hist_figure_id covers the entire union, but assigning the whole thing seems more general. I guess it shouldn't hurt, but the first seems unnecessary?


There are a couple of small UI bugs:

  1. When a search is active (showing "Search: X of Y") and the player scrolls the list it still shows "Search: X of Y" even though the highlight is no longer drawn.

    It should probably revert to "Search" or "Search: Y matches"?

  2. When a search is active (showing "Search: X of Y") and the player adds (or removes) a matching order it still shows "Search: X of Y".

    It should probably revert to "Search", or "Search: Z matches" (new Z value reflecting new number of matches), or "Search: W of Z" (might need to take care with deleting the last match).

button->specflag = order->specflag;
button->job_item_flag = order->material_category;
button->specdata = order->specdata;
button->art_specifier = order->art_spec.type;
Copy link
Contributor

@ChrisJohnsen ChrisJohnsen Jan 1, 2026

Choose a reason for hiding this comment

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

This isn't in getName. Was is needed for a particular kind of order?

If so, maybe getName should adopt it, too.

I didn't notice any orders that seemed to incorporate job_art_specifier_type details in the UI text, but I could easily have missed something.

std::string desc;
auto button = df::allocate<df::interface_button_building_new_jobst>();
button->mstring = order->reaction_name;
button->specdata.hist_figure_id = order->specdata.hist_figure_id;
Copy link
Contributor

@ChrisJohnsen ChrisJohnsen Jan 1, 2026

Choose a reason for hiding this comment

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

For confirmation by someone that works more on the C++ side:

This may not be necessary since specdata is copied as a whole later?

It seemed to work for me when I leaving this one off (copying one of them was definitely needed for some orders (e.g. differently-sized clothes)).

Copy link
Contributor

@ChrisJohnsen ChrisJohnsen left a comment

Choose a reason for hiding this comment

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

Here is a review to tick the requested-review box. My previous comment also mentions a couple of remaining UI bugs.

@ChrisJohnsen
Copy link
Contributor

Found a leftover while reviewing my local notes:

If getManagerOrderName ends up in the C++-exported Lua API (whether it ends up in Jobs or elsewhere), it should probably get an entry in docs/dev/Lua API.rst (similar to the one for dfhack.job.getName).

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.

4 participants