orders: add position management commands and overlay - #5891
Conversation
|
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:
|
43e4ee3 to
f97d81a
Compare
Really? Maybe there is something wrong with my understanding or how I play, but can't repeating orders starve each other?
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?
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:
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)
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. 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. |
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.
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?
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.
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.
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. |
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 withinworld.manager_orders.all.orders.positionadds inline position fields to the Work Orders screen and uses the same move logic as the command.The native
ordersplugin dispatches the new subcommands to Lua. Expected validation failures are returned normally, avoiding the extraFailed Lua callmessage.Testing
Built the
ordersplugin with the DFHack Visual Studio build.Ran:
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.
Documentation
Includes
orderscommand andorders.positionoverlay documentation, plus the requireddocs/changelog.txtentry.