Can't load alternate missions #20
Comments
|
Alternate missions work fine here. I usually have more than a dozen mn2 files listed. When you say it breaks handling of alternate missions, do you mean that the game crashes with that callstack? What is the output of info locals and info args in each of the three innermost frames? What is the value of f.size() and sizeof(tex) in set_briefing_filename? |
|
yes, it crashes there, when I try to start the Vertigo mission. |
|
Here's where it crashes when I start the built-in mission: |
|
If I run it outside the debugger, I can play the builtin mission, but vertigo crashes, and the OS reports: |
|
Also maybe of note, after applying f1a6278, the vertigo mission plays, but still never displays the briefing screens or the robot movies. |
Mission names are passed in without an extension. Short mission names read beyond the end of the buffer, causing AddressSanitizer to kill the program. Reported by btb: #20
|
Playing Vertigo crashed for me when using AddressSanitizer. Playing custom missions was fine both with and without AddressSanitizer. Your crash starting a builtin mission is different. I cannot reproduce it, but it might be dependent on some data from the player profile. I see an abort starting Vertigo level 1 at similar/main/gamesave.cpp:1109 from a sanity check on whether a materialization center trigger is set correctly. Running NDEBUG bypasses that Int3 and the game plays normally. I wonder if the level is actually broken or if the Int3 is legitimate. Commit 7c24f68 allows Vertigo briefings to play for me. Commit 84011e5 switches find_hli_entry to use partial_range, which should throw an exception if you have an invalid NHighestLevels. Do you see different results when running with 84011e5? I do not see the Vertigo abort in bm_read_extra_robots. Can you determine which line calls Error? |
|
results are the same with 84011e5. I do get the Vertigo briefings now, provided I still have this revert applied. |
|
It's dying at |
|
You get no output because both Error_puts and Error call Int3 before printing their message. Zico moved the Int3 from after print_exit_message to before it in d9e2337. That move was meaningless until your recent pull request for reactivating Int3. Now, it actually interrupts and kills the program. :) Commit a270317 moves the Int3 back below the printing. For your larger problem, somehow you are ending up with Current_mission_filename = "" while Current_mission->enhanced != 0. When I start Vertigo with a270317, I have Current_mission->enhanced = 2 and Current_mission_filename = "d2x". Neither Valgrind nor AddressSanitizer report any problems starting Vertigo mission 1 for me with that commit. At the time of the error, what is the output of p Current_mission? Where do you keep your Vertigo files? |
|
|
That debugger output was less helpful than I expected. I want to see the contents of the Mission pointed at by Current_mission; the command only printed the memory address of the Mission. |
|
I agree. It seems inordinately difficult to work with these complex data types in lldb, perhaps its easier in gdb, I haven't tried lately. |
|
the problem seems to be that Current_mission is not properly initialized. In in load_mission(), immediately after executing: If I execute the next line: Note that DXX_HAVE_CXX14_MAKE_UNIQUE is false for me, so it's using the workarounds in compiler-make_unique.h |
|
I read that to be that your compiler did not emit an out-of-line copy of operator_() or operator->(), and the debugger cannot figure out how to evaluate the expression without that out-of-line copy. What happens if you try _p _Current_mission._ptr.first? That expression should take the variable Current_mission, explicitly name each of the submembers down to the actual pointer, then dereference that, which should work even without the out-of-line operators. |
|
Sorry for the Github markdown problems. Preview/Edit does not work for me, and the Markdown parser seems to be written with the idea that no one will write code in the middle of a sentence. |
|
Use the backtick to surround inline code.
|
|
Now I'm interested if I'm wrong |
|
That seems right. Did I mention I don't really get modern c++? -Brad On Jan 3, 2015, at 5:31 PM, vLKp notifications@github.com wrote: I read that to be that your compiler did not emit an out-of-line copy of — |
|
Well, a working preview would help too, so I could check for and fix these things. Instead Preview just jumps to the top of the page. Yay buggy AJAX. |
|
in so mission->filename is empty here, but it appears that it was okay in |
|
Can you disable the lldb pretty-printer? It is hiding useful information, such as where the iterator really points. We indirectly see that information after the iterator breaks because of the error message about being unable to read the string, but in the good case, we only see the value referenced by the iterator, but not its address. The line you identified is the C++ equivalent of |
|
haven't figured out how to get begin or distance to work, but i got this: |
|
okay then |
|
Your debugger is probably refusing to evaluate those because they are fully inline. Try adding this block after the assignment of printf("mission->path = %p\n", &mission->path[0]);
printf("mission->filename = %p\n", &*mission->filename);
printf("Current_mission->path = %p\n", &Current_mission->path[0]);
printf("distance(Current_mission->path) = %zu\n", distance(begin(mission->path), mission->filename));
printf("begin(Current_mission->path) = %p\n", &*begin(Current_mission->path));
printf("Current_mission->filename = %p\n", &*Current_mission->filename); |
|
Is all that output from a single run? If so, something is very wrong. Your |
|
yeah, same run. I'm having a hard time figuring out where, because lldb bizarrely won't even acknowledge the existence of the |
|
Actually no. what happens is that It appears that I only have 2 missions installed now, the builtin mission d2, and d2x.mn2. We're in then |
|
obviously it worked before fea751b because |
|
Valgrind shows no sign of problems here, but your explanation is mostly reasonable. Moving path elsewhere in memory should be fine, unless the small string optimization is in effect or your std::string is copied instead of moved. Your earlier output seems to show a 22-byte buffer for the small string optimization, which could cause it to apply fairly often. This should be fixable by defining a move constructor to update |
|
Please sync to 24bdbfb and try again. |
|
looks like that will do it. |
ghost
mentioned this issue
ghost
mentioned this issue
Commit 88b5e61 ("Replace calls to window_set_visible in DoPlayerDead() with stop/start_time()") switched from hiding the game window to only stopping time, and that only if hiding the window would have stopped time. In multiplayer, this allows time to continue running. This introduced a crash if the player dies during the mine countdown. When the player dies, the game checks if the reactor has been destroyed. If so, the game immediately advances to the next level. That advance will try to draw the game window if it is visible. When the window is drawn, if time is not stopped, then game logic runs. Some of that logic is not prepared to deal with the inconsistent state present when a new level is only partially loaded. That inconsistent state then causes a crash. Call stack: #11 slew_frame () at similar/main/slew.cpp:152 #12 in d2x::object_move_one () at similar/main/object.cpp:1758 #13 in d2x::object_move_all () at similar/main/object.cpp:1956 #14 in d2x::GameProcessFrame () at similar/main/game.cpp:1848 #15 d2x::game_handler () at similar/main/game.cpp:1615 #16 in dcx::window_send_event () at common/include/window.h:116 #17 dcx::event_process () at common/arch/sdl/event.cpp:176 #18 in newmenu_do2 () at similar/main/newmenu.cpp:498 #19 in newmenu_do2<dcx::unused_newmenu_userdata_t> () at common/main/newmenu.h:184 #20 newmenu_do<dcx::unused_newmenu_userdata_t const> () at common/main/newmenu.h:190 #21 newmenu_do<1ul, dcx::unused_newmenu_userdata_t const> () at common/main/newmenu.h:196 #22 net_udp_wait_for_requests () at similar/main/net_udp.cpp:4563 #23 net_udp_level_sync () at similar/main/net_udp.cpp:4607 #24 in multi_level_sync () at similar/main/multi.cpp:3458 #25 in d2x::StartNewLevelSub () at similar/main/gameseq.cpp:1803 #26 in d2x::StartNewLevel () at similar/main/gameseq.cpp:2018 #27 in d2x::AdvanceLevel () at similar/main/gameseq.cpp:1648 #28 in d2x::DoPlayerDead () at similar/main/gameseq.cpp:1721 The root cause of this is the layering violation: - Killing the player can have the side effect of advancing the level - Advancing the level can have the side effect of calling multiplayer code while the level data is in an inconsistent state - Calling multiplayer code can cause the event system to redraw the game - Redrawing the game can cause game logic to run Hack around this by restoring the logic that hides the game window, so that the window is not redrawn and the game logic is not run. This does not fix the layering problem, but prevents crashing affected users. To avoid undoing the feature from the breaking commit, hide the window only when advancing to a new level, rather than unconditionally. A player advancing to a new level already lacks the move-at-cold-start capability even on successfully escaping the mine, so no functionality is lost with this change. Players who are dead and do not advance to a new level retain that capability. Fixes: 88b5e61 ("Replace calls to window_set_visible in DoPlayerDead() with stop/start_time()") Reported-by: Ninjared <https://forum.dxx-rebirth.com/showthread.php?tid=1097>
commit d83972d upstream. Commit 88b5e61 ("Replace calls to window_set_visible in DoPlayerDead() with stop/start_time()") switched from hiding the game window to only stopping time, and that only if hiding the window would have stopped time. In multiplayer, this allows time to continue running. This introduced a crash if the player dies during the mine countdown. When the player dies, the game checks if the reactor has been destroyed. If so, the game immediately advances to the next level. That advance will try to draw the game window if it is visible. When the window is drawn, if time is not stopped, then game logic runs. Some of that logic is not prepared to deal with the inconsistent state present when a new level is only partially loaded. That inconsistent state then causes a crash. Call stack: #11 slew_frame () at similar/main/slew.cpp:152 #12 in d2x::object_move_one () at similar/main/object.cpp:1758 #13 in d2x::object_move_all () at similar/main/object.cpp:1956 #14 in d2x::GameProcessFrame () at similar/main/game.cpp:1848 #15 d2x::game_handler () at similar/main/game.cpp:1615 #16 in dcx::window_send_event () at common/include/window.h:116 #17 dcx::event_process () at common/arch/sdl/event.cpp:176 #18 in newmenu_do2 () at similar/main/newmenu.cpp:498 #19 in newmenu_do2<dcx::unused_newmenu_userdata_t> () at common/main/newmenu.h:184 #20 newmenu_do<dcx::unused_newmenu_userdata_t const> () at common/main/newmenu.h:190 #21 newmenu_do<1ul, dcx::unused_newmenu_userdata_t const> () at common/main/newmenu.h:196 #22 net_udp_wait_for_requests () at similar/main/net_udp.cpp:4563 #23 net_udp_level_sync () at similar/main/net_udp.cpp:4607 #24 in multi_level_sync () at similar/main/multi.cpp:3458 #25 in d2x::StartNewLevelSub () at similar/main/gameseq.cpp:1803 #26 in d2x::StartNewLevel () at similar/main/gameseq.cpp:2018 #27 in d2x::AdvanceLevel () at similar/main/gameseq.cpp:1648 #28 in d2x::DoPlayerDead () at similar/main/gameseq.cpp:1721 The root cause of this is the layering violation: - Killing the player can have the side effect of advancing the level - Advancing the level can have the side effect of calling multiplayer code while the level data is in an inconsistent state - Calling multiplayer code can cause the event system to redraw the game - Redrawing the game can cause game logic to run Hack around this by restoring the logic that hides the game window, so that the window is not redrawn and the game logic is not run. This does not fix the layering problem, but prevents crashing affected users. To avoid undoing the feature from the breaking commit, hide the window only when advancing to a new level, rather than unconditionally. A player advancing to a new level already lacks the move-at-cold-start capability even on successfully escaping the mine, so no functionality is lost with this change. Players who are dead and do not advance to a new level retain that capability. Fixes: 88b5e61 ("Replace calls to window_set_visible in DoPlayerDead() with stop/start_time()") Reported-by: Ninjared <https://forum.dxx-rebirth.com/showthread.php?tid=1097>
fea751b breaks handling of alternate missions for me. Playing the builtin mission works fine, as long as there are no alternate missions available (i.e. the mission menu never pops up)
f1a6278 fixes it, by just reverting the meat of that commit. I honestly can't even parse this modern new-fangled C++ well enough to know how to fix it otherwise.
The text was updated successfully, but these errors were encountered: