Sleeping when headlamp are turned on can lead to not responding

[ZikoWong]

Describe the bug
Sleeping when headlamp are turned on can lead to not responding. When you sleep with the headlamp on.the program will recover after a period of no response. And the headlamp were not properly turned off when sleeping. The program does not report errors, but it will not respond for a period of time. If you turn off the headlight first and then go to bed. everything is normal.

To Reproduce
Steps to reproduce the behavior:

1. Wear headlamp
2. turn on headlamp
3. Sleep

Versions and configuration(please complete the following information):

• OS: Win10
• Game Version: 0.D8753、0.D8742
• Graphics version: [Tiles]
• Mods loaded: [dark days ahead]

[kevingranade]

Wild guess, the prompt for turning the light off might be getting drawn incorrectly.

[neitsa]

I did a CPU profiling session on the portion of the sleep() code.

https://github.com/CleverRaven/Cataclysm-DDA/blob/c2068fe5a2249f5fc8595a68654205edbae721d3/src/handle_action.cpp#L720-L779

Between the time you enter the sleep() function and the moment the menu is displayed (note: it is displayed correctly) the code spend 99.56% of its time in it->is_tool_reversible()

        if( it->active && it->charges > 0 && it->is_tool_reversible() ) {

A count breakpoint indicates in my case that this line is only hit 14 times for a total of 55 secondes... (I'm on a debug build so the overhead is really sensible).

I'm going to quickly review the traversed functions (read from top to bottom):

• item::is_tool_reversible()
  • revert.type->invoke( n, revert, tripoint( -999, -999, -999 ) );
• long itype::invoke( player &p, item &it, const tripoint &pos ) const
  • return invoke( p, it, pos, use_methods.begin()->first );
• long itype::invoke( player &p, item &it, const tripoint &pos, const std::string &iuse_name ) const
  • const auto ret = use->can_call( p, it, false, pos );
• ret_val<bool> use_function::can_call( const player &p, const item &it, bool t, const tripoint &pos ) const
  • return actor->can_use( p, it, t, pos );
• ret_val<bool> iuse_transform::can_use( const player &p, const item &, bool, const tripoint & ) const
  • inv.form_from_map( p.pos(), 1 );
• void inventory::form_from_map( const tripoint &origin, int range, bool assign_invlet, bool clear_path )
  • form_from_map( g->m, origin, range, assign_invlet, clear_path );
• void inventory::form_from_map( map &m, const tripoint &origin, int range, bool assign_invlet, bool clear_path )

In inventory::form_from_map, two lines takes almost 99% of the time spent here:

    items.clear();
    for( const tripoint &p : m.points_in_radius( origin, range ) ) { // 48.30%
        // can not reach this -> can not access its contents
        if( clear_path ) {
            if( origin != p && !m.clear_path( origin, p, range, 1, 100 ) ) { //49.49%
                continue;
            }
        }

On these two lines we have:

• tripoint::point_generator::operator++ takes 27%
• operator != 20.6%
• map::clear_path18.7%
• the remainder is split between various other things.

Simple Fix

A very simple change would be to keep the first two checks (they cost 0 in time) and if they are true, proceed to is_tool_reversible(). That would at least prevent the obligatory check using it->is_tool_reversible():

if( it->active && it->charges > 0) {
    if(it->is_tool_reversible() ) {
        active.push_back( it->tname() );
    }
}

I took a look at the various operators used in inventory::form_from_map but they already seem to be quite effective... It's gonna be hard to optimize :(

[ZikoWong]

I used task manager to check CPU usage。 Redownloaded 0.D-8769 After entering the sleep command, the CPU occupies the crazy rising program and does not respond. Return to normal after the confirmation sleep menu appears My CPU is I7 6800K，0.D8769 is a new download, all of which are default settings. I also tested the Curses version.The same problem will arise.

[communistkiro]

On 3 separate saves with at least one resource-draining-per-time item ON be it (heavy) flashlight, oil lamp, mp3 player, cell/smartphone 1 core of my i7 2620 goes haywire for 20-30s w/ v 0.D-1739-gc00528a (tiles) b 8776

[Farseer2]

@neitsa It would only execute if the first two checks succeeded anyway

Anyway, the bug is deeper - is_tool_reversible creates an npc (item.cpp:4982), which, by default, has the position (-1, -1, 500). It then uses it as the invoker of revert item. Many calls deeper, in form_from_map, the npc's position is passed to points_in_radius, which creates a range. Because pos.z is so high, the calculations there causes minz to be larger than maxz, which causes the loop to run 0x100000000 - 499 times. I'm submitting a PR which fixes the overflow, but it is only a partial solution - the fake position should not be used at all. This is my second issue in C:DDA, so I'm not really sure how to solve it, maybe someone reading this can suggest something?