Add game information report and SDL screenshot (#2) #30313

neitsa · 2019-05-08T00:09:52Z

Summary

SUMMARY: Features "Add game information report and SDL screenshot."

Purpose of change

(Note: this is a replay of #30244 after having messed up my branch)

Close Provide the ability to copy relevant version & config info from game. #30062

Describe the solution

Following Night-Pryanik and DracoGriffin suggestions the debug menu is visible from the main menu under some circumstances:

if (and only if) the debug menu has a bound key, then the whole menu is visible from the main menu:

If the debug menu has no bound key then only a specific portion of the menu is shown:

Add a "Generate Game Report" option in the Debug > Info menu.

The report , as of now, looks like this:

- OS: Windows [64-bit]
- Game Version: 0.D-2546-g2ac90b3f96-dirty
- Graphics Version: Tiles
- Mods loaded: [
    Dark Days Ahead [dda],
    Disable NPC Needs [no_npc_food],
    Filthy Clothing [filthy_morale],
    Generic Guns [generic_guns],
    Icecoon's Arsenal [ew_pack],
    Modular Turrets [modular_turrets],
    Salvaged Robots [Salvaged_Robots],
    Crazy Cataclysm [crazy_cataclysm],
    Alternative Map Key [alt_map_key],
    Mapgen Demo [mapgen_demo],
    Disable Religious Texts [no_religious_Texts],
    No Fungal Monsters [No_Fungi],
    Classes and Scenarios Mod [more_classes_scenarios],
    Manual Bionic Installation [manualbionicinstall],
    Safe Autodoc [safeautodoc],
    StatsThroughSkills [StatsThroughSkills]
]

Except for the mod list, all the other information are collected through compiler defines, it's obviously not perfect but:

it does not rely on an external lib.
It's almost a no-brainer and doesn't require reinventing the wheel

The above report is written to the debug.log file and in the clipboard (for SDL).

Add a "Save screenshot" option in the debug menu.

I intended to do it for bug reporting, but while coding it I thought that taking a screenshot while the process heap might be in a bad shape was a probably bad idea. The code was there so I left it.

The screen shot is basically a copy of the SDL viewport (so, not the screen, just the game view) translated into a PNG. The file is saved into the game save world, into a "screenshots" folder.

So if your current workd is named Amidon the screenshots are in /save/Amidon/screenshots.

The naming scheme of the screenshots is: [<character_name>]_YYYY-MM-DD_HH-MM-SS_<GMT_OFFSET>

So for example:

[Julian 'Fish' Willis]_2019-05-04_15-49-08_+0100

The date format is similar to ISO 8601, except for some characters that are replaced because they are not allowed on some file systems.

I also added a function (filesystem.[h,cpp]) that filters possible invalid characters from the player name when building the filename.

Describe alternatives you've considered

N/A.

Additional context

In the end I'd like to do a semi-automatic reporting:

include debug.log (with the above game info report)
include screenshot at time of crash?
include game save file
include meaningful stack trace
include meaningful dump / core.
zip everything

I'd also like to get more information about the system, like the distribution and its version for Unices (e.g. Ubuntu 18.04) and the Major and Minor versions for Windows (e.g. Windows 10 1809).

- check for nullptr - check for no loaded mods - Idiomatic c++

- Check if the SDL copy to clipboard function error-ed or not. - Manual astyle...

- Remove check on world validity. The world is already playing at this point.

- Name it take_screenshot() in game.[h; cpp]. - This is the only location where we can access the SDL renderer (except catatiles.cpp but it's really more meaningful in sdltiles.cpp).

- Reorder menu.

neitsa · 2019-05-08T17:00:46Z

Integrated all suggestions from #30244. I think it's ready for review now.

neitsa · 2019-05-08T17:05:13Z

Side note: for an unknown reason both Astyle 3.1 and 3.0.1 (windows binaries) fail to format the very end of the debug_menu.cpp file... I couldn't understand why they both failed and I had to format everything manually.

- Remove unused header.

- It is fine to have it outside since it returns false if SDL/TILES are not used.

- Otherwise the code in game.cpp can't find it. - It returns false if TILES is not defined - Remove obsolete function comment.

- As sdltiles is the only file where the renderer can be accessed, but obviously is only usable when TILES is defined, you can only use save_screenshot() if SDL is defined. - Make it so that game::take_screenshot() returns false if TILES is not defined.

…s... - Make a global context capture.

neitsa · 2019-05-09T15:26:34Z

It compiles :) there's an error on a test but this PR didn't touch any core game components.

codemime

Overall looks good.

src/debug.cpp

codemime · 2019-05-09T19:23:05Z

src/debug.cpp

+        return string_format( "%s [%s]", mod->name(), mod->ident.str() );
+    } );
+
+    auto result = join( mod_names, ",\n    " );


What's the point in the trailing blanks?

4 spaces before the mod name:

- Mods loaded: [ Dark Days Ahead [dda], Disable NPC Needs [no_npc_food], Filthy Clothing [filthy_morale], ... ]

It acts like a tabulation but I never liked \t because it looks different depending on the font you use, unless it is a fixed font... A pet peeve of mine.

