search

CleverRaven / Cataclysm-DDA

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

Editing your skills with the debug menu segfaults the game #75679

Closed Standing-Storm closed 1 week ago

Standing-Storm commented 1 month ago

Describe the bug

As title: editing your skills with the debug menu segfaults the game

Attach save file

N/A trivially reproducible

Steps to reproduce

1) Start game 2) Open debug menu and pick Player -> Change All Skills 3) Change the value of a skill 4) Segfault

Expected behavior

The debug menu does not cause a segfault

Screenshots

No response

Versions and configuration

Additional context

crash.log debug.log

PatrikLundell commented 1 month ago

/Confirmed

Blows up in wish.cpp operation debug_menu::wishskill section "if(skill_id>0&&skset>=0)" at skmenu.textformatted[0] =... due to an access violation reading location 0x18. The debugger didn't want to provide any information on anything but the "you" variable so any other info is unknown.

the textformatted field isn't explicitly set in the code, so it wouldn't be unreasonable to assume it doesn't actually have any elements, i.e. has a zero size, at least the first time through the loop.

ZhilkinSerg commented 1 month ago

Should use emplace_back instead of accessing by index [0]:

            skmenu.textformatted.emplace_back( string_format( _( "%s set to %d             " ),
                                               skill.name(),
                                               get_level( skill ) ) );
PatrikLundell commented 1 month ago

I did think about that, but it's in a loop, so what happens the second time around? It should at least be considered whether "emplace_back" should be used if the vector is empty and replacement (as is currently used) if not, as it seems the code iterates until you exit, so you would have been able to modify multiple entries before being satisfied (somewhat hard to verify with it crashing currently).

ZhilkinSerg commented 1 month ago

Funny thing textformatted field is not even used anymore after imgui change, so this is the real bug.

image