search

CleverRaven / Cataclysm-DDA

Cataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.
http://cataclysmdda.org
Other
10.65k stars 4.18k forks source link

Game crash while crafting black tea. #48599

Open Caelum-Updates opened 3 years ago

Caelum-Updates commented 3 years ago

Describe the bug

Crashes when crafting black tea item. Error message pops up and game freezes inputs.

Steps To Reproduce

Steps to reproduce the behavior:

  1. Open save file attached
  2. Craft black tea in crafting menu
  3. Game crash

Expected behavior

Get black tea.

Versions and configuration

Additional context

Solstice.zip crash.log debug.log *Crash log added for formality. Check debug

Hirmuolio commented 3 years ago

The debug log says that the last thing that happened was that a monster triggered a telepad trap. After being teleported the monster crashed the game. Creature_tracker::update_pos(monster const&, tripoint const&) being the last thing on the log.

Caelum-Updates commented 3 years ago

The debug log says that the last thing that happened was that a monster triggered a telepad trap. After being teleported the monster crashed the game. Creature_tracker::update_pos(monster const&, tripoint const&) being the last thing on the log.

I added the crash log (since it said so) but I think the error logs are in the debug file. The error causes the game to freeze and not receive any inputs.

Hirmuolio commented 3 years ago

Actually on second look the debug log has logs from many events. The thing about tleporter was probably old and not relevant.

The last addition in there is something about putting item in a container. ERROR: item_contents::only_item called with 0 items contained

Hirmuolio commented 3 years ago

I think it has something to do with multicooker. That thing has normal pocket and a magazine well. Somehow this can confuse the code so that it can seem empty and not-empty at the same time. Calling empty() on its contents returns false as if it was not empty but at the same time calling num_item_stacks() on its contents return 0 as if it was empty.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not \'bump\' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

krisskrr commented 3 years ago

I am having similar issue: image

image

image

Save file at exact place as above images: auto_before_launch72.zip

Game version: Stable Frank

I blame some kind of container, this error pops up any time i make liquid stuff like clean water or tea.

krisskrr commented 3 years ago

I think it has something to do with multicooker. That thing has normal pocket and a magazine well. Somehow this can confuse the code so that it can seem empty and not-empty at the same time. Calling empty() on its contents returns false as if it was not empty but at the same time calling num_item_stacks() on its contents return 0 as if it was empty.

Tested: tried to fling my multicooker away from my area, no more error shows up, so i can confirm this might be multicooker issue.

dholmes215 commented 2 years ago

I encountered this in my own game and decided to look into it. Here's the relevant part of the stack trace from my game:

cataclysm-tiles.exe!realDebugmsg<unsigned __int64>(const char * const filename, const char * const line, const char * const funcname, const char * const mes, unsigned __int64 && <args_0>) Line 78
cataclysm-tiles.exe!item_contents::only_item() Line 1181
cataclysm-tiles.exe!item::only_item() Line 13893
cataclysm-tiles.exe!item_ptr_compare_by_charges(const item * left, const item * right) Line 12006
[External Code]
cataclysm-tiles.exe!Character::check_eligible_containers_for_crafting(const recipe & rec, int batch_size) Line 412
cataclysm-tiles.exe!select_crafting_recipe(int & batch_size_out, const string_id<recipe> goto_recipe) Line 1405
cataclysm-tiles.exe!Character::craft(const cata::optional<tripoint> & loc, const string_id<recipe> & goto_recipe) Line 326
cataclysm-tiles.exe!game::do_regular_action(action_id & act, avatar & player_character, const cata::optional<tripoint> & mouse_target) Line 2300
cataclysm-tiles.exe!game::handle_action() Line 2899
cataclysm-tiles.exe!do_turn() Line 704
cataclysm-tiles.exe!WinMain(HINSTANCE__ * __formal, HINSTANCE__ * __formal, char * __formal, int __formal) Line 766

It appears what's happening in is, shile sorting the list of containers, we're comparing a multicooker (left) with a battery to an empty bottle (right) with this function:

bool item_ptr_compare_by_charges( const item *left, const item *right )
{
    if( left->empty() ) {
        return false;
    } else if( right->empty() ) {
        return true;
    } else {
        return right->only_item().charges < left->only_item().charges;
    }
}

left->empty() returns false for the multicooker, which contains a battery as a magazine. But left->only_item() fails because num_item_stacks returns 0, because there's an inconsistency between what item_contents::empty() and item_contents::num_item_stacks consider to be a real item. Here's item_contents::empty():

    for( const item_pocket &pocket : contents ) {
        if( pocket.is_type( item_pocket::pocket_type::MOD ) ) {
            // item mods aren't really contents per se
            continue;
        }
        if( !pocket.empty() ) {
            return false;
        }
    }
    return true;

...and here's item_contents::num_item_stacks():

    for( const item_pocket &pocket : contents ) {
        if( pocket.is_type( item_pocket::pocket_type::MOD ) ||
            pocket.is_type( item_pocket::pocket_type::MAGAZINE_WELL ) ) {
            // mods and magazine wells aren't really a contained item, which this function gets
            continue;
        }
        num += pocket.size();
    }

One function thinks magazines are real contained items, and the other doesn't, but item_ptr_compare_by_charges expects them to be consistent.

Is it normal for multicooker's battery to be a magazine and not a battery? That seems odd to me and I assume it could be why this issue is specific to multicookers. Edit: It appears that's normal. What differentiates multicooker from other battery-powered kitchen tools seems to just be that the others aren't also containers.

Irrespective of that, it seems to me that the mod/magazine check should be consistent between the two functions above (and probably refactored out).

mqrause commented 2 years ago

It might be enough to use item::empty_container instead of item::empty, but if that causes issues with software pockets and the like, it just needs another empty method with the proper pockets checked.

l29ah commented 2 years ago

What differentiates multicooker from other battery-powered kitchen tools seems to just be that the others aren't also containers. What about mess kit, survivor mess kit?

dholmes215 commented 2 years ago

The mess kits have the "CONTAIN" quality, but do not have a container pocket like the multi cooker does:

    "pocket_data": [
      {
        "pocket_type": "MAGAZINE_WELL",
        "rigid": true,
        "flag_restriction": [ "BATTERY_MEDIUM" ],
        "default_magazine": "medium_battery_cell"
      },
      {
        "pocket_type": "CONTAINER",
        "watertight": true,
        "rigid": true,
        "max_contains_volume": "2 L",
        "max_contains_weight": "4 kg",
        "max_item_length": "35 cm",
        "moves": 50
      }

(But I'm just speculating as to the significance of this)