/home/deen/isos/ddnet/ddnet-source/src/engine/client/discord.cpp: In member function ‘bool CDiscord::Init(FDiscordCreate)’:
/home/deen/isos/ddnet/ddnet-source/src/engine/client/discord.cpp:39:17: error: ‘mem_zero’ was not declared in this scope
39 | mem_zero(&m_ActivityEvents, sizeof(m_ActivityEvents));
| ^~~~~~~~
Prevent potential dead lock when using `/map` chat command in combination with Teehistorian. Closes#7619. I could not reproduce the issue on Windows though.
This also means resetting game settings should actually be efficient now, because it only involves iterating over all game settings and directly resetting their value.
Previously, only one buffer was allocated to store each snapshot holder structure, the snapshot data and alternative snapshot data. This prevents tools like ASAN from detecting some invalid accesses, e.g. to the snapshot holder structure by underflowing the snapshot data pointer. Now three separate allocations are used instead. This hopefully helps debugging #2677.
We support little and big endian but not PDP endian (Middle-endian).
Define endianness as string `CONF_ARCH_ENDIAN_STRING` to avoid conditional compilation when printing endianness.
The `LoadMapSearch` function returns an error message or `nullptr` on success but the condition was incorrectly changed in #7580 so the opposite was checked instead.
Closes#7597.
Setting a vote timeout longer than 60 seconds with `sv_vote_time` caused the vote network messages to be discarded with the error message `weird message 'Sv_VoteSet' (15), failed on 'm_Timeout'` by the client, as the protocol did not allow longer vote timeouts.
This changes the protocol so the vote timeout can be any positive integer although for now the maximum `sv_vote_time` value is changed back to 60 again to preserve compatibility with old clients.
Closes#7583.
Support registering arbitrary number of interfaces with `IKernel` instead at most 32.
Assert when incorrect arguments are passed to `IKernel` functions instead of returning a `bool`, which was not being handled in all cases. These functions are not expected to fail expect on programming errors.
Consistently order and format creation and registration of kernel interfaces in client and server.
Additionally, ensure snapshots are cleared and dummy is disconnected when disconnecting programatically with `DisconnectWithReason`.
Add `NETADDR_ZEROED` and `UUID_ZEROED` for more convenient initialization without using `mem_zero`.
Add stricter error handling when converting between UTF-16 (wide characters) and UTF-8 (multi-byte) on Windows.
The `windows_wide_to_utf8` function now returns an `std::optional`, which will be empty if the argument contains invalid UTF-16. Files/folders with names containing invalid UTF-16 are now ignored on Windows. It was previously not possible to use these files either, as converting their names to UTF-8 changed the invalid codepoints to unicode replacement characters.
The `windows_utf8_to_wide` function now fails with an assertion error if the argument contains invalid UTF-8, as this should never happen.
Closes#7486.
Add `CHttpRequest::ExpectSha256` function to specify expected SHA256 for HTTP downloads. If the download completes with a different SHA256 hash than expected, then the download fails and the file is deleted.
Closes#7485.
Move all code for handling of config variables from console to config manager. The console no longer depends on the config manager, instead the config manager now depends on the console.
Add `struct`s to manage config variables of different types (int, color and string). The config manager now keeps a list of all config variables, so usage of the preprocessor can be avoided except for code to initially create all config variables. Additionally, a separate list of just the game config variables (config variables with `CFGFLAG_GAME`) is kept to optimize the `ResetGameSettings` function, as this function is called whenever connecting to a server and should be fast. Previously, this function was even less efficient because it preformed a linear search for every individual game config variable to find the respective command data.
Move console commands that opperate only on config variables (`reset`, `toggle` and `+toggle`) to config manager. Ensure that these commands only opperate on the desired config variables (client or server, respectively) by checking `IConsole::FlagMask`.
Add `IConfigManager::SetReadOnly` function to set/unset config variables as read-only instead of manually overriding the command handlers for the `sv_rescue` and `sv_test_cmds` config variables. This also fixes that read-only config variables could still be changed by using the `reset` command. A console message is now printed when trying to change a read-only config variable. Removing the special handling for these two config variables is additionally useful so the console does not need to keep a pointer to config values and manager.
Use a `CHeap` for the config variables, their help texts and the previous values of string config variables to avoid many separate allocations as well usage of static variables. Also use the heap to store the unknown commands instead of using `std::string`s.
Properly trigger command chain when resetting config variables with the `reset` command and when resetting game settings on map loading. Closes#7461.
Format default value for color variables as RGB/RGBA hex with dollar sign prefix. Closes#5523.
Add log message when using `reset` with a variable that does not exist. Use `log_error` instead of `dbg_msg` when saving config file fails.
Support unlimited number of config save callbacks instead of at most 16. The code also becomes more readable by using an `std::vector` instead of a fixed-size array and a separate num variable.
Consistently name `MACRO_CONFIG_*` parameters when declaring the macros.
Add `IConsole::CMDLINE_LENGTH` constant to represent the maximum length of the console input and thereby reduce usage of magic numbers for buffer sizes.
Add a templated `CHeap::Allocate` function to simplify memory allocation of objects using `CHeap`. The function uses perfect forwarding to construct objects with the specified arguments.
Apply the name bans system also to player clans, meaning players joining with banned clan names are kicked and changing the clan to a banned name while ingame has no effect.
Additionally, trim UTF-8 whitespace from beginning and end of clan. This was already done for player names but not for clans.
Closes#7516.
Unify all code for name bans in new class `CNameBans` in the existing `name_ban.cpp/h` files. The previously global function `IsNameBanned` is now the member function `CNameBans::IsBanned`. The existing name ban tests are extended for the `CNameBans` class.
Move `CNameBan` constructor definition to source file to avoid including `system.h` in the header file. Use `bool` instead of `int` for `m_IsSubstring`. Reorder `CNameBan` constructor arguments and remove unnecessary default value.
The contents of `variables.h` are moved to `config_variables.h` instead of being included with the preprocessor. The file `variables.h` is removed, so all config variables can be found in a single file instead of being spread over two files without any clear structure. The original declaration order of config variables is preserved. The unnecessary header guard `GAME_VARIABLES_H` from `variables.h` is removed, as the comment `// This file can be included several times.` already serves the same purpose of silencing the header guard check.
A comment is added at the end of `config_variables.h` to mark the location where modders should ideally declare all of their own config variables to avoid merge conflicts with us in the future.
Closes#7472.
Interfaces should not have member variables, so the variable is moved to `CConsole`. Only a getter `IConsole::Cheated` is added because the cheated state of the console is never reset.
So other interfaces/components can check which console commands are active. In particular this will be used to refactor the config manager. The respective setter function already exists.
Due to swapped `str_copy` arguments, the old value for string game variables was being overridden by the current value instead of the other way around in `CConsole::ResetGameSettings`.
There are currently no string game settings affected by this.
Handling color settings the same as int settings in `CConsole::ResetGameSettings` is not correct, as this causes the color variable data to be cast to int variable data.
There are currently no color game settings affected by this.
Call expected server callback functions to simulate clients dynamically connecting and disconnecting when changing the `dbg_dummies` variable. This makes the debug dummies more useful for debugging. Previously, the debug dummies were considered invalid clients, whereas they are now considered to be ingame, so they should behave mostly like real clients being connected to the server. The debug dummies also have correct client names now, e.g. "Debug dummy 42".
The game server code is cleaned up by moving all special handling for debug dummies to the engine server function `CServer::UpdateDebugDummies`.
The left/right direction inputs for debug dummies are now properly added to the client input array, so their input handling should be consistent with normal clients, which fixes some inconsistent prediction with debug dummies.
Greyscale images with alpha channel (i.e. channel count = 2) were incorrectly handled as RGBA images, causing the client to crash when loading such images. Now the images can successfully be loaded with the image loader, but the client still only supports loading RGB and RGBA images like before.
The image data is freed when the image format is unsupported, but `CImageInfo::m_pData` would still point to the freed memory, so double-frees were possible.
Ensure that community/country/type filters do not exclude all allowed elements, which can happen when a previously selected community is not available anymore or when arbitrary community filter values are set with the console.
Initialize nontrivial types with a constructor instead. Make the
compiler aware that some of our constructors are indeed trivial.
This allows `mem_zero` calls to actually always zero the memory.
Partially replaces #5690.
POD types are just memset. Other types are either destructed if not
trivial and/or constructed if not trivial. Types need to have a default
constructor.
Virtual classes can be mem_zeroed only if they already have been
constructed, otherwise it is UB.
Zeroing the entire ringbuffer memory is unnecessary and inefficient. Only the first item in the ringbuffer has to be initialized properly.
It's unlikely that existing code depends on allocated memory being zeroed, as recycled ringbuffer items where never zeroed.
On Unix, the encoding of filenames is unspecified, although UTF-8 is likely used in most cases. Detecting and converting from several possible encodings to UTF-8 would be too much effort, considering that most systems already use UTF-8 per default. Therefore, any filenames which are not valid UTF-8 will be ignored when listing directories with `fs_listdir(_fileinfo)`, `fs_storage_path` will fail if the storage location is not valid UTF-8 and `fs_getcwd` will fail if the current working directory is not valid UTF-8. Paths specified in `storage.cfg` must also be valid UTF-8.
On Windows, we already ensure that all filenames are converted to UTF-8.
Use `CLock` and `CLockScope` instead of `std::mutex` and add clang thread-safety analysis annotations everywhere except for usages in engine graphics and video, as those usages also involve `std::condition_variable`.
Fix lock not being unlocked on all code paths in `CFutureLogger::Log`, which is caught by the static analysis.
Replace usages of platform specific `lock_*` functions with `std::mutex` through the wrapper class `CLock`. Move lock classes to `base/lock.h`.
The `CLock` wrapper class is only necessary because the clang thread-safety attributes are not available for `std::mutex` except when explicitly using libc++.
After the error message popup is shown for graphics assertions, the client window was destroyed but the process was not terminated properly. Now the graphics assertion error is handled like a normal assertion error, as those are also shown in an error message popup and correctly cause the client to break into the debugger or terminate.
However, calling `dbg_assert` while already owning the lock that `WaitForIdle` waits on will cause a deadlock, so error handling must be delayed until after the lock is released.
The buffer size for assertion messages is increased, as it was not sufficient for some graphics assertions.
The `ICommandProcessor::GetError` and `ICommandProcessor::GetWarning` functions are marked as `const`.
Previously, the client would crash with an assertion when writing more than 1024 items, 1024 data items or 64 extended item types to a map file. This arbitrary limitation is removed by using `std::vector`s, so much larger maps can be written. This was only a limitation when writing map files. Old clients can correctly read the map files having more items/data but will crash when the map is saved in the editor.
Remove the DDNet and KoG tabs and replace them with a community filter, which works like the country and type filters. Community information (ID, name, icon SHA256, countries, types, servers, ranked maps) is loaded dynamically from the DDNet info json.
The official checkmark icon is replaced with community icons. The icons are downloaded to the `communityicons` folder in the config directory from a URL specified in the DDNet info json. The icons are only re-downloaded when their SHA256 has changed.
Per default, the Internet tab with all communities is selected. When starting the client for the first time, only the DDNet community is selected. The community, country and type filters can also be used on the Favorites tab now. The LAN tab remains unchanged.
The country and type filters are refactored. Previously, using these filters changed the total number of servers and players that were displayed, as the filter was implemented as a separate step. Now the community, country and type filters work like the other browser filters. This also has the benefit that the server list and DDNet info json are not reloaded each time these filters are changed, as updating the local filtering/sorting is enough now.
The check for finished maps is made more efficient by using an `std::unordered_set` instead of linear search.
Closes#5654.
Some calls to `LoadTextureRaw` where passing the image format also in place of the texture load flags. Since the image format would be RGBA most of the time, this was interpreted as the flag `IGraphics::TEXLOAD_NOMIPMAPS` causing various textures (all sprites textures, assets when shown in menus, menu images, menu theme icons) to be loaded without mipmaps.
This is fixed by passing `0` as the texture load flags and by replacing uses of `LoadPNG` and `LoadTextureRaw` with `LoadTexture`.
Rename `InvalidTexture` to `NullTexture` to avoid confusion because the null texture is not invalid. The function `CTextureHandle::IsNullTexture` is added to make it easier to check the result of using `IGraphics::LoadTexture`, which returns the null texture on error instead of an invalid texture handle. An assertion is added to ensure that `IsNullTexture` works correctly.
The handle returned by `LoadTextureRaw` is either valid or invalid (i.e. `-1`) but never the invalid texture itself.
Additionally, ensure that the `LoadTexture` function returns the invalid texture also when `LoadTextureRaw` fails.
Using `CMenus::PopupWarning` will immediately set the popup and override the current warning, which can hide warnings when multiple are shown at the same time. Now all warnings are added to the queue with `IClient::AddWarning`.
Add assertions to prevent map items and data that are too large to be represented by the map format from being written to maps. Additionally, ensure that the size of the whole map file is not too large to be represented by the map header.
Prevent `malloc` of size 0, which is implementation defined and should be avoided, when adding items without data, which happens for example when adding an empty array of envelope points.
Check whether the demo file has been closed due to a playback error when calling `DoTick` in loops, to prevent crashes on operating systems with assertions for correct C library usage. On Windows, `warning: Invalid parameter passed to C runtime function` was printed to the debug console in this case.
Ensure server password can fit username and password separated by colon, so there is no discrepancy where some passwords work in rcon but not for reserved slots.
There is no need to limit the length of server password, rcon username, rcon password and rcon commands when packing them into their respective messages. The source buffers for these strings are already limited in length and the message packer ensures that the maximum packet size is not exceeded.
Add parameter `int Conn` to `SendInfo` and `SendReady` functions and use existing `SendEnterGame` function to reduce duplicate code for sending the same messages for main and dummy connections.
The host lookup job and the engine interface are independent so they are moved to separate files.
The include of `engine.h` in `client.h` is therefore unnecessary and other includes also had to be adjusted because of this.
The variable `m_VersionServeraddr` is unused and therefore removed. The host lookup job is currently not used on the client-side.
Handle the return values of all uses of the `io_read`, `io_skip` and `io_tell` functions in `CDemoPlayer`, to handle truncated demo files and other unexpected file reading and seeking errors during demo playback.
Show more detailed error message popups when demos cannot be loaded due to errors and when demo playback is stopped unexpectedly due to errors. Previously, only a generic message "Error loading demo" was shown when loading failed and no error message was shown when demo playback is stopped due to errors.
Add checks to ensure that the ticks read from demo chunk headers are in the valid range (cf. `MIN_TICK` and `MAX_TICK` constants). Playing demos with invalid ticks is prevented entirely, as the invalid ticks would cause issues in other places. The server never uses ticks outside the valid range, so invalid ticks should never occur in correctly recorded demos.
Additionally, checks are added to detect if a tickmarker chunk with a tick delta occurs before a tickmarker chunk defining an initial tick. The tick delta is only meaningful when an initial tick has already been defined, so if this is not the case the demo is also considered invalid.
Make the title of warnings adjustable, with the default title being "Warning" to preserve existing code.
Make auto-hiding configurable, so the automatic closing of warning popups after 10 seconds can be toggled.
We only show the string value directly in the UI instead of using the demotype enum literals. There also should not be any reason to change any logic depending on whether the current demo is a server- or client-demo.
The SHA256 was effectively not optional anymore when recording demos, as it and the SHA256 extension UUID were always written to the demo file without checking for `nullptr`. Therefore the SHA256 is now passed by const reference instead of by pointer and redundant checks for `nullptr` are removed.
We already use `CClient::LoadMapSearch` to load maps for demo playback, so maps in the `maps` folder are already considered. Using the map CRC is deprecated, we do not want to check it.
Using an `std::vector` is simpler and more readable than first allocating a linked list on a heap and later copying the list to an array.
The variable `m_SeekablePoints` is removed from `CPlaybackInfo`, as it does not need to be exposed outside of `CDemoPlayer` and can be replaced with the size of the vector.
The client/server jobpools were previously only using a fixed number of 2 threads. Now the pools use `2 * hardware-concurrency + 2` threads, which should provide better performance overall, as we expect threads to often wait on I/O.
Previously all demo players checked `IVideo::Current` to render a video, which would cause issues when rendering a demo while a demo slicing background job is running. Now video rendering can be toggled for each demo player individually and is only enabled for the main demo player.
Use member instead of static variables for demo player snapshot data. The static variables would otherwise cause issues when multiple demo players are playing at the same time, especially demo players used in background jobs for replay demo slicing. This hopefully closes#7068.
Use composition instead of inheritance for demo player listener to avoid temporarily storing a pointer to a local variable in a member variable.
Fix demo player file not being closed when demo recorder file cannot be opened during demo slicing.
Add assertions to check that demo recording/playback is not started when already running.
Add assertions to check that demo recording/playback is stopped before the respective demo recorder/player is destroyed.
The TODO in the `Dilate` function is removed, as the code already appears to be safe without additional checks. The variable `k` is at most `(w * h - 1) * BPP`, as `ix` and `iy` are clamped to maximum `w - 1` and `h - 1` respectively. Because `p < BPP - 1` the index `k + p` is therefore always valid for the buffers. (The caller must ensure that the source and destination buffers are of size `w * h * BPP`.)
Improve layout of demo browser buttons, search and checkbox. Use icons for most buttons. Decrease size of UI elements.
Move demo details from below the file browser to a side panel on the right, which is the same size as the server browser side panel. Use ellipsis to truncate the SHA256 instead of rendering only half of it with reduced font size. Show "map not included" instead of "0.00 KiB" for the map size of demos that don't include a map.
Remove the "Markers" column, as it is not very useful to see for all demos and sort by.
Closes#7275.
Simplify the usage of datafile reader and writer by adding utility functions to read and write zero-terminated UTF-8 strings.
Improve validation of string data read from datafiles. It is ensure that string data is null-terminated, has no internal NUL-characters and is valid UTF-8.
Fix loading of external sounds in the editor. The wrong path variable was being used, so the sound files would not be loaded from correct folder.
Add tests for new datafile reader/writer functions.
When `IMap::Load` fails, other components will continue to use the old map. However, if `IMap::Load` failed after the map was already successfully read with the datafile reader then other components kept their pointers to the old, invalid datafile reader items and data, causing random crashes in collision code. This is fixed by using a separate datafile reader to read the new map and only applying the datafile reader globally when loading was entirely successfully.
An error message is added for the case that a map has an unsupported version, which is currently the only case where a map can fail to load after the datafile was read successfully.
In particular, the block maps `blmapPepe`, `blmapV5` and `blmapDT-UPTU` did not have a version map item and were fixed separately.
Closes#7218. Regression from #5737.
The pixel size (bytes per pixel) always has to be 4 for the `Dilate` function to work correctly. This is already checked before calling the function, so the redundant argument which is always `4` can be removed.
The wrong color channel count was being used for loading images that use an indexed color palette, which was causing the loading to fail with the error "bytes in row incorrect". Now the correct color channel count is retrieved using the libpng API.
Closes#7157.
Use config manager to reset `ui_page` to the default, instead of using `CMenus::PAGE_DDNET` directly (which is the default).
Check current type of serverbrowser instead of checking the current `ui_page` against `CMenus::PAGE_DDNET` and `CMenus::PAGE_KOG`.
Using `dbg_stress 1` now only does the following (in debug build):
- Randomly send inputs.
- Randomly send chat messages.
- Randomly connect/disconnect to server configured with `dbg_stress_server` (`localhost` by default).
Previously it also did the following, which is not useful for this debugging feature and only complicates the code unnecessarily:
- Cause images and sounds not to be loaded.
- Render only every tenth frame.
- Always use inactive graphics refresh rate.
Instead of keeping track of a permanently empty `CSnapshot` object in client and server separately, add `CSnapshot::EmptySnapshot` to access a singleton empty `CSnapshot`.
Mark pointer parameters of snapshot functions as `const` when possible.
Simplify the handling of free texture indices by using `-1` only for indices which are currently in use, whereas the size of the vector is now used to indicate the last free index. Otherwise the assertions incorrectly detect the last texture index always being in use because `-1` was used for both states.
Previously, if the demo header strings did not contain zero-termination, the client would render the strings and any following non-zero memory from the demo header.
Now, demos will not be loaded, if any string in the header is not zero-terminated or not valid UTF-8.
When using `auth_remove`, the key indices for the default helper, mod and admin passwords were not properly adjusted, causing the wrong passwords to be used for the username-less logins.
The key indices for connected clients were also not properly adjusted, causing the wrong identity to be shown for currently authenticated clients when using the `status` command.
Closes#6427.
Most variables used in the sound engine were static globals, as they are used in the static sound mixing function. The global variables are replaced by member variables, by passing the sound interface as user-data for the SDL mixing callback. The `Mix` function is made a public member function of `ISound` instead of being exposed using `ISoundMixFunc GetSoundMixFunc()`.
This allows to remove the direct dependency of the engine sound on the engine video, by instead passing the sound mixing function as a lambda to the engine video in the engine client.
The old WavPack reader function interface does support passing a user-data pointer to the callback function, so global variables are still used here.
Replace existing hard-coded support for two "networks" with support for a dynamic list of "communities" which are indexed by a string-ID, though right now the DDNet and KoG communities are still hard-coded. The communities now also support an arbitrary number of associated countries, types and servers. This is a refactoring to prepare for getting the list of communities dynamically from the server and removing the DDNet and KoG tabs from the serverbrowser.
New UI elements were being created for every server info after refreshing the server list. At the same time, old UI elements were not being deleted when the server info objects are deleted. The use of `mutable` for this purpose was also rather unclean.
Now, a separate `std::vector` of UI elements is kept for all server browser entries, instead of associating the UI elements directly with the server info.
Move the "Filter", "Info" and "Friends" tabs to the top, above the tab content. Use icons instead of text for the tabs. Use animator to animate the tabs on mouse-over. Closes#6613.
Make spacings, corners and font sizes used in the filter, details and friends tabs more consistent.
Remove some unnecessary dark UI rect backgrounds.
Improve alignment of the number of friends with the heart icon for entries in the server list.
Improve layout of countries and types filters. Make the filters scrollable when there are many entries.
Refactor most of the server browser in preparation for replacing the DDNet and KoG tabs with a community filter, which will work like the countries and types filters. Split rendering of different server browser sections into multiple functions to improve readability. Reduce duplicate code for the countries and types filters.
Make the skipping duration adjustable with a dropdown menu. The dropdown menu includes the durations 1s, 5s, 10s, 30s, 1m, 5m and 10m. The default duration is 5s. Skipping durations longer than the current demo are not shown. The dropdown menu is only shown if two or more durations would be shown.
Add buttons for skipping the duration, which was previously only possible with the hotkeys.
Add Ctrl+Left/Right hotkeys for skipping to chapters.
Add Shift+Left/Right hotkeys for adjusting the skipping time.
The Left/Right arrow keys and the J/L keys work identically for all hotkeys now.
Ignore ctrl, shift and alt keys for demo speed changes with the mouse wheel, to better support actions like zooming being bound to alt+mousewheel etc.
Also handle keypad enter key for play/pause like the normal return key.
Use arrow up/down icon for speed adjustment buttons, so that the "backward/forward" icons can be used for duration skipping instead.
Closes#7064.
Edgar
ChillerDragon
