Skip to content

orders: add position management commands and overlay - #5891

Open
Robuske wants to merge 3 commits into
DFHack:developfrom
Robuske:better-orders-lua-integration
Open

orders: add position management commands and overlay#5891
Robuske wants to merge 3 commits into
DFHack:developfrom
Robuske:better-orders-lua-integration

Conversation

@Robuske

@Robuske Robuske commented Sep 1, 2026

Copy link
Copy Markdown

Summary

Adds direct manager-order position management to orders:

  • orders positions [--show-id] lists fort-wide manager orders in displayed priority order.
  • orders move <current-position> <new-position> [--show-id] moves the existing order pointer within world.manager_orders.all.
  • orders.position adds inline position fields to the Work Orders screen and uses the same move logic as the command.
  • Shares Work Orders list geometry between the existing search overlay and the new position overlay.
    • The existing search result indicator was moved a bit to avoid overlap (see second screenshot).
  • Adds documentation, a Future changelog entry, and contributor metadata.

The native orders plugin dispatches the new subcommands to Lua. Expected validation failures are returned normally, avoiding the extra Failed Lua call message.

Testing

  • Built the orders plugin with the DFHack Visual Studio build.

  • Ran:

    test -d C:/Users/rodbu/Developer/Dwarf/dfhack/test -t orders
    

    Result: 18/18 tests passed, 227/227 checks passed.

  • Performed in-game testing of command moves, validation errors, inline editing, cancellation, focus handling, scrolling, and search-overlay interaction.

Normal Editing + Search result indicator
Screenshot 2026-09-01 192827 Screenshot 2026-09-01 192906

Documentation

Includes orders command and orders.position overlay documentation, plus the required docs/changelog.txt entry.

@chdoc

chdoc commented Sep 4, 2026

Copy link
Copy Markdown
Member

Before I spend a lot of time reviewing the code in detail, let me first ask a few high-level questions and make a few high-level comments:

  1. What is the motivation for this change? I have never actually felt the need to reorder work orders. The only reason I can see to reorder work orders is to avoid low frequency work orders being starved by high frequency work orders. That is the purpose of orders sort.
  2. Assuming I can be convinced that this has general utility, the described workflow of selecting a work order and then entering a number where it should be placed it does not sound appealing. If I wanted to sort orders, I would appreciate label buttons to move the selected order (we can select orders) one up, one down, to the top, or to the bottom. Has this been considered?
  3. I cannot see a small UI/UX improvement like this requiring a change of roughly 900 additional lines plus another roughly 400 lines of tests. This seems over-engineered and gives me strong AI code vibes. Please give me a description in your own words (that is without the use of AI) of the general idea behind the implementation and why you think that this cannot be achieved using a smaller, more focused change.
  4. In fact, some of the changes look like you are refactoring the tool at the same time as you are changing features. Please keep refactoring pull requests and feature pull requests separate. Is just because it caught my eye: Replacing self.subviews.filter with self.subviews.[SEARCH_FILTER_VIEW_ID] is an absolute no go, because it reduces readability for no gain whatsoever.

@Robuske
Robuske force-pushed the better-orders-lua-integration branch from 43e4ee3 to f97d81a Compare September 5, 2026 01:46
@Robuske

Robuske commented Sep 6, 2026

Copy link
Copy Markdown
Author

@chdoc

  1. What is the motivation for this change? I have never actually felt the need to reorder work orders. The only reason I can see to reorder work orders is to avoid low frequency work orders being starved by high frequency work orders. That is the purpose of orders sort.

Really? Maybe there is something wrong with my understanding or how I play, but can't repeating orders starve each other?
If I already had a repeating order for making steel armor, and add a new one for steel swords, won't the steel armor have priority? If I want the swords to have priority, I will have to bump its order up one by one in the list

  1. Assuming I can be convinced that this has general utility, the described workflow of selecting a work order and then entering a number where it should be placed it does not sound appealing. If I wanted to sort orders, I would appreciate label buttons to move the selected order (we can select orders) one up, one down, to the top, or to the bottom. Has this been considered?

I am a little confused by this question, the game already has an option for moving one up or down, what I added is a way to move a arbitrary number, including to the top or bottom if you want, you just use the 1 for the top and whatever the last order is for bottom. Maybe what I have implemented isn't very clear?

  1. I cannot see a small UI/UX improvement like this requiring a change of roughly 900 additional lines plus another roughly 400 lines of tests. This seems over-engineered and gives me strong AI code vibes. Please give me a description in your own words (that is without the use of AI) of the general idea behind the implementation and why you think that this cannot be achieved using a smaller, more focused change.

Oh yeah, the "core" of this change is basically a few lines, removing an order from the array and moving it to another position, but:

  1. It includes "cli" commands instead of just the overlay, it seemed to me that DFHack usually includes both
  2. Giving a good UX to text fields is a pain in the ass, it deselects if you right click, press esc, click a random place, click another field, enter some shortcuts, etc...

