Function `object_create_explosion_sub` is supposed to be given a weapon
and an object on which to explode it. However, the original game abused
`object_create_explosion_sub` by also calling it from
`object_create_badass_explosion`, which can be called from
`explode_badass_object`, which can be called from
`explode_badass_player`. Chained together, this lets
`object_create_explosion_sub` check the weapon ID of a dying player,
which is not a valid operation. This causes a diagnostic from
`get_weapon_id`. Add an explicit test that the object is a weapon so
that non-weapons do not cause a diagnostic.
Reported-by: ryusei117 <https://github.com/dxx-rebirth/dxx-rebirth/issues/302#issuecomment-272048330>
Analyzed-by: zicodxx <https://github.com/dxx-rebirth/dxx-rebirth/issues/302#issuecomment-272710279>
Commit e36abb25cb fixed one problem with demo access (invalid bits in
the high byte of index values), but created another. That commit
switched from direct loading of `front_wall_num` to instead load into
`type`, then move the value to `front_wall_num`. However, `type` is
`sbyte` (an archaic spelling of `int8_t`), so assignments from `type` to
`front_wall_num` were implemented as a sign-extending move, rather than
a zero-extending move. When the wall number was 0x80 or greater, the
sign-extending move produced an incorrect result, which led to a crash
when valptridx trapped the invalid index.
Fix this by changing the types of all three byte-sized variables to
`uint8_t`. None of them need to be signed.
Reported-by: Dosgamer <http://www.dxx-rebirth.com/frm/index.php/topic,2151.0.html>
Fixes: e36abb25cb ("Fix invalid access reading demos")
`get_weapon_id` must be called only on objects of type `OBJ_WEAPON`.
One path in `multi_compute_kill` could call `get_weapon_id` on an object
that is either a robot or a weapon. The code tested for a robot, but
only after performing an access that assumed it had a weapon. Reorder
the tests to prevent the type mismatch diagnostic.
Reported-by: ryusei117 <https://github.com/dxx-rebirth/dxx-rebirth/issues/302#issuecomment-275546550>
By design, valptridx will throw an exception on invalid input. This is
better than silently permitting invalid input to corrupt program state.
Past releases blindly trusted that multiplayer peers would not send
invalid input. Conversion to the valptridx design eliminated the
undefined behavior when peers send invalid input, but still allowed
multiplayer peers to remotely crash the game by sending invalid inputs.
Add a mechanism to trap invalid inputs and gracefully ignore those
messages. This may cause game consistency issues, but will not allow
data corruption.
Contains 2 calls - to obj_create and init_ai_object. For safety reasons and tidiness compared to using obj_create directly. The call to init_ai_object in recreate_thief was already redundant.
All releases to date have a bug where they treat certain segment number
fields as an int, not a segment number. Storing segment_none (0xffff)
into the save file causes affected releases to crash in various places
because it fails to recognize that this is segment_none.
Current code correctly treats segment_none as a non-segment and works
correctly without this hack. The hack is only required to get past
releases to work correctly after loading a saved game written by current
code.
All releases to date have a bug where they treat MarkerObject as an int,
not an object number. Storing object_none (0xffff) into the save file
causes affected releases to exhibit several problems. Most obviously,
they crash if the automap is opened, because they try to show
Objects[0xffff] as a marker, but no such object exists. Additionally,
they refuse to let the player drop a marker because none of the elements
of MarkerObject are -1, so they incorrectly tell the player that all
marker slots are busy. Finally, they will crash if the guidebot is
killed, because the guidebot death code deletes marker 0xffff because it
fails to recognize that this is object_none.
Current code correctly treats object_none as a non-object and works
correctly without this hack. The hack is only required to get past
releases to work correctly after loading a saved game written by current
code.
Fixes: 9125ae32cd ("Make objnum unsigned")
All releases to date have a bug where they treat
Dead_controlcen_object_num as an int, not an object number. Storing
object_none (0xffff) into the save file causes affected releases to
crash when treating object_none as a valid object number.
Current code correctly treats object_none as a non-object and works
correctly without this hack. The hack is only required to get past
releases to survive destroying the reactor after loading a saved game
written by current code.
Fixes: 9125ae32cd ("Make objnum unsigned")
Fixes bug where in the editor, you insert a robot with normal behaviour then play the level - it fails the check on the hide_segment index in init_ai_objects(). This is because hide_segment was written as the poisoned value of 0xfdfd if the behaviour of the robot is AIB_NORMAL - because hide_segment wasn't initialised. Therefore, always setting hide_segment to suppress this exception (it's an inexpensive operation seldom called).
zicodxx reported a problem where poison=overwrite builds caused robots
dropped by other robots not to spawn. I diagnosed it as a problem
caused by not setting `dying_start_time` and proposed using
`init_ai_object` to set this (and other) fields. Kreatordxx posted pull
request #311 to implement this for dropped robots. However, his pull
had the unfortunate side effect of zeroing the dropped robot's velocity.
This change is based on his pull, but with the `init_ai_object` call
moved higher so that the velocity is (unnecessarily) zeroed by
`init_ai_object`, then initialized to `new_velocity` by `drop_powerup`
(as it always was).
Reported-by: zicodxx <https://github.com/dxx-rebirth/dxx-rebirth/issues/293>
Patch-inspired-by: kreatordxx <https://github.com/dxx-rebirth/dxx-rebirth/pull/311>
This is where a client in a multiplayer game hitting a wall fails an assert. Initialise obj->mtype.phys_info.flags in multi_reset_player_object() instead of adding flags. (Tested and still works OK without specifying PF_THRUST.)
Initialise ConsoleObject->mtype.phys_info.flags in reset_player_object() instead of adding flags. The only place this was initialised properly was when reading the player object from disk.
Use std::equal_range to find the upper and lower bounds in a single
binary search, rather than relying on a linear search to find the first
sought element.
Some bitblt code had guards of the form:
#if A
xxx
#if !A
yyy
#endif
zzz
#endif
If A is true, !A is false, so the inner block can never be included.
Delete it.
Also happened when a client to a multiplayer game dropped out due to some network error. Delay call of multi_leave_game() until responding to EVENT_WINDOW_CLOSE, so the game isn't in an unstable state between handling the network event and the game closing.
Commit 4cc801f changed `object_move_one` to return `window_event_result`
instead of `void` and added a default return value at the bottom.
However, it added the value inside a `#if D2` block, so the D1 build now
fails with:
similar/main/object.cpp: In function 'dcx::window_event_result d1x::object_move_one(d1x::vobjptridx_t)':
similar/main/object.cpp:1627:7: error: variable 'result' set but not used [-Werror=unused-but-set-variable]
similar/main/object.cpp:1866:1: error: control reaches end of non-void function [-Werror=return-type]
Move the return statement out of the conditional block to fix both these
errors.
Fixes: 4cc801f42f ("Remove calls to window_close(Game_wind) when game finished or over")
Popular Windows tools for stack unwinding lack support for the DWARF
debug format. Unwinding the stack without DWARF is unreliable without
frame pointers. Default frame pointers to enabled for Windows so that
these tools work despite their lack of DWARF support.
In multi_do_frame(), replace call to window_close(Game_wind) with returning window_event_result::close whenever multi_quit_game is true. Only using this return value where multi_do_frame() is directly called by GameProcessFrame(). multi_quit_game will only be set back to 0 when a new multi game is started.
Closing a window within its handler is problematic - it can result in an unstable state.
Replace call to window_close(Game_wind) with returning window_event_result::close to game_handler. Applies to when there is a failure in net_udp_level_sync(). Closing a window within its handler is problematic - it can result in an unstable state.
Replace call to window_close(Game_wind) with returning window_event_result::close to game_handler. Applies to whenever newdemo_stop_playback() is called. Closing a window within its handler is problematic - it can result in an unstable state.
