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

Robots don’t see me on large distance #11

Closed
Totktonada opened this issue Sep 19, 2014 · 4 comments
Closed

Robots don’t see me on large distance #11

Totktonada opened this issue Sep 19, 2014 · 4 comments

Comments

@Totktonada
Copy link

For example on descent 1 level 24. I can stop, quietly take aim and destroy each consecutively. I use d1x-rebith-0.58.1 on Linux.

@ghost
Copy link

ghost commented Sep 19, 2014

It depends on what distance we talk about. Can you be more specific on which robots you talk about? A screenshot would be sufficient.

@Totktonada
Copy link
Author

Totktonada commented Sep 19, 2014

Demo file is okay? distance_test.dem.zip
In this try second robot was detect me. Maybe in previous try he was not enough fast for it.

Edited 2016-12-29: readded file (there was broken link).

@ghost
Copy link

ghost commented Dec 29, 2016

After examining the demo (which I luckily still had saved), I can conclude that this is also related to the unique robot behavior addressed in #286. Logically this issue was not affected by the bug addressed in #286 but it explains @Totktonada 's observation which is that bots don't see the player over a large distance estimated by find_connected_distance unless it happens during the FCD flush and even then a lot of AI modes may not trigger an attack. Comparing this demo to the original behavior shown in the MS-DOS version running via DOSBox, I can't see much of a divergence to current master. Closing.

@ghost ghost closed this as completed Dec 29, 2016
@Totktonada
Copy link
Author

Christian, thank you for take it into consideration! I’m okay with closing with the reason that it matches the original behaviour.

I edited my comment above to readd the demo file to make the issue more consistent.

vLKp added a commit that referenced this issue Dec 8, 2018
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>
vLKp added a commit that referenced this issue Dec 8, 2018
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>
plomari pushed a commit to plomari/dxx that referenced this issue Jun 11, 2021
I got this in Fight For Your Life, level 19:

main/wall.c:314(wall_open_door): Assert(w->type == WALL_DOOR)
=================================================================
==3862770==ERROR: AddressSanitizer: global-buffer-overflow on address
0x562dee57e448 at pc 0x562decd2169c bp 0x7ffc5d624400 sp 0x7ffc5d6243f8
READ of size 2 at 0x562dee57e448 thread T0
    #0 0x562decd2169b in find_connect_side main/gameseg.c:85
    dxx-rebirth#1 0x562ded061683 in wall_open_door main/wall.c:367
    dxx-rebirth#2 0x562decc10ccf in collide_robot_and_wall main/collide.c:115
    dxx-rebirth#3 0x562decc3d5a1 in collide_object_with_wall main/collide.c:2598
    dxx-rebirth#4 0x562decfa4ef1 in do_physics_sim main/physics.c:529
    dxx-rebirth#5 0x562decf8f1c5 in object_move_one main/object.c:1727
    dxx-rebirth#6 0x562decf92000 in object_move_all main/object.c:1858
    dxx-rebirth#7 0x562deccc8090 in GameProcessFrame main/game.c:1126
    dxx-rebirth#8 0x562deccc5ea9 in game_handler main/game.c:904
    dxx-rebirth#9 0x562decb5c486 in window_send_event arch/sdl/window.c:230
    dxx-rebirth#10 0x562decb4ed2c in event_process arch/sdl/event.c:184
    dxx-rebirth#11 0x562decb4efa2 in window_run_event_loop arch/sdl/event.c:206
    dxx-rebirth#12 0x562decdfbe8d in main main/inferno.c:341
    dxx-rebirth#13 0x7f7ec936ed09 in __libc_start_main ../csu/libc-start.c:308
(/mnt/big/doh/sandgrube/dxx-rebirth/d2x-rebirth-gl+0x7bcd69)

0x562dee57e448 is located 4 bytes to the right of global variable
'd_tick_step' defined in 'main/mglobal.c:38:5' (0x562dee57e440) of size
4
0x562dee57e448 is located 56 bytes to the left of global variable
'Segments' defined in 'main/mglobal.c:40:9' (0x562dee57e480) of size
10720000
SUMMARY: AddressSanitizer: global-buffer-overflow main/gameseg.c:85 in
find_connect_side
Shadow bytes around the buggy address:
  0x0ac63dca7c30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7c40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7c50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7c60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7c70: 00 00 00 00 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
=>0x0ac63dca7c80: 04 f9 f9 f9 f9 f9 f9 f9 04[f9]f9 f9 f9 f9 f9 f9
  0x0ac63dca7c90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7ca0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7cb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7cc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac63dca7cd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==3862770==ABORTING

Possibly unreproducable. Judging from collide_robot_and_wall, maybe it's
a case where a wall was marked as needing a key, but there's no door?
Unknown. Try to avoid this by returning if it makes no sense.
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant