Per C99, this code was always wrong. The results of `snprintf` are
undefined if the target string array and one of the supplied input
arguments refer to the same buffer. In practice, this particular usage
happened to work by wastefully copying a string onto itself, then adding
more content at the end. Switch to appending properly, which also fixes
the gcc-8 -Wrestrict warning.
References: <https://github.com/dxx-rebirth/dxx-rebirth/issues/388>
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>
Python 2 permitted defining __new__, then deleting it once it had been
used. Python 3 does not directly reject that pattern, but instead
always fails with an error about wrong number of arguments to object().
Switch to a simple classmethod that is explicitly invoked by the first
__init__().
In Python 3, filter returns an object that can be iterated to get the
filtered content. However, there is no way to reset the object, so it
is useless for persistence. Spend the one iteration copying the data
into a tuple, which can be iterated multiple times.
Python 3 `dict.keys()` returns an object that cannot be added to other
instances of itself. Pass the returned objects as a tuple, then let the
callee use itertools.chain to combine them during iteration.
Python 3 removed `dict.keys` and `dict.items`, then renamed
`dict.iterkeys` and `dict.iteritems` to `dict.keys` and `dict.items`,
respectively. The iteration semantics are preferable here, so add shims
to use the iteration API under a version-appropriate name.
Change PreservedEnvironment.__getitem__ back to a class method, instead
of an instance variable. Python 3 refuses to search the instance
dictionary for __getitem__, so the optimization of copying
`self.flags.__getitem__` to `self.__getitem__` breaks in Python 3.
Python 3 changed the rules for list comprehensions, so the comprehension
can no longer see a class-scope variable without a class qualifier. The
class qualifier is not legal until the class is finished. Work around
this limitation by expanding the use inline.
Materialization centers ("Matcens") have a governor to prevent
overrunning the level in robots. However, due to a logic ordering
error, multiplayer games failed to record how many robots had been
destroyed, so for the purpose of detecting whether there were currently
"too many" robots in play, the game pretended that no robots had ever
been destroyed. Therefore, once enough robots had been created to reach
the "too many" threshold, matcens stopped generating robots, regardless
of how efficiently the player(s) had been destroying robots.
Reported-by: Mako88 <https://github.com/dxx-rebirth/dxx-rebirth/issues/132>
Reported-by: Sirius-TR <https://github.com/dxx-rebirth/dxx-rebirth/issues/386>
If no character matches the typed character, the loop should exit when
`lb->citem == cc`, which should happen after every entry is visited
once. However, if no item is selected, `lb->citem == -1`, and `cc` is
never `-1`, so the exit path never triggers. Rewrite the loop to be
bounded by number of steps, rather than bounded by returning to a
particular offset. This also protects against undefined behavior if the
menu had zero elements.
[Kp: as discussed in pull #377
<https://github.com/dxx-rebirth/dxx-rebirth/pull/377>, the previous
implementation requested a parameter combination not permitted by
OpenGL. At best, this error was silently ignored. Fix the error by
falling through to a switch case statement that sets valid parameters.]
Users with disabled VSync might not expect any forced waiting on the GPU,
and the GL sync methods were intented to fix issues with VSync only,
so the new heuristic for SYNC_GL_AUTO is to enable GL sync only if
VSync is enabled, too.
Users can still request to use a specific GL sync method via the
-gl_syncmethod switch, independent of the VSync setting.
[Kp: folded `else { if () }` into `else if ()` to avoid moving
`ogl_have_ARB_sync` lines. Original change visible at
<https://github.com/dxx-rebirth/dxx-rebirth/pull/381>.]
This mirrors the logic from calc_frame_time(). When VSync is enabled,
waiting for the previous frame to complete might induce long wait periods,
up to the complete frame time, so we should also handle multiplayer there.
For the other sync modes, the GL calls will simply block, so we can't do
anything about that (except going multithreaded, but that's a totally
different can of worms). Note that even without sync, the SwapBuffers()
call in gr_flip() may also block if VSync is on (and enough frames are
queued up), so the issue is not the GL sync code itself.
SYNC_GL_FENCE_SLEEP is now probably the nicest mode for multiplayer
with VSsync, as it is the only one which has the potential to continue
handling multiplayer packets during the wait for VBlank time.
This script was written years ago and has sat unused since then.
Recently, someone asked for a copy, so I may as well publish it. The
HXM support may not have been tested, but the base HAM support can
successfully deconstruct Vertigo and reassemble a bit-wise identical
copy.
Global `Viewer` was copied to local `viewer`, and the local should have
been used in this block. Instead, the global was incorrectly used.
gcc-5 warns for this unused variable. gcc-6 and gcc-7 incorrectly do
not warn.
Reported-by: Jayman2000 <https://github.com/dxx-rebirth/dxx-rebirth/issues/375>
Fixes: 131c1b9f4d ("Add support for PNG screenshots")
Mako88 requests, as have others in the Rebirth forum, that the FPS
indicator be placed on the right side. Moving to the right side was
quick. Moving it low, handling all the different rendering combinations
(cockpit, statusbar, fullscreen; standard vs alternate; single player vs
multiplayer), and not overlapping anything in any of them was time
consuming.
If anyone wants more changes in this area, the existing modes ought to
be revisited and unified. As is, there are pointless inconsistencies
among the modes, which makes it unnecessarily difficult to position an
element correctly.
Requested-by: Mako88 <https://github.com/dxx-rebirth/dxx-rebirth/issues/122>
Commit d580328 eliminated junk whitespace in the individual line items
for joinable games. Unfortunately, this whitespace was not junk. It
was an undocumented workaround (possibly even unknowing workaround) for
a bug that undercounts the space required to show the header. Restore
the "junk" padding to force the window back to its old width.
Reported-by: Mako88 <https://github.com/dxx-rebirth/dxx-rebirth/issues/374>
Fixes: d580328698 ("Combine direct_join allocations")
Mako88 reports incorrect images captured when using a screen resolution
of 1366x768. 1366 is not a multiple of 4. Debugging also shows memory
corruption at this resolution, as Mesa writes off the end of the
allocated buffer. Padding the buffer to tolerate these writes is
insufficient, as libpng then crashes with an alignment fault trying to
read unaligned data from the buffer. All these problems are eliminated
by rounding the width and height to the next multiple of 4.
Reported-by: Mako88 <https://github.com/dxx-rebirth/dxx-rebirth/issues/373>
Fixes: 131c1b9f4d ("Add support for PNG screenshots")
Ambient sound effects have always been buggy in certain topologies,
since `ambient_mark_bfs` refuses to cross its own path. Consider a
corridor:
L _ _ _ _ _ _ _ _ _ _
_ _
_ L
Where L is a lavafall emitting sound, whitespace is insignificant, _ is
a segment, * is a tagged segment, and / is a segment that should be
tagged, but is not. Let the leftmost L be a lower number segment than
the lower L. Sound propagation after the first step is:
L * * * * * _ _ _ _ _
* _
* L
After the second step, it should be:
L * * * * * * * * _ _
* *
* L
However, `ambient_mark_bfs` will stop when it hits the intersection, so
instead the result is:
L * * * * * / / / _ _
* *
* L
To further confuse the issue, emitter segments are processed in memory
order, so if the leftmost L is a higher index segment than the lower L,
the lower L will be processed first and the steps will be:
L _ * * * * * * * _ _
_ *
_ L
L * * * * * * * * _ _
/ *
/ L
Rewrite the propagation to record the travel depth remaining at each
node, and permit it to cross a segment with a lower depth remaining than
the current step. This still stops early when traversal attempts to
backtrack over itself, but permits it to visit, tag, and cross segments
that were previously visited by a different emitter.
clang issues -Wformat-nonliteral when `vsnprintf` is passed the format
string from its caller. This is generally not useful. Fortunately, the
warning can be suppressed by annotating the function as format(printf).
Presumably, this is because clang now trusts that the caller would have
been warned for a bad format string.
Reported-by: kreatordxx <https://github.com/dxx-rebirth/dxx-rebirth/issues/369>
Fixes: dc79bb8e4a ("Remove hack for bypassing buddy time restriction")
clang rightly warns for `if (!var&1)`, which parses as `if (!(var &
1))`, which is probably not what the original author intended.
Unfortunately, the author never commented what *was* intended. The
author might have meant to reject any row with an even length (`if
(!(var & 1))`, but that seems strange in this context. Remove the `&
1`, which retains the sense of what the code has always done.
clang warns for taking the address of unaligned data, but not for taking
a reference to it. It should warn for both. The data should be fixed
not to be unaligned, but for now, this change will quiet the warning.
OS X clang warns when a variable is captured but not used. Linux gcc
does not. Remove the unused variable.
Reported-by: kreatordxx <https://github.com/dxx-rebirth/dxx-rebirth/issues/368>
Fixes: 7da64d3782 ("Add new autoselect-while-firing mode: "when firing stops"")