I prefer to have the mod names with a slight offset, it looks better in my opinion.

src/debug.cpp

src/cata_tiles.cpp

src/debug.cpp

src/debug_menu.cpp

src/debug.cpp

src/game.cpp

ZhilkinSerg

Some minor style fixups required for consistency with the rest of the code.

Co-Authored-By: ZhilkinSerg <[email protected]>

neitsa · 2019-05-10T08:56:33Z

Thank you for your review!

ZhilkinSerg · 2019-05-10T09:19:44Z

src/action.cpp

-// Get the key for an action, used in the action menu to give each action the
-// hotkey it is bound to.
-// We ignore bindings to '?' because that will already do something else in
-// this menu (open the menu keybindings).
 long hotkey_for_action( action_id action )


This doesn't return values properly for function keys, e.g. if debug action is set for F10, hotkey_for_action would return -1, thus rendering debug menu unavailable (tested on Windows with VS and MSYS).

Yep you're right, I can confirm this behavior, functions keys are not understood by keys_bound_to. Hmmm I don't know exactly what should be the next step here...

This is the check that returns the bound key from keys_bound_to (which returns nothing at the moment):

Cataclysm-DDA/src/input.cpp

Lines 667 to 670 in f2df59c

if( events_event.type == CATA_INPUT_KEYBOARD && events_event.sequence.size() == 1 &&

events_event.sequence.front() < 0xFF && isprint( events_event.sequence.front() ) ) {

result.push_back( static_cast<char>( events_event.sequence.front() ) );

}

The above comparison with F9 bound to the debug menu:

events_event.type == CATA_INPUT_KEYBOARD: true

events_event.sequence.size() == 1: true

events_event.sequence.front() < 0xFF: false (0x111 for F9 for ex.)

isprint( events_event.sequence.front(): false (0x111 is not printable)

So technically there's a bound key but the check enforces that it must be a 8-bit key code and be printable...

Is it worth a new function or a patch? should I kill this PR?

We can address this in another PR. It is not your fault and this function behavior existed before.

Ok understood! Thanks for your feedback. I'll try to address this in another PR as you suggested. Thank you!

neitsa added 6 commits May 8, 2019 01:11

Replay work.

8e20fd6

Rework mods_loaded().

e23d520

- check for nullptr - check for no loaded mods - Idiomatic c++

Remove function from cata_tiles (moved to game.[h,cpp])

8dacccd

Add save_screenshot function.

909b702

Change ensure_valid_file_name() with actually used parameter.

9ca5591

[WIP] report and screenshot actual implementation.

b0f04ce

ZhilkinSerg self-assigned this May 8, 2019

neitsa added 2 commits May 8, 2019 15:44

Astyle game.cpp.

812a32c

Add success and error message for screenshot.

92dc655

- Check if the SDL copy to clipboard function error-ed or not. - Manual astyle...

ifreund added <Enhancement / Feature> New features, or enhancements on existing [C++] Changes (can be) made in C++. Previously named `Code` SDL: Tiles / Sound Tiles visual interface and sounds. labels May 8, 2019

neitsa added 6 commits May 8, 2019 16:31

Rework all menu options and entries selection.

b6eb8d0

Pass filename to screenshot function...

6628a4f

- Remove check on world validity. The world is already playing at this point.

Add missing lambda param.

8a5028b

Move save_screenshot() to sldtiles.cpp.

cf26131

- Name it take_screenshot() in game.[h; cpp]. - This is the only location where we can access the SDL renderer (except catatiles.cpp but it's really more meaningful in sdltiles.cpp).

Astyle.

2b6dcef

Move "quit to main menu" to debug only.

e6d4b03

- Reorder menu.

neitsa marked this pull request as ready for review May 8, 2019 17:00

neitsa added 9 commits May 8, 2019 22:12

Add guard for include.

a53575a

- Remove unused header.

Remove unused function parameters.

b1698f6

get save_screenshot() outside of 'TILES' definition.

567b7d5

- It is fine to have it outside since it returns false if SDL/TILES are not used.

'extern'-ize save_screenshot().

fd93c7d

typos.

5d1f793

The save_screenshot() function can't be guarded by TILES define.

2d39a0a

- Otherwise the code in game.cpp can't find it. - It returns false if TILES is not defined - Remove obsolete function comment.

Astyle.

02487e9

Proper location outside TILES define.

382d209

Proper location.

1896f25

- As sdltiles is the only file where the renderer can be accessed, but obviously is only usable when TILES is defined, you can only use save_screenshot() if SDL is defined. - Make it so that game::take_screenshot() returns false if TILES is not defined.

neitsa added 3 commits May 9, 2019 13:32

silence compiler warning.

3468b89

Astyle.

92a4324

Linux clang and Apple clang do not agree on required lambda parameter…

2ba01d3

…s... - Make a global context capture.

codemime reviewed May 9, 2019

View reviewed changes

neitsa added 3 commits May 10, 2019 02:17

Fix differing parameter name in declaration and function comment.

a14359d

Integrate codemime's review.

0b708b3

Remove unused headers.

45ca9fc