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")
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.
`struct object_rw` is poisoned prior to initializing and sending it.
However, some fields are legitimately unininitialized (other than their
memset or poison value) at send time. Add and use a poison variant that
can clear those fields, without marking them unreadable.
This is in a redundant check, as we shouldn't (and don't) call newdemo_record_start_frame if nd_record_v_no_space is true, i.e. we ran out of disk space recording a demo. We want to return window_event_result::close with every call to newdemo_stop_playback() to close the game, as this is safer than calling window_close on the game window within its handler. In this case, it's much simpler (and safe) to just remove it.
Even if newdemo_record_start_frame is called when nd_record_v_no_space is true, we don't want to close the game, just exit the function.
Replace calls to window_close(Game_wind) with returning window_event_result::close to game handler. Applies to when DoEndGame() is called, DoGameOver() is called, aborting in the kmatrix screen (multiplayer game) during AdvanceLevel() and playing one demo frame causes playback to stop in GameProcessFrame(). Closing a window within its handler is problematic - it can result in an unstable state.
Old versions of Rebirth sometimes saved games with bogus object data,
such that `obj->type == OBJ_NONE`, but `obj->movement_type`,
`obj->control_type`, or `obj->render_type` were not their respective
*_NONE constants. This confused the game loader code such that it would
load invalid data from the `mtype`, `ctype`, or `rtype` unions and use
it without checking.
Try to counter some of this by exiting early if the object has type
OBJ_NONE.
Valgrind warns when writing uninitialized data to a file. The Descent
savegame format requires writing certain fields that are no longer used.
Set those fields to 0 to prevent leaking stack data into the file.
Levels which fit in MAX_SEGMENTS_ORIGINAL read exactly
MAX_SEGMENTS_ORIGINAL segments worth of light_subtracted, even though
fewer segments were defined. Prepare light_subtracted into a temporary
stack buffer instead of doing byte-at-a-time writes. Initialize this
stack buffer to 0 when necessary. Write the buffer once it is fully
prepared.
player_info::cloak_time is only defined if the player is cloaked.
player_info::invulnerable_time is only defined if the player is
invulnerable.
In both cases, the game save code tried to read the ::*_time variable
even when it was undefined. Modify the saving code to check player
flags before reading the associated timer.
zicodxx reports that f7d0c85 made the thief bot passive and timid.
His analysis suggests that the problem is because f7d0c85 changed
find_connected_distance to return vm_distance::maximum_value() in places
where it previously returned magic values that were not maximum (caching
a distance of F1_0*1000 and returning a distance of -1). Rather than
try to fix the underlying code that relied on these magic values, revert
those return paths to return these unusual values. Move the unusual
values to named constants in file scope so that they are easier to find
and correlate.
Reported-by: zicodxx <https://github.com/dxx-rebirth/dxx-rebirth/issues/286>
Analyzed-by: zicodxx
Fixes: f7d0c853ba ("Use special types for distance/magnitude")
init_player_stats_new_ship used select_primary_weapon and
select_secondary_weapon to assign the player's weapons. However, those
functions read the current weapon and jumped according to its value. A
new ship has no defined value for current weapons, so the jump triggered
an uninitialized value warning from Valgrind.
Add new functions set_primary_weapon, set_secondary_weapon that work
like the previous select_* functions, but always take the path used
for weapons not equal, without checking. This prevents the warnings
from Valgrind, as well as a theoretical risk of initializing the ship
improperly.
An optimization meant to allow change_light to stop early caused it to
stop much too early, so it rarely did any work. Flip the sense of the
test so it does not stop early.
Fixes: 2bd538f353 ("Sort delta light indices")
Demos do not record difficulty level or track hostages correctly. Do
not show these fields in the demo pause dialog, since they reflect
the state of the player's last played game, not the state of the player
recording the demo.
Past releases, when rendering an invalid primary texture, would
Int3() and then reset the texture to zero. Commit d767f7c changed the
logic to return without resetting the texture, since the reset seemed to
be unnecessary. Unfortunately, it is necessary. Some levels, including
those shipped with the retail game data, specify bogus primary textures
on some surfaces. After d767f7c, rendering a surface with an invalid
primary texture causes the surface to be invisible, even if it has a
valid secondary texture.
Remove the return statement added in d767f7c. Extend
validate_segment_side to validate the primary texture on the tested
side. When an invalid texture is found, reset it and log a diagnostic.
For built-in levels, log at level CON_VERBOSE since players cannot
readily fix the level. For external levels, log at level CON_URGENT so
that level authors know to fix their level before releasing it.
Fixes: d767f7cd5e ("Pass vcsegptridx to render_face")
The previous attempt to assign defaults for values that the savegame
lacks was ignored because it overwrote a variable after the last read.
An early draft of that patch worked, but a last minute reorganization to
keep similar purpose code together made the change ineffective.
Narrow the scope of that variable and write to the correct variable.
Fixes: f62ed80205 ("Reset values that should have been in the savegame, but are not")
std::remove_if can be called on an iterator that returns a proxy object,
but the results of that call are not generally useful. When remove_if
attempts to move-assign over removed elements, it instead assigns over
the temporary proxy object. If the proxy object has typical move
semantics, move-assigning over it has no effect on the underlying
container.
Revert to using partial_range on the underlying container.
Reported-by: Mako88 <https://github.com/dxx-rebirth/dxx-rebirth/issues/283>
Fixes: 1f434f98ad ("Use valptridx for ActiveDoors")
When the reactor is forcibly destroyed by the kill goal code, print to
the console and the game log the data that players could otherwise
capture with a screenshot of the network player information.
scores_maybe_add_player might close Game_wind, if it does not return
early. All two callers unconditionally close Game_wind. Remove the
unnecessary closure in scores_maybe_add_player.
