Prefer creating a copy explicitly, then appending or extending it as
needed, rather than using `__add__` and allowing it to duplicate the
sequence. `__add__` has two undesirable properties in this script:
- Using `A + B + C` can create multiple temporaries unnecessarily.
- `A + B` typically requires that `A` and `B` be the same type of
sequence, but this script does not need to require that. Using an
explicit copy+extend allows `A` and `B` to be different types of
sequence.
Make use of the latter property to turn some member variables that were
`list` for the sake of `__add__`'s type requirement into `tuple`, since
they are never rewritten.
The link line is not used by clang, but is recorded in the hope that it
is useful for other tools. Stabilize the output by requesting the
`str`, not the `repr`, of the input nodes. `repr` in scons-4.4
generates a bare Python repr, rather than any useful text. In addition
to not being useful, the bare repr exposes the memory address of the
`SCons.Node.FS.File`, which can vary from run to run even with no
changes to the code, causing needless rebuilds of the compilation
database. The `str` of a node is its filename, which is stable across
runs.
env['CPPDEFINES'] may return a deque object in scons 4.5.0+ (see
SCons/scons#4321).
Explicitly convert it to a list where needed.
Tested with scons 4.4.0, 4.5.0, 4.5.1 and 4.5.2.
According to `timeit`, this is slightly faster. Also, `.copy()` will
work when `V` is a `collections.deque`, but `[:]` fails in that case.
SCons issue https://github.com/SCons/scons/issues/3876 (first released
in SCons 4.5.0) changed the storage of the `CPPDEFINES` variable from a
`list` to a `deque`, which broke use of `env.get(name, [])[:]`, since
`deque` cannot copy itself using `[:]`. Switch to using `.copy()` to
obtain a shallow copy.
```
>>> # timing comparison
>>> timeit.timeit(stmt='a.copy()', setup='a = [1, 2, 3, 4]')
0.05652548000216484
>>> timeit.timeit(stmt='a[:]', setup='a = [1, 2, 3, 4]')
0.06801111809909344
```
```
>>> # deque cannot copy using slice notation
>>> a = [1, 2, 3]
>>> from collections import deque
>>> b = deque([1, 2, 3])
>>> a.copy()
[1, 2, 3]
>>> a[:]
[1, 2, 3]
>>> b.copy()
deque([1, 2, 3])
>>> b[:]
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: sequence index must be integer, not 'slice'
```
Reported-by: Kreeblah <https://github.com/dxx-rebirth/dxx-rebirth/issues/704>
Switch from showing the Python tuple, which is unambiguous but hard to
run at a prompt, to showing the output of `shlex.join`, which is
normally correct for use at a shell prompt, and should still be
reasonably unambiguous.
clang-15 warns for a write-only parameter. The parameter is only used
in a debug print, which has been commented out since the code was
originally imported. Remove the parameter and the commented print.
SDL2_mixer changed how it upsamples sounds, and some users complained
about the difference. Add an internal resampler that emulates how
SDL_mixer upsamples sounds. Since all Rebirth upsampling is an integer
upsample (11Khz -> 44KHz, 22KHz -> 44Khz), this internal emulation is
considerably simpler than a general purpose resampler.
With this commit, the builder can choose which resamplers to enable.
The available resamplers are chosen by preprocessor directive, and
presently do not have an SConstruct flag. For each resampler, if no
choice is made, then the resampler will be enabled if it is reasonable.
At least one of the resamplers must be enabled, or the build will fail
with a `#error` message.
The user may choose at program start time which of the available
resamplers to use for that execution of the program, through passing one
of the command line arguments:
- `-sdlmixer-resampler=sdl-native`
- `-sdlmixer-resampler=emulate-sdl1`
- `-sdlmixer-resampler=emulate-soundblaster16`
Runtime switching is not supported. If the user does not choose, then
the first enabled resampler from the list below will be used. The
available resamplers are:
- sdl_native (DXX_FEATURE_EXTERNAL_RESAMPLER_SDL_NATIVE) - delegates to
SDL_mixer / SDL2_mixer, the way Rebirth has historically done.
- emulate_sdl1 (DXX_FEATURE_INTERNAL_RESAMPLER_EMULATE_SDL1) - an
internal resampler that emulates how SDL_mixer worked. This should be
equivalent to sdl_native when using SDL_mixer, so by default it is
enabled when Rebirth is built to use SDL2_mixer and disabled when
Rebirth is built to use SDL_mixer. It can still be enabled manually
even when building for SDL_mixer, but this does not seem likely to be
useful.
- emulate_soundblaster16
(DXX_FEATURE_INTERNAL_RESAMPLER_EMULATE_SOUNDBLASTER16) - an internal
resampler submitted by @raptor in
5165efbc46. Some users reported audio
quality issues with this resampler, so it is not presently the
default.
Remove the method for adjusting the count later, and instead store the
list in a PHYSFSX_uncounted_list until the count is available, then move
it into the PHYSFSX_counted_list along with the count. This prevents
using a list with its count unset.
Store the list directly instead of a reference to a local variable in
the caller's scope. No compiler has warned about this yet, but gcc-12
warned about a similar construct in newmenu_do2, so fix this up before
it becomes a problem.
Support for explicitly deleted functions has been mandatory for a long
time. Historically, this was handled through a special SConf test to
work around a clang-3.4 bug that warned when a deleted function had
named parameters. Support for clang-3.4 was dropped some time ago, and
newer versions do not require this workaround. Simplify the SConf
script by declaring explicitly deleted functions a required feature, so
that they can be tested as a group with the other required features.
For the same reason as 26ab08d55f884ae77ba0e1d400a2d6a92d1c0831: gdb
refuses to fully show an instance of an enum that has incomplete type,
even when the size is known.
UI_DIALOG::event_handler's case `EVENT_WINDOW_CLOSE` can only be hit if
`rval == window_event_result::ignored`. Any other result returned
before entering the switch. If `rval == ignored`, then returning
`ignored` will cause `window_close` to delete the object. Delegate to
that deletion instead of having an explicit deletion in this handler.
Only trust the output of pkg-config if it exited success. Capture any
text sent to its stderr and reprint it, with decorations, to the screen
and to the log.
gcc-12 can issue `-Warray-bounds` warnings for code inlined from system
headers, and warns about `std::sort()` on a small array (
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107986> split from
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104165>). Per a comment
in a related Fedora report
<https://bugzilla.redhat.com/show_bug.cgi?id=2051783#c3>, and local
testing, adding an explicit unreachable on the impossible case that the
end iterator exceeds the end of the array will discourage gcc from
warning without needing to disable -Warray-bounds.
This failure was previously observed in early testing with gcc-12, and
was set aside in the hope that gcc-12 would be fixed before it came into
widespread use. That has not happened, and JoeNotCharles reported
<2644e0cb93>
this independently. JoeNotCharles proposed disabling the warning for
the call to `std::sort`. This commit instead leaves the warning
enabled, but encourages gcc to see that the bad path is impossible,
allowing gcc to delete the bad path before the warning is issued.
JoeNotCharles reports that gcc-12 (architecture unspecified) warns
because `newmenu::process_until_closed` stores the address of a stack
local into a heap-allocated structure. Switch to a less efficient
implementation that does not provoke a compiler warning:
- Store `shared_ptr<int>`, not `int *`, in the `newmenu`
- `shared_ptr::operator*()` and `shared_ptr::operator bool()` allow most
use sites to be unchanged relative to a bare pointer
- Load the return value from the `shared_ptr<int>`. If the newmenu
terminated before return, as should always happen, this is a slightly
less efficient version of the same code as before. If the newmenu was
still open despite its `exists` flag claiming otherwise, then the
returned value may be incorrect, but the read will be well-formed, the
`newmenu`'s eventual write will write to the heap-backed int (rather
than writing into the stack allocated to
`newmenu::process_until_closed`), and the memory will be freed when
both `process_until_closed` has returned and the `newmenu` has been
destroyed.
Reported-by: JoeNotCharles <10a2b2d337>
gcc-12 does enough escape analysis to notice that
newmenu::process_until_closed stores the address of a stack local into a
heap-allocated structure, but not enough analysis to notice that the
stack variable always outlives the escaped address. The compiler then
warns about the address escaping the local scope. Reworking the calling
code not to do this is somewhat invasive, and gcc seems unlikely to
change behavior. Switch to a less efficient implementation that does
not provoke a compiler warning:
- Store a shared_ptr<bool> in the object
- In the object's destructor, write through the pointer to clear the
shared boolean
- In the caller, store a copy of the shared_ptr<bool> as a local, and
use that copy to monitor the shared boolean
This is similar to a change proposed by JoeNotCharles
<10a2b2d337>,
but differs in its details. Among other things, this version takes the
opportunity to move the variable out to a mixin, so that only windows
which expect to be tracked can be tracked. Previously, all windows were
capable of this, even though most never needed it.
JoeNotCharles reported
<1f1903a3b9>
an overflow in `net_udp_send_mdata_direct`. This overflow is impossible
as currently written, because it can occur only if
`multi_send_data_direct` passes an oversized buffer to
`net_udp_send_mdata_direct`, and no messages are large enough to trigger
this. JoeNotCharles proposed adding a runtime check to abort the
program if this happens. Instead, this commit adds a compile-time check
to detect use of an excessively large input buffer.
