gcc before gcc-7 failed to build valptridx:
```
common/include/valptridx.h: In instantiation of 'valptridx<managed_type>::ptridx<policy>::ptridx(const valptridx<managed_type>::ptridx<rpolicy>&) [with rpolicy =
valptridx<dcx::segment>::vm; typename std::enable_if<(policy:: allow_nullptr || (! rpolicy:: allow_nullptr)), int>::type <anonymous> = 0; policy = valptridx<dcx::segment>::vc; managed_type = dcx::segment]':
similar/main/endlevel.cpp:586:105: required from here
common/include/valptridx.h:686:14: error: 'using vptr_type = class valptridx<dcx::segment>::ptr<valptridx<dcx::segment>::vm>' is protected within this context
vptr_type(static_cast<const typename ptridx<rpolicy>::vptr_type &>(rhs)),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
common/include/valptridx.h:666:31: note: declared protected here
using vptr_type = ptr<policy>;
```
This cannot be fixed in the obvious way because the error message is
wrong:
```
664 using containing_type = valptridx<managed_type>;
665 public:
666 using vptr_type = ptr<policy>;
667 using vidx_type = idx<policy>;
668 using typename vidx_type::array_managed_type;
```
The type declarations _already are_ *public*, not *protected* as the
message states. Relaxing the protection on the containing class
resolves the error. This looks bad from an encapsulation perspective,
but does not make the code incorrect.
arch_close calls various library shutdown routines, some of which may
not be in good order after atexit hooks begin executing. Call it before
returning from main, so that the libraries are still fully initialized.
gcc considers strong_typedef to be POD both with and without the
explicit default. clang considers it to be POD only if the constructor
is explicitly defaulted.
For correctness, valptridx::ptridx instances must not be sliced down to
their component ::ptr or ::idx base classes. Previously, this was done
with a dummy template parameter to ensure that a bare ::idx had a
different type than the idx base of a ::ptridx. This extra distinction
complicates analysis of the code, and is not needed when the code is
already correct. Add the ability to build without slice checking.
Fix a bug where the ptridx converting move constructor delegated to the
ptr converting copy constructor, since the ptr copy constructor had
filename/line arguments, the ptr move constructor did not, and the
ptridx move constructor always passed filename/line.
clang-9 reports:
```
similar/main/mglobal.cpp:210:44: error: 'report_error_uses_exception' is a protected member of 'valptridx_specialized_type_parameters<unsigned char, 90, valptridx_untyped_utilities::report_error_style::exception, valptridx_untyped_utilities::report_error_style::exception>'
template <typename T, bool = valptridx<T>::report_error_uses_exception::value>
^
similar/main/mglobal.cpp:229:16: note: in instantiation of default argument for 'instantiation_guard<dcx::active_door>' required here
template class instantiation_guard<dcx::active_door>::type::index_range_exception;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
common/include/fwd-valptridx.h:42:2: note: constrained by protected inheritance here
protected valptridx_specialized_types<managed_type>::type
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
common/include/cpp-valptridx.h:175:8: note: member is declared here
using report_error_uses_exception = std::integral_constant<bool,
```
Add a `using` statement to switch `report_error_uses_exception` to
`public` access. Nothing else uses it, and there is no design purpose
served by keeping it `protected`.
Each axis can act as two buttons in both ways.
For example, a player might map slide left and slide right to J1 -A1 and J1 +A1 as button presses instead of the slide L/R axis.
This is mostly to fix XBox 360 Controller Left and Right triggers. But it can work on every axis if the player wishes to bind them.
The subcanvas is positioned based on the dimensions of the containing
window, so it must be repositioned if the outer window is resized.
Reported-by: vLKp <https://github.com/dxx-rebirth/dxx-rebirth/issues/394>
Fixes: f491059ed7 ("Enable building with SDL2")
This removes the need to walk all objects when creating a new one, since
each object can have a private generation counter, unaware of other
objects. For compatibility with demos, mix in the object's index when
writing the signature value.
Remove the fallback to Boost.Begin. C++14 is now the minimum supported
standard, and any conforming C++14 compiler should have a working C++11
std::begin.
Descent 1 mangles colors during `g3_init_polygon_model`, so this must
not be called on polygons not designed for mangling. Rearrange the
logic to allow Descent 1 to verify that polygon models are well-formed
without using the functions that mangle the colors.
Fixes: 42a2e3ab0b ("Avoid crash loading polymodels with invalid subcalls")
Reported-by: derhass <https://github.com/dxx-rebirth/dxx-rebirth/issues/416>
Bitmaps based on grs_main_bitmap own their data. Bitmaps based on
grs_bitmap do not. Adjust prototypes to prevent initializing a
grs_main_bitmap with data it will not own.
Processing sharepath in SConf is incorrect, because targets can share a
build directory (and therefore an SConf run), but not share a sharepath.
Move sharepath handling out of SConf. Move DXX_USE_SHAREPATH handling
from CGameArg to GameArg, since one game can be built with a sharepath
while the other is built without.
For each link given as http://, verify that the site is accessible over
https:// and, if so, switch to it. These domains were converted:
* llvm.org
* clang.llvm.org
* en.cppreference.com
* www.dxx-rebirth.com
* www.libsdl.org
* www.scons.org
Add macro cf_assert ("control flow" assert) to hint to gcc that certain
conditions are impossible. Use it to avoid generating range checks for
situations that never happen. If the event did happen, the only
consequence would be truncated UI text, rather than a correctness
problem.
This commit enables Rebirth to build with SDL2, but the result is not
perfect.
- SDL2 removed some sticky key support. Rebirth may behave differently
now in this area.
- SDL2 removed some key-repeat related support. Rebirth may behave
differently now in this area.
- SDL2 gained the ability to make a window fullscreen by sizing it to
the desktop instead of by changing the desktop resolution. Rebirth
uses this, and it mostly works.
- Resizing while in the automap does not notify the automap code, so
the view is wrong until the player switches out of automap mode and
back in.
- SDL2 changed how to enumerate available resolutions. Since
fitting the window to the desktop is generally more useful than
fitting the desktop to the window, I chose to drop support for
enumerating resolutions instead of porting to the new API. Users can
now enter an arbitrary window dimension and Rebirth will make an
attempt to use it.
- It might be useful to cap the window dimension at the desktop
dimension, but that is not done yet.
- Entering fullscreen mode through the Controls->Graphics submenu
failed to notify the relevant subsystems, causing the rendered
content not to rescale. For now, compile out the option to toggle
full screen through that menu. Toggling through Alt+Enter works
properly.
Despite these quirks, this is a substantial improvement over the prior
commit, where SDL2 cannot be used at all. The remaining issues can be
resolved in future work.
References: <https://github.com/dxx-rebirth/dxx-rebirth/issues/82>
GCC std::remove_if overwrites removed elements using:
*dstiter = move(*srciter);
This is fine for normal containers, but produces incorrect results when
*dstiter returns a proxy object instead of a reference. In that case,
the proxy object is move-assigned from the source, then goes out of
scope. If the move assignment did not write to underlying storage, as
valptridx proxy objects do not, then incorrect results occur. This
broke ActiveDoor handling (fixed in 4a01fab66d98[1]) and has been a trap
waiting to recur. Apply reference-qualifiers to valptridx objects so
that move-assignment requires an lvalue for the left-hand side. This
permits normal use of move-assignment, but forces a compile error if
std::remove_if or similar are used on valptridx proxy objects.
[1]: 4a01fab66d
Future work is simpler without this, and the stringified name is only
visible in debug output, which is in turn rarely used due to its
verbosity and lack of filtering controls.
Various files included compiler-static_assert.h to use the compatibility
macros for compilers that lacked a working C++11 static_assert.
However, some source files used static_assert without this inclusion,
and no one ever reported problems. From this, assume that no one uses a
compiler which lacks C++11 static_assert. Remove the inclusions that
were only for the compatibility macro. Keep the inclusions that use the
assert_equal helper.
- Remove the one test that clang-5 still fails.
- Require all remaining tests to pass using only C++11 native
static_assert.
- Remove preprocessor-based alternative static_assert implementations.
gcc-8 adds calls to static_assert with a comma inside its first
argument in the implementation of std::vector. This is legal within the
standard, but conflicts with Rebirth's use of static_assert as a
two argument preprocessor macro. The macro is not substantially useful,
and was only present to compensate for the clang limitation removed in
the previous commit. Remove the macro.
References: <https://github.com/dxx-rebirth/dxx-rebirth/issues/388>
valptridx contains `static_assert` statements of the form:
static_assert(var.m, "");
where `var` is a non-`constexpr` reference and `m` is a `static
constexpr` member of a base type of `var`. gcc recognizes that a
`static constexpr` member is a constant expression and permits this.
clang rejects this, presumably because `var` is not a `constexpr`
variable. In the almost 3 years since this was added, clang has not
improved to permit this usage. Rather than continuing to suppress
static_assert in clang, rewrite this expression to be less clear, but be
compatible with clang.
gcc-8 has special cases in its std::advance that require the target to
support `operator++` and `operator--`. These are easy to support, so
add them.
References: <https://github.com/dxx-rebirth/dxx-rebirth/issues/388>
gcc-8 warns that static_cast<dt &>(d) is useless. In general, it is
useless. It is present to force a compile error in cases when `d` is
not convertible to `dt &`. Switch to a compound statement that declares
a local reference to `d` of type `dt &`. This achieves the same
checking effect, but does not provoke the -Wuseless-cast warning.
References: <https://github.com/dxx-rebirth/dxx-rebirth/issues/388>
- Enable display of subsecond (Linux: microseconds; Windows:
milliseconds) precision on gamelog timestamps.
- Add disabled support for YYYY-MM-DD leaders on gamelog timestamps.
Activate it by defining DXX_CONSOLE_TIME_FORMAT_YMD to true.
- Add disabled support for capturing caller __FILE__, __LINE__ in calls
to con_printf, con_puts. Activate it by defining
DXX_CONSOLE_SHOW_FILE_LINE to true. If captured, write those to
gamelog after the timestamp and before the text. This feature (and
only this feature) requires that DXX_HAVE_CXX_BUILTIN_FILE_LINE be
defined, which is conditional on if the compiler has __builtin_FILE()
and __builtin_LINE(). If the compiler lacks this support, attempts to
enable this feature are ignored.
- Switch to using GetLocalTime on Windows.
Rather than use an inline wrapper and rely on the compiler optimizer to
redirect gr_set_current_canvas(nullptr) to gr_set_default_canvas,
rewrite all relevant calls directly in the source.
git grep -l 'gr_set_current_canvas' | xargs sed -i -e 's:gr_set_current_canvas(\s*NULL\|nullptr\s*);:gr_set_default_canvas();:'
Only one caller exists, and that caller alway passes a non-nullptr
value. Switch to a reference and remove the unused special case to
handle a nullptr input.
Most uses pass an orientation matrix. All sites are deterministic about
whether a matrix is passed. Make the matrix mandatory for sites that
passed it, and split out a separate version of g3_start_instance_matrix
for the 2 sites which do not provide orientation.
Remove the `basic_` prefix from valptridx<T>::basic_ptr, ::basic_idx,
and ::basic_ptridx. Since the public names are typedef aliases of these
classes, these class names appear frequently in debug information and
error messages. The `basic_` prefix is unnecessary. Remove it.
git grep -lz '\<basic_\(ptr\|ptridx\|idx\)\>' -- common/include/ | xargs -0 sed -i -e 's/\<basic_\(ptr\|ptridx\|idx\)\>/\1/g'
Early implementations of integer_sequence used a naive implementation
that required one level of template depth per additional integer in the
sequence. Rebirth uses a private alternate implementation named
make_tree_index_sequence that requires only log(N) steps for an
N-element index_sequence. Recent versions of gcc ship a log(N) version
of integer_sequence. Probe for that version and, if found, use it
instead of the private implementation, on the theory that the compiler
writers did at least as good a job as I did, and possibly better if they
were able to leverage compiler implementation details.
Future work will introduce uses of d1x/d2x in the other game when the
types can be usefully shared. Extend the anti-nesting guards to cover
mistakes that might arise when referring to the namespaces by their
real name, rather than the alias name dsx.
Delete stub "compiler-type_traits.h" header. Redirect all uses to the
standard <type_traits> header.
git grep -wlz 'compiler-type_traits.h' -- '*.cpp' '*.h' | xargs -0 perl -p -i <<EOF
BEGIN {
$i = 0;
}
if (($i == 1 && $_ eq "\n") || ($i < 2 && /^#include "/)) {
# First blank line or first user-include after a system-include.
# Print, then never again for this file.
print "#include <type_traits>\n";
$i = 2;
} elsif ($i == 0) {
$i = 1 if (/^#include </);
} elsif ($_ eq "#include \"compiler-type_traits.h\"\n") {
# Remove this line if found.
$_ = '';
}
# Reset state machine when moving to next file.
$i = 0 if eof;
EOF
All supported compilers have an acceptable <type_traits>. Commit
4cb3d46148 ("Move <type_traits> test to Cxx11RequiredFeature") made
<type_traits> support mandatory in August and no one has objected.
Remove the indirection and use namespace std directly for type_traits
members.
Previously, valptridx used PREFIX for allow-invalid+mutable, c#PREFIX
for allow-invalid+const, v#PREFIX for require-valid+mutable, vc#PREFIX
for require-valid+const. Convert the types, factories, and all usage
sites to specify a qualifier for all four combinations:
im#PREFIX -> allow-invalid+mutable
ic#PREFIX -> allow-invalid+const
vm#PREFIX -> require-valid+mutable
vc#PREFIX -> require-valid+const
Changes to common/include/valptridx.h and common/include/fwd-valptridx.h
are manual. All other changes are generated by:
git grep -lz -e '\(obj\|seg\|clwall\|wall\|actdoor\|trg\)\(ptridx\|ptr\|idx\)\(_t\)\?\>' | xargs -0 sed -i -e 's/\<\(v\?\)\(\(obj\|seg\|clwall\|wall\|actdoor\|trg\)\(ptridx\|ptr\|idx\)\(_t\)\?\)\>/\1m\2/g'
for the 'm' prefix and:
git grep -lz -e '\(obj\|seg\|clwall\|wall\|actdoor\|trg\)\(ptridx\|ptr\|idx\)\(_t\)\?\>' | xargs -0 sed -i -e 's/\<\([cm]\(obj\|seg\|clwall\|wall\|actdoor\|trg\)\(ptridx\|ptr\|idx\)\(_t\)\?\)\>/i&/g'
for the 'i' prefix.
Past releases had a debug-only assertion that 25 was sufficient, but
then dynamically allocated enough storage for larger models as needed.
Commit 22a34809ee ("Move interpreter g3s_lrgb onto stack") added a
hard limit of 25, but did not detect attempts to exceed this. Custom
models that exceeded 25 would cause a stack buffer overwrite and likely
crash. Raise the limit to 64 and add sanity checking to refuse to
render models that exceed MAX_POINTS_PER_POLY.
Fixes: 22a34809ee ("Move interpreter g3s_lrgb onto stack")
Type `SyncGLMethod` was defined conditional on DXX_USE_OGL, but header
`common/include/ogl_sync.h` used `SyncGLMethod` unconditionally. This
works fine in the normal build since SDL builds never include
`common/include/ogl_sync.h`, but broke the check_header_includes=1 test
since that compiles `common/include/ogl_sync.h` even for SDL builds.
Wrap type `ogl_sync` in `#if DXX_USE_OGL` to hide it from the
check_header_includes=1 test in SDL mode.
clang becomes confused trying to determine which vm_distance_squared
constructor to use for a literal input of 0x7fffffffffffffff, even
though the size of the input requires it to be `long` and only one
constructor can take a `long`. Switch from an explicit
0x7fffffffffffffff to the symbolic constant INT64_MAX, which has the
same value, but a platform-appropriate suffix to force the compiler to
pick the right type.
For general clarity, switch some other instances of integer maximum
literals to symbolic constants of the same value.
This commit has no effect on the generated code (except for changes to
line numbers).
Reported-by: kreatordxx <https://github.com/dxx-rebirth/dxx-rebirth/pull/324>
Fixes: 17208cca79 ("Disallow int for vm_distance_squared")
When using `gcc -fsanitize=undefined`, the compiler proves trivial
results, so `DXX_CONSTANT_TRUE` is defined. It then fails to prove that
`DXX_CONSTANT_TRUE(m_state == checked)` is false, causing a compile-time
error. Relax the check to occur only when it can prove `m_state` equal
to a disallowed value, rather than when it cannot prove `m_state` equal
to an allowed value.
Move the preprocessor guard so that the runtime check is always visible.
Optimizing compilers can still eliminate that check at compile-time when
it provably never fails.
As a macro, it always refers to the global grd_curcanv. This interferes
with converting canvas handling to be an argument. Expand GHEIGHT so
that uses of grd_curcanv can be changed individually.
As a macro, it always refers to the global grd_curcanv. This interferes
with converting canvas handling to be an argument. Expand GWIDTH so
that uses of grd_curcanv can be changed individually.
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.
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.
`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.
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")
The declaration of valptridx_specialized_types needed to be found by
Argument Dependent Lookup, but this was inconvenient for some types.
Split the declaration of valptridx_specialized_types out from the
definition of valptridx global subtype.
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")
Headers included by SConstruct tests cannot rely on the existence of
"dxxsconf.h"; instead, SConstruct must define the symbols that would
have been in it.
Previously:
constexpr vTYPEptr{};
constexpr vcTYPEptr{};
Now:
__attribute_unused static vTYPEptr{};
constexpr vcTYPEptr{}; // unchanged from above
This is necessary for future work. It should have no user observable
effects for now.
Use a compound statement to cache the success condition as a local
boolean, then reference the local in the macro expansions. This should
hint to the optimizer that this is always the same expression, which
should encourage it not to repeat the test in the generated code.
Actual results vary. x86_64-pc-linux-gnu-g++-5.4.0 generates code that
is bigger, but uses fewer instructions.
This preserves virtually all game information, including active fire. The saved game 'gamesave.sge' can also be loaded with File->Restore Game State. The level can still be saved as usual (but it warns the user if there's an unsaved game state). This functionality may be useful for testing (or cheating! ;) )
Various points in the game code call `hide_menus()`, then later use
`show_menus()` to reverse the effect. If the forcibly hidden window is
deleted before `show_menus()` is called, the attempt to show it would
write to freed memory. Add a hook to forget those windows when they are
deleted, so that `show_menus()` does not try to make them visible later.
Constructing valptridx::basic_ptr with a known-invalid magic index does
not require an array. Simplify the static_assert to reject uses of
factory functions with known-invalid magic index values. Fix the two
sites that fail with the stricter static_assert.
If a game was playing, continue playing. If it was in the main menu, return to the main menu. (If all files are put in place the editor can load again.)
As the UI_DIALOG client is now responsible for freeing all gadgets (via unique_ptr's), this function now causes issues where it attempts to access freed gadgets (resulting in std::runtime_error("unknown gadget kind") exception when closing the AI Properties dialog on my Macbook, for example). Removing the offending function fixes the issue.
Split ogl_get_verinfo() into ogl_tune_for_current() and
ogl_extensions_init(), and consolidate all the OpenGL extension handling
into ogl_extensions.cpp. Unify the code paths for texture anisotropy and
GPU synchronization for OpenGL and OpenGL ES.
Currently, our renderer only uses GLES 1.0, so no real world implementation
will support sync objects for such an old context, but the logic is valid,
and this way, the GLES specific code paths are reduced.
This patch also fixes an issue where the old ogl_get_verinfo() did modify
the texture filtering mode if no anisotropic filter was available. This
was some leftover from the time when the anisotropic filter was a just
a specific CGameConfig.TexFilt mode, and not a separate, orthogonal setting
CGameCfg.TexAnisotropy that it is now.
The segiter code used objptridx because end is signalled by object_none,
and vobjptridx does not allow object_none. However, the compiler
produces better code if segment_object_range_t returns vobjptridx and
future cleanups are easier if iterating objects_in yields vobjptridx
objects. Add a special-case override of the normal checking rules, move
the required checks into segiter, and then let segiter break the rules
normally enforced by valptridx. Add a comment explaining that this
permits unsafe coding and should be done only with a clear understanding
of the responsibilities it brings.
Add 'track' method to dcx::window, which takes a bool and will set it to 'false' when the window closes. Use for newmenu_do2 and simpler newmenu_do's, since there's the possibility of another window without its own polling loop opening on top - meaning window_event_result::deleted would be returned for that window, not the one we're polling on.
This will allow checking for window_event_result::deleted, instead of relying on window_exists, which could return a false positive if a new window was allocated with the same pointer value as the deleted one.
This will be used to inform the event system in future, in removing calls to window_exists. This applies to if the handler returns a window_event_result::close (which is and should be instead of the handler calling window_close itself, at least in most cases).