To be clear I have used AI, I am an experienced iOS developer, but I only have a basic knowledge of C++ and none of Lua so it was basically vibe coded, that said I did check if the code "seemed reasonable" and tested the experience a lot, if it is overengineered it is more my own fault than the AI's (with exception of the tests, I admit that I didn't do much besides running them)

  1. In fact, some of the changes look like you are refactoring the tool at the same time as you are changing features. Please keep refactoring pull requests and feature pull requests separate. Is just because it caught my eye: Replacing self.subviews.filter with self.subviews.[SEARCH_FILTER_VIEW_ID] is an absolute no go, because it reduces readability for no gain whatsoever.

One of the text fields UX issues I have run into is selecting/deselecting the correct field, by default multiple fields would stay selected. The search field is the only other text field in that screen, so instead of searching the subviews "manually" for controlling focus, it was easier to define an ID to access it.
The refactoring is just an extraction of what both overlays have in common, mostly the code for following the orders that the search highlight and this new position overlay use. I can move it to another PR, but this refactor doesn't really make sense without the position overlay introduction, so I didn't really see the reason to do that.

I am just explaining why I did the things I did, like I said, I am an absolute noob when it comes to Lua or DFHack itself, it is very likely that there are better ways to do parts of what I am doing (or maybe there is something that you just prefer done another way), just let me know and I can fix it.

@chdoc

chdoc commented Sep 6, 2026

Copy link
Copy Markdown
Member

Really? Maybe there is something wrong with my understanding or how I play, but can't repeating orders starve each other? If I already had a repeating order for making steel armor, and add a new one for steel swords, won't the steel armor have priority? If I want the swords to have priority, I will have to bump its order up one by one in the list

My understanding is that every work order will only generate at most one job per applicable workshop. That is, unless you are very short on steel or have an extremely large number of forges, one order starving another is actually pretty rare.

I am a little confused by this question, the game already has an option for moving one up or down, what I added is a way to move a arbitrary number, including to the top or bottom if you want, you just use the 1 for the top and whatever the last order is for bottom. Maybe what I have implemented isn't very clear?

Apologies, that question is merely an indication that in hundreds of hours of playing DwarfFortress I never felt the need to reorder work orders so I never used those buttons or knew they existed. Still, I would expect a button to toggle the overlay (with the position indicators, this screen feels cluttered to me), and buttons to move things to the top or the bottom. If this positioning can be computed reliably, then maybe putting them to the left or the right of the central column might make sense?

Oh yeah, the "core" of this change is basically a few lines, removing an order from the array and moving it to another position, but:

1. It includes "cli" commands instead of just the overlay, it seemed to me that DFHack usually includes both

2. Giving a good UX to text fields is a pain in the ass, it deselects if you right click, press esc, click a random place, click another field, enter some shortcuts, etc...

One of the main reasons of CLI-tools existing in addition to GUI-tools is the CLI-tool predating the GUI-tool. Another is a command you might reasonably want to run automatically (e.g. scripting). The third is performance considerations. I'm not sure any of that applies here. I mean, you basically have to have the window open to get the numbers. I don't see any point in listing the orders by number on the command line and then fiddling with them there. That sounds extremely cumbersome, I'm not sure why anyone would want to do this. Don't add CLI-tools just for the sake of adding CLI-tools.

To be clear I have used AI, I am an experienced iOS developer, but I only have a basic knowledge of C++ and none of Lua so it was basically vibe coded, that said I did check if the code "seemed reasonable" and tested the experience a lot, if it is overengineered it is more my own fault than the AI's (with exception of the tests, I admit that I didn't do much besides running them)

I don't have anything against using AI in general. However, I am very cautious about even starting to do an in-depth review of code that was written by people who, by their own admission, do not really know the language the code is written in. It is a requirement to be willing and able to engage in a meaningful discussion about the design of the change. You have engaged in a discussion instead of adopting a "take it or leave it" attitude. Not everyone does. So we are good here.

One of the text fields UX issues I have run into is selecting/deselecting the correct field, by default multiple fields would stay selected. The search field is the only other text field in that screen, so instead of searching the subviews "manually" for controlling focus, it was easier to define an ID to access it. The refactoring is just an extraction of what both overlays have in common, mostly the code for following the orders that the search highlight and this new position overlay use. I can move it to another PR, but this refactor doesn't really make sense without the position overlay introduction, so I didn't really see the reason to do that.

Please try to stay away from refactoring as much as possible, and please try to minimize the amount of lines changed (every line changed is a line I need to review). In particular because this is your first contribution.

For this manual tinkering with order positions, I think you can safely drop the command-line part. I don't see any reason for this to touch the C++ part at all.

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.

2 participants