Add functions reverse-engineered from announcement code
Added autoDFAnnouncement, which takes a report_init and a string, and handles them like DF does. (E.g., handling repeats and ignoring those in unrevealed map.)
Added pauseRecenter, which recenters on an xyz coord and optionally pauses, while respecting pause_zoom_no_interface_ms.
~~Added recenterViewscreen, which recenters on an xyz coord using a report zoom style (Generic, Item, or Unit. Existing revealInDwarfmodeMap function names "Unit" style "bool center". Generic style ignores coords and just enforces valid view bounds.)~~ Just replace revealInDwarfmodeMap with new code.
~~Added parseReportString, which parses a string using '&' as a control character (&r as two newlines, && as &) and cuts to a certain length w/o splitting words.~~ Reduced this to a utility function, unlikely to be needed outside gui.cpp.
~~Changes can be made to existing functions based on information gained. Will address in separate PR.~~ Might as well address here.
I'm not entirely sure "recenterViewscreen" uses "report_zoom_type", but it's the closest enum I could find that makes sense.
From what I can tell, zoom_type1 and zoom_type2 in reports are always left at 0 (report_zoom_type::Generic) and are ignored by the game. They aren't used during the initial announcement, which calls pauseRecenter, which always calls recenterViewscreen with value 1 (which corresponds to report_zoom_type::Item if it's the correct enum.) Pressing 'z' in game when highlighting a report isn't affected by changing these either. So "report_zoom_type" isn't actually used by DF reports (anymore.)
I'm not sure how the report_zoom_type enum was determined, but I would think "Unit" and "Item" would be swapped if that's what recenterViewscreen uses, since following a unit in game uses the lazy style that doesn't put them in the exact center, while zooming to an item from stocks screen centers you on it. That's the only thing that makes me suspicious of it.
could you merge latest develop? the diff seems to have gone wonky.
You might need to merge latest develop to get the tests to pass. It looks like your structures and scripts HEADs are different from upstream.
It looks like your library/xml and scripts are still misaligned. Could you fix those up? You might have committed a change to which commit those submodules point to that you have to revert.
Overall, code looks good. Thanks for getting that submodule thing figured out!
I'd be a lot more comfortable with this if it had some fortress-mode unit tests in https://github.com/DFHack/dfhack/blob/develop/test/gui.lua (or a new sibling file since that one is currently
config.mode = 'none')
Is this something that needs to be done before merging the PR? I don't have much experience with unit testing.
It's really not hard. It's like regular coding. It's asking the question "what do I expect to happen when I call this function?" Then you call the function and check that it happened.
Go to your cmake settings and set BUILD_TESTS to install the test harness. It's just a regular script named "test".
In this case, when you run the test command, the test harness will load the save in region1 (if a save is not already loaded) and run all tests. Each of your tests should set the viewport to some initial state, run one of the new functions, and then test to make sure the viewport is now set as you expect. Since the functions may change behavior based on window size, the unit tests have to be flexible and test ranges, not exact values.
Here's an end-to-end guide:
- Build with BUILD_TESTS=ON to install the test harness
- Create a file named
test/gui_viewport.luawith the following contents:
config.mode = 'fortress'
function test.zoom_to_unit()
-- set viewport to some initial location
-- run dfhack.gui. function
-- test viewport has changed as we expect
end
- ninja install (or however you usually build and install)
- DFHack# test -t gui_viewport
Look at other tests in the test tree for examples. The expect functions used to verify your expectations are defined in https://github.com/DFHack/dfhack/blob/develop/library/lua/test_util/expect.lua
Do we have a special region1 save for doing tests on? I see test_save.tgz mentioned in build logs.
Yeah, you can download it from here, but really the tests should work on any fortress save. If they don't work on a save you already have available, it's probably a problem with the test.
Thanks. Looks like a 1x1 embark, which might be inadequate for testing viewport scrolling.
Thanks. Looks like a 1x1 embark, which might be inadequate for testing viewport scrolling.
The "official" tests are run at 80x25, which does allow for scrolling even in a 1x1 (though the tests should still pass even if the dev who runs the tests happens to have such a large screen that the map doesn't actually scroll, but that could be the case no matter how large an embark we use). If you need a larger embark to get any value, though, I can definitely make that happen. I just used a 1x1 embark because it gave the smallest file size so our build runs faster.
Sorry for the long delay!
Sorry for the long delay!
FYI, I never made any progress on the unit tests. I went on hiatus due to selling house and moving out of state.
I understand. I just didn't want the work you did to get lost, especially now that a new release is coming out