Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid alpha talker while sleeping causes seg fault #78952

Open
vmmmviii opened this issue Jan 5, 2025 · 3 comments
Open

Invalid alpha talker while sleeping causes seg fault #78952

vmmmviii opened this issue Jan 5, 2025 · 3 comments
Labels
(S2 - Confirmed) Bug that's been confirmed to exist

Comments

@vmmmviii
Copy link
Contributor

vmmmviii commented Jan 5, 2025

Describe the bug

Going to sleep triggers an invalid alpha talker error and seg faults the game right after. I tested it a bit and just tried to wait through the night but it seems like sleeping has something to do with the crash?

Attach save file

Marseilles-trimmed.tar.gz

Steps to reproduce

  1. Go to sleep
  2. Shortly after, the error will occur

Expected behavior

I have no idea what's causing the crash but I would expect to be able to sleep without crashing.

Screenshots

No response

Versions and configuration

  • OS: Windows
    • OS Version: 10.0.19045.5247 (22H2)
  • Game Version: fda59d7 [64-bit]
  • Graphics Version: Tiles
  • Game Language: System language []
  • Mods loaded: [
    Dark Days Ahead [dda],
    Portal Storms Ignore NPCs [personal_portal_storms],
    Slowdown Fungal Growth [no_fungal_growth],
    Translate Complex Dialogue [translate_dialogue]
    ]

Additional context

crash.log
debug.log

@vmmmviii vmmmviii added the (S1 - Need confirmation) Report waiting on confirmation of reproducibility label Jan 5, 2025
@PatrikLundell
Copy link
Contributor

/Confirmed

It's not reliably repeatable though.

It's a damned mess of lambdas, function parameter calls, and recursion making it hard to trace what's going on, as the call stack just references the lambdas, and the sometimes fails, sometimes not nature of it makes it harder.

Anyway, it's the EOC_YUGG_DEATH that's referenced in the error message, and that's what I see debugging.
When it fails it seems to be the second (of two) functions and then a recursion of that called from talk_effect_t::apply in npctalk.cpp (so it's a recursed call into this operation). The call stack when it crashes is reported to be:

cataclysm-tiles.exe!std::_Func_impl_no_alloc<conditional_fun::anonymous namespace'::f_player_see'::2'::<lambda_1>,bool,const_dialogue const &>::_Do_call(const const_dialogue & <_Args_0>) Line 876 C++ cataclysm-tiles.exe!std::_Func_impl_no_alloc<talk_effect_fun::anonymous namespace'::f_if'::2'::<lambda_1>,void,dialogue &>::_Do_call(dialogue & <_Args_0>) Line 874 C++
cataclysm-tiles.exe!talk_effect_t::apply(dialogue & d) Line 7133 C++
[External Code]
cataclysm-tiles.exe!talk_effect_t::apply(dialogue & d) Line 7133 C++
cataclysm-tiles.exe!effect_on_condition::activate(dialogue & d, bool require_callstack_check) Line 328 C++
cataclysm-tiles.exe!monster::die(Creature * nkiller) Line 3016 C++
cataclysm-tiles.exe!teleport::teleport_to_point(Creature & critter, coords::coord_point_ob<tripoint,5,0> target, bool safe, bool add_teleglow, bool display_message, bool force, bool force_safe) Line 232 C++
cataclysm-tiles.exe!teleport::teleport(Creature & critter, int min_distance, int max_distance, bool safe, bool add_teleglow) Line 53 C++
cataclysm-tiles.exe!map::monster_in_field(monster & z) Line 2054 C++
cataclysm-tiles.exe!map::creature_in_field(Creature & critter) Line 1803 C++
cataclysm-tiles.exe!`anonymous namespace'::monmove() Line 311 C++
cataclysm-tiles.exe!do_turn() Line 658 C++

It seems this is caused by monster::die in monster.cpp is called with a null killer, possibly because the cause of death wasn't a critter, but rather a result of a teleport collision, so the alpha talker becomes null.
Somewhere, somehow, there should probably be a check in the EOC (support?) stuff to handle the absence of an alpha talker in this case. I leave the how and where to those who understand the EOC stuff.

@github-actions github-actions bot added (S2 - Confirmed) Bug that's been confirmed to exist and removed (S1 - Need confirmation) Report waiting on confirmation of reproducibility labels Jan 5, 2025
@GuardianDll
Copy link
Member

GuardianDll commented Jan 7, 2025

oddly, the only place where EOC_YUGG_DEATH calls for alpha talker is in player_see_u check, which is handled by f_player_see, which already declare creature and then check if it's null or not? or i misunderstand what if( c ) does?

conditional_t::func f_player_see( bool is_npc )
{
return [is_npc]( const_dialogue const & d ) {
const Creature *c = d.const_actor( is_npc )->get_const_creature();
if( c ) {
return get_player_view().sees( *c );
} else {
return get_player_view().sees( d.const_actor( is_npc )->pos_bub() );
}
};
}

@PatrikLundell
Copy link
Contributor

Further thoughts triggered by @GuardianDll's post:

My compiler claims d.const_actor(is_npc) refers to npc_talk.cpp operation const_dialogue::const_actor, which returns the error message generated and then returns a null pointer through alpha.get().

This means the error message is generated when failing to create c, c is tested and fails, and the alternative path is called, where the same operation is used to extract the same null pointer, which is then used to try to get the non existent creature's position. This would mean the else branch of f_player_see should probably return a static result of false as there is (still) no creature from which to extract a position. A possible alternative would be to check if the position of the beta talker could be extracted when no alpha talker is present (and vice versa for a flipped is_npc value).
However, in this call chain it's weird to look for the location of the creature killing the target, as it would be more logical to look for the position of the killed creature: the message is about what spawns at the location, not the killing critter (if any).

At a guess, you don't really want to check "if_player_see_u" (which may be incorrectly implemented, given the killer is referenced), but rather to check whether the player sees "death_loc", as that's the location the stuff the message refers to is spawned.
Thus, I think there are issues both with the JSON and with the "player_see_u" logic. It's possible, however, the "player_see_u" might work with the JSON (although less transparently, in my opinion) if it was checking the dead creature, and the dead creature actually still exists and can be referenced after death (I don't know if the beta talker is the dead critter or the PC, for instance).
Again, I leave it to those who have some understanding of the field to figure out the hows and wheres.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(S2 - Confirmed) Bug that's been confirmed to exist
Projects
None yet
Development

No branches or pull requests

3 participants