dfhack icon indicating copy to clipboard operation
dfhack copied to clipboard

Add functions reverse-engineered from announcement code

Open Bumber64 opened this issue 3 years ago • 10 comments

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.

Bumber64 avatar Apr 25 '22 07:04 Bumber64

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.

Bumber64 avatar Apr 30 '22 07:04 Bumber64

could you merge latest develop? the diff seems to have gone wonky.

myk002 avatar May 30 '22 06:05 myk002

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.

myk002 avatar Jun 05 '22 02:06 myk002

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.

myk002 avatar Jun 06 '22 00:06 myk002

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.

Bumber64 avatar Jun 11 '22 19:06 Bumber64

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:

  1. Build with BUILD_TESTS=ON to install the test harness
  2. Create a file named test/gui_viewport.lua with 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
  1. ninja install (or however you usually build and install)
  2. 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

myk002 avatar Jun 11 '22 21:06 myk002

Do we have a special region1 save for doing tests on? I see test_save.tgz mentioned in build logs.

Bumber64 avatar Jun 19 '22 02:06 Bumber64

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.

myk002 avatar Jun 19 '22 06:06 myk002

Thanks. Looks like a 1x1 embark, which might be inadequate for testing viewport scrolling.

Bumber64 avatar Jun 19 '22 06:06 Bumber64

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.

myk002 avatar Jun 20 '22 22:06 myk002

Sorry for the long delay!

myk002 avatar Nov 19 '22 01:11 myk002

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.

Bumber64 avatar Nov 23 '22 03:11 Bumber64

I understand. I just didn't want the work you did to get lost, especially now that a new release is coming out

myk002 avatar Nov 23 '22 03:11 myk002