Add additional checks to ensure that the `CVideo::Stop` function and the functions called by it will correctly stop the current video also if the video was not started successfully, i.e. if `CVideo::Start` returned `false` from any of the error branches.
In the `CVideo::Stop` function, iterate over the vectors of video and audio threads directly instead of using `m_VideoThreads` and `m_AudioThreads`, which do not reflect the actual count if the initialization failed before the threads were created.
In the `CVideo::Stop` function, only call `av_write_trailer` if the video recording was stated successfully, i.e. only if `avformat_write_header` was called successfully, as this will otherwise cause the client to crash. Closes#6375.
In the `CVideo::Stop` function, only call `avio_closep` if the format context was allocated.
In the `CVideo::FinishFrames` function, ensure that the codec has been allocated and opened, otherwise using it is not allowed.
Add assertions to the `CVideo::Start` and `Stop` functions to ensure that the same video is not started/stopped multiple times.
Crash with assertion when the size of the graphics is different from the video currently being rendered, instead of causing weirder bugs and a corrupted video file.
The wrong sampling rate was being used for video recording if the client is not restarted after changing the `snd_rate` config variable.
Ensure that the correct bit rate is used if the sample rate was adjusted because the selected value is not supposed.
Use `log_error` for all errors and consistently format all error messages.
Handle all ffmpeg errors and output the formatted ffmpeg error message when possible.
Register a log callback for ffmpeg log messages to delegate them to our logging system, to fix the log messages being interleaved with our log messages and not using the correct line breaks on Windows.
Stop video and demo immediately and show an error message popup if the video could not be started successfully.
Remove unnecessary debug output from ffmpeg.
Removed member prefix m_ used for local variables.
Removed all b, c, i hungarian notation prefixes for boolean, const and integers.
Fixed local variables using lower_snake_case instead of UpperCamelCase.
Renamed all ``float wSearch = TextRender()->TextWidth..`` to ``float SearchWidth = ..``.
When using the `remove_vote` command, the entire heap of vote options is allocated again without the entry being removed. This allocation was not considering the required alignment for `CVoteOptionServer` objects and potentially wasting space by aligning with `alignof(std::max_align_t)`. When allocating the entries with the `add_vote` command, the alignment is already specified correctly.
Relevant upstream commit:
d2924b5ad6
Closes https://github.com/ChillerDragon/ddnet/issues/7
The snap item obj_character contains a field called m_TriggeredEvents
https://chillerdragon.github.io/teeworlds-protocol/07/snap_items.html#obj_character
It is responsible for effects and sounds. Those flags are set in the gamecore.
So if the servers gamecore ticks twice and resets the flags before a snap is
sent the client misses the information. Which is not too big of a
problem since the client has his own gamecore running (prediction) which
also sets those flags. But it is still wrong and teeworlds does always
include the triggered events in the snap.
So this commit fixes it using the same approach as teeworlds.
By not resetting the triggered events until a snap was sent.
Always use the server address from the network client to reconnect to the correct server after being disconnected due to wrong password. The password popup is only shown after being disconnected, so the server address of the network client should always the address of the server which the client should reconnect to with the password. Using `ui_server_address` was causing the wrong address to be used after being redirected to another server and if the config variable is changed while the password popup is open.
Closes#8260.
Fix leak of pending future logger log messages if the future logger is not set, in particular when the `logfile` config variable is not set or the file could not be opened, by setting a logger that discards all log messages in this case.
Closes#8265.
When changing the screen width, height or refresh rate config variables to 0 or negative values, which are not allowed by the backend, automatically revert the config variables to the actual values again to ensure that the config variables stay in sync with the state of the window. This fixes the client crashing in the graphics settings when setting the screen width and height to 0 via the console, which causes a division by zero when calculating the aspect ratio.
According to the documentation, this function returns `0` on success and a negative number (error code) otherwise, which would cause an allocation of an invalid size.
The third parameter of the `op_read` function specifies the remaining size of the buffer, but we always passed the total size of the buffer without respecting the position at which the data is written into the buffer.
Set the data pointer of the sample only when the sample has been loaded successfully, so the invalid sample data is not freed again when decoding fails.
Otherwise the snapshots may incorrectly be invalidated because a change of the `cl_dummy` variable is detected when connecting to another server after disconnecting while the dummy is active.
Send dummy ready and enter game when connection is online based on its state instead of when the security token is not unknown anymore, which should effectively be the same condition but more understandable. This is also how it's already done for the main connection so the `SecurityTokenUnknown` function can be removed as it's unused now.
The console was not keeping its current scoll position if entries from the backlog are removed due to being recycled when new entries are added. For this purpose, a callback function is added to the ringbuffer to handle popped items, so the scrolling position of the console can be updated based on the number of lines of the removed backlog entries.
Fix backlog corruption in `CConsole::PumpBacklogPending` when many backlog entries are allocated at the same time. When allocating many entries from the `m_Backlog` ringbuffer at the same time, the first entries being allocated may already have been recycled again, so the pointers to them being stored in the temporary vector of new backlog entries were pointing arbitrarily into the ringbuffer data, which could cause corruption of the structural and user data of the ringbuffer. Now, we iterate over the entire backlog and only update uninitialized entries instead of storing the new entries separately.
This was sometimes caught as a misaligned access with UBSan:
```
src/engine/shared/ringbuffer.cpp:160:14: runtime error: member access within misaligned address 0x00014126f4df for type 'struct CItem', which requires 8 byte alignment
0x00014126f4df: note: pointer points here
<memory cannot be printed>
0 0x5825349a6a1c in CRingBufferBase::Prev(void*) src/engine/shared/ringbuffer.cpp:160
1 0x5825334e8934 in CTypedRingBuffer<CGameConsole::CInstance::CBacklogEntry>::Prev(CGameConsole::CInstance::CBacklogEntry*) src/engine/shared/ringbuffer.h:59
2 0x5825334d13e6 in CGameConsole::OnRender() src/game/client/components/console.cpp:1259
3 0x582533bce058 in CGameClient::OnRender() src/game/client/gameclient.cpp:715
4 0x582532f3cc44 in CClient::Render() src/engine/client/client.cpp:894
5 0x582532f9d236 in CClient::Run() src/engine/client/client.cpp:2971
6 0x582533002e5e in main src/engine/client/client.cpp:4523
```
Fix out-of-bounds accesses when rendering ghost players, which use the client ID `-2`. This was causing the hook collision line of ghost players to be affected by real players, which is not correct, as ghosts do not interact with other characters.
Closes#8239.
Add optional filename argument to `start_video` command, to start recording to a video file with a specific filename, instead of always using the current timestamp.
Add log messages to `start_video` and `stop_video` commands to indicate success and errors.
Make the `CClient::StartVideo` function non-`static` and reduce duplicate code in the `Con_StartVideo` function.
Determine the video filename outside of the `CVideo` constructor, same as for demos.
Fix game times and prediction not being updated when only exactly two snapshots have been received, due to the conditions `m_aReceivedSnapshots[...] >= 3`. These specific condition are not necessary and replaced with simpler checks whether the current snapshot is set. Some duplicate nested conditions are also removed.
Pump the network first in `CClient::Update` before updating anything else, to ensure that snapshots are received from the network client before the game times and prediction are being updated based on the current snapshots.
Fix current and previous game tick always being `0` on the first call of `IGameClient::OnNewSnapshot` when two snapshot have been received. Now, the game ticks are properly initialized from the two initial snapshots.
Fix old inputs sometimes being used in prediction after changing map because inputs with tick `-1` were not being ignored.
Ensure all snapshots and game times are properly cleared when entering the game.
`GetConsoleMode` can fail with `ERROR_INVALID_HANDLE` when redirecting output to `nul`, which is considered a character file but cannot be used as a console.
The purpose of this condition is to only update the freeze time every 50 ticks so the freeze bar keeps being refilled after one second when standing in freeze, but it incorrectly prevented freeze from being applied during the first 50 ticks after the map has been loaded. Now, freeze is also applied if not currently frozen, so the behavior directly after changing the map is identical to subsequent respawns.
Clear current server info when playing demos to ensure that the score kind (points, time) is detected correctly. Previously, when playing a demo with points score kind and `cl_demo_assume_race 0`, the score was not shown as points when the last server that the client connected to had a race gametype.
Move the implementation for `cl_demo_assume_race` together with the rest of the demo server info initialization in the `CClient::DemoPlayer_Play` function.
The demo player needs to be rendered also when the menu is inactive, to handle the hotkeys for controlling the demo player and to render the play/pause and speed indicators.
Closes#8208.
These conditions are contained in an `if(State() == IClient::STATE_ONLINE)` branch and the state does not change while inside this branch, so these checks are unnecessary.
Previously, when connecting to servers repeatedly, the local tee would appear at the position it had on the previous server for a short time when joining.
This (and potentially other bugs) are fixed by clearing all game related `CGameClient` member variables in the `OnReset` function. Additionally, the `OnReset` function is now used in the `OnInit` function to ensure everything is initialized correctly when starting the client and to avoid duplicating the code.
In particular, this bug was limited to use of `cl_predict 1`, because the predicted gameworlds were not being reset when disconnecting, causing the predicted world and character from the previous server to be used. This bug was introduced in version 15.6, which added the predicted gameworlds in #1620. Closes#4339.
For the internet/favorite tabs, instead of combining the country/type filters of all communities in one view, track the country/type filters separately for these tabs using the new, reserved community name `all`. This should make the filters less confusing to use, as changing the country/type filters in one tab will not influence the other tabs anymore. Though the country/type filters of the internet and favorite tabs are still combined, as this is also the case for the community filter.
However, this made it possible to select country/type filters that exclude all servers, by first excluding some countries/types and then changing the selected communities so all selectable countries/types are excluded. To prevent this, the filters will now include all countries/types, if they would otherwise exclude all selectable countries/types.
To do this more efficiently, the community cache is moved from the menus to the engine serverbrowser. To avoid using the UI page in the engine serverbrowser, the serverbrowser type is instead used to detect if the community cache should be updated. This required additional changes in the menus to ensure that the UI page and the serverbrowser type stay in sync with each other, which would otherwise cause incorrect server entries to be shown for one frame when switching tabs. The serverbrowser type is now refreshed immediately when the menu page is changed with the `CMenus::SetMenuPage` function, which allowed removing duplicate code for the server browser tab buttons. The `CMenus::RefreshBrowserTab` function does not take the page to be refreshed as argument anymore, as it always was only used to refresh the current page. Instead, a `bool` argument is used to specify whether the refresh should be forced even if the server browser type has not changed.
Closes#8158.
Adding the snap to the demo is the last usage of `pTmpBuffer3`
all prior usages of it copy the data if needed.
So we can edit it in place. No need to copy it into a new buffer.
Remove unnecessary, inconsistent margin at the top of all settings menus.
Reserve space for restart message and button only when necessary, so more height is available for settings menus per default.
Improve vertical alignment of restart message and button with settings menus.
Ensure consistent margin on all sides of settings menus.
Use consistent margins for all settings pages and titles.
Fix chat preview overlapping with the restart warning.
Simplify layout code. Remove array of `CButtonContainer`s with arbitrary size and use individual variables instead.
Simply hide the favorite community tabs in the offline server browser with screen resolutions where not enough width is available.
The ingame server browser can always show at least 5 favorite community tabs with all screen resolutions.
Skip rendering various client components (players, ghosts, nameplates, HUD etc.) while not ingame to improve performance. This increases FPS when rendering the start menu by around 300-400 both in debug and release builds.
This also fixes potential crashes when components using the map data where rendered while the client is loading a new map.
The rcon username requirement was not reset correctly when disconnecting from a server while connecting/loading, at which point the `NETMSG_RCONTYPE` message has already been received.
Closes#4170.
Pass `CImageInfo` arguments to various graphics and text render functions instead of passing the components (data pointer, width, height, format/pixel size) of the image info separately.
Pass texture/file name to loading functions when available to improve error/warning messages.
Split menu rendering explicitly by client state. This makes the code more readable by not mixing code for rendering different states together as much and fixes issues where the menu was being rendered inconsistently for one frame when the state is changed, at the cost of some additional duplicated code.
The fullscreen popups shown when in the connecting and loading client states are now hard-coded for these states, instead of being mixed together with the other fullscreen popups, which can be shown when offline, ingame or during demo playback.
As a side effect of rendering the demo controls like the regular menu, the UI color will now also affect the demo controls menu background.
Move `CBackground::ChangePosition` calls inside functions to reduce duplicate code and improve readability.
Remove unnecessary `CMenus::m_ActivePage` variable.
Closes#7834.
The function `fs_makedir_rec_for` for recursively creating folders did not handle paths containing backslashes correctly and only created folders after every occurrence of a regular slash. This could cause the recursive folder creation to fail when a parent folder is not created before the child folder. For example when recursively creating folders for the path `D:\Games/DDNet\downloadedskins`, the `DDNet` folder was not being created because this path segment does not end in a forward slash. Now, backslashes are handled the same as regular slashes, which is consistent with the other filesystem functions.
Additionally, the function tried to create folders for drive letters on Windows (e.g. `C:` and `D:`). Trying to create a system drive as a folder will fail due to access being denied, whereas it seems to work for already existing non-system drives. For example when recursively creating folders for the path `C:/Games/DDNet/downloadedskins`, the `C:` "folder" could not be created which caused the entire operation to fail. Now, the function will not try to create folders for drive letters anymore, i.e. if the name of the to be created folder ends with `:`.
Closes#8148.
m_FreezeStart is now corrected by the ticks the character was paused
Possible physical changes: Until now it was possible if you sit inside freeze to re-freeze instant
after unpause. Now you re-freeze after the second that was dawned before
the pause. I do not excpect that this is used on any map, and it did not
bring any benifit.
Invalid UTF-8 will not be read from config files with `CLineReader` anymore and the local console never allowed entering invalid UTF-8, so this additional mangling of invalid UTF-8 string config variable values is unnecessary.
The engine graphics do not need to be involved with freeing image data.
The `Free` function now also ensures that all other member variables are cleared when freeing the image data.
The `unknown address` log message got printed very often because many servers have 0.6 and 0.7 addresses but only 0.6 addresses are considered valid by the client.
Support scrolling all scroll regions with the mouse wheel also while popup menus are open. Support overlapping scroll regions by always scrolling the top-most hovered scroll region on mouse wheel events.
The hot scroll region is now tracked separated by `CUi`, as tracking the IDs of all UI elements which are contained in scroll regions is not feasible. The separate active state for scroll regions is therefore unnecessary.
It's still necessary to disable `CListBox`es when popup menus are open, to ensure that only one list box consumes the key events.
Closes#8087. Supersedes #8090.
Add strict validation for `StrToInts` function. Because this function should only be used with trusted internal strings, assertions are added to ensure that the string is not truncated. Some buffer sizes are adjusted accordingly, so truncation cannot happen.
Add less strict validation for `IntsToStr` function. An additional argument specifying the size of the output buffer is added to assert that the size of the output buffer is sufficient. However, because this function is used to decode data sent by the server and read from maps and ghosts, invalid input data should never result in crashes or invalid UTF-8 strings. The function will now unpack an empty string and return `false`, if the string contains invalid UTF-8.
The inline definition of the functions is not wanted, because it requires adding a `system.h` include in `gamecore.h`. Therefore the tools now have to depend on game-shared, which previously only worked because the functions were inline.
Tests are added to ensure the function still behaves the same as before for valid inputs and correctly handles invalid inputs.
`CNetConsole` will now ignore received lines containing invalid UTF-8 codepoints. Previously, it was possible to crash the server on Windows with Econ commands like `exec <invalid UTF-8>`.
Previously, usage of `void *`, `unsigned char *` and `uint8_t *` was mixed in various places for pointers to raw image data and the pointers ended up being cast to `uint8_t *` at some point anyway. Now only `uint8_t *` is used consistently, which improves type safety and readability. Casts to `uint8_t *` are now only necessary when using `malloc` or when reading data from a map.
Show completion options for key names for all bind commands (`bind`, `binds`, `bind_reset`) in the local console.
At the moment this only works for completing the first key name but not for composite binds that use `+`, which would work differently than other console argument completion.
Unnamed keys (starting with `&`) are not shown as completion options.
Refactor console argument completion to reduce duplicate code.
We want to change the state of the job from `STATE_QUEUED` to
`STATE_RUNNING`, but not from `STATE_ABORTED`. Previously, the code
usedd `exchange` to change the state and compared non-atomically
afterwards. This led to `STATE_ABORTED` being replaced by
`STATE_RUNNING` in a race. With `compare_exchange_strong`, this can no
longer happen.
Fixes#8044.
Fix HTTP client effectively shutting down by entering the `ERROR` state when a request cannot be added. Now only the invalid request is aborted immediately. The `ERROR` state is now only entered when a curl function fails in a way where recovery is not possible.
Fix occasional deadlock when HTTP client is shutting down after entering the `ERROR` state, by also immediately aborting new requests when the HTTP client is already in the `ERROR` state.
Remove unused `CHttp::EState::STOPPING`.
Prevent the `tune_zone` command from being shown and used in the local console, which could cause issues with prediction, by only registering it as a game setting but not as a client command. The command callback is still correctly called when map settings are loaded in the `CGameClient::LoadMapSettings` function.
Previously:
```
2024-03-03 12:17:40 I git-revision: 885ae7ecae
2024-03-03 12:17:40 I server: version 18.0.3 on linux amd64
2024-03-03 12:17:40 I server: git revision hash: 885ae7ecae
```
The color selection should only be shown for normal envelope points but not for bezier control points, because it always changes the previously selected normal envelope point. This caused the client to crash when right-clicking a bezier control point when no normal envelope point was previously selected.
Closes#8020.
When image blitting is supported by the Vulkan backend, the color picker was reading incorrect pixel values, because the offset positions for the blitting region are the positions of the top-left and bottom-right corners, but instead the top-left offset and size (width, height) were passed as arguments.
Closes#8040.
Only building the switch entities layer when the current map has a switch layer (#8011) also does not work, because the entities textures are cached for each type and not reloaded unless the entities are changed manually. First joining a server with a map that does not have a switch layer will cause the textures for the type of that server to be built without the switch entities layer, so the switch entities texture will be missing when joining a server of that type with a map that does have a switch layer.
Instead, the switch entities layer textures are always built now, so the cached entities textures are can be used on all maps of the respective server type. This is expected to slightly increase the total memory usage after joining multiple servers of different types. As before, tiles which are unused are masked unless `m_DontMaskEntities` is set.
Instead of defining the macro `WIN32_LEAN_AND_MEAN` and sometimes also the macro `_WIN32_WINNT` in each file that directly or indirectly includes `<windows.h>`, only define these macros once consistently in `CMakeLists.txt`.
Also define `NTDDI_VERSION`, which is the new macro to specify the minimum Windows version starting with Windows Vista. This macro needs to be defined in addition to old `_WIN32_WINNT` macro, according to the documentation.
See https://learn.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers
Assert on failures in all `thread_*` and `sphore_*` functions on all operating systems instead of only printing log messages on Unix, as these functions are only expected to fail when used with incorrect arguments or in some cases when a dead-lock is detected.
On macOS, `sphore_wait` was not correctly calling `sem_wait` in a loop to repeat the wait operation if it is interrupted by a signal.
On Windows, the AIO tests were failing with the additional assertions. The maximum count that semaphores on Windows could be incremented to was previously, arbitrarily limited to 10000, which was causing the `ReleaseSemaphore` call to fail as the AIO semaphore is signaled 65536 times (for each write operation) in multiple of the AIO tests.
- Remove `ValidateFCurve` function because it's small and only used once.
- Remove unnecessary checks in `SolveBezier`, as all of these conditions are already checked before the function is called.
- Remove unnecessary double negation of `InTang` to improve readability.
- Use `double` literals for `double` comparisons instead of `float` literals.
- Fix comments.
There is no need to divide the times by 1000 when evaluating bezier curves, as all times are relative and the division adds significant inaccuracy, to the point where evaluation of bezier curves goes completely wrong in some cases.
Closes#8005.
Add `IJob::Abortable(bool)` function which jobs can call to specify whether they can be aborted. Jobs are not abortable per default. Abortable jobs may have their state set to `IJob::STATE_ABORTED` at any point if the job was aborted. The job state should be checked periodically in the `IJob::Run` function and the job should terminate at the earliest, safe opportunity when aborted. Scheduled jobs which are not abortable are guaranteed to fully complete before the job pool is shut down. However, if the job pool is already shutting down, no additional jobs will be enqueue anymore and abortable jobs will immediately be aborted.
In particular, the sound loading, community icon loading, master chooser and host lookup jobs are specified as being abortable. Conversely, the jobs saving replay demos, editor maps and screenshots are expected to finish before the client is shut down.
When the client is quitting/restarting, it will now disconnect from the current server first, before saving the config, to ensure that any actions that happen on disconnect (demo recorders being stopped etc.) happen first. The shutdown message is rendered before disconnecting and waiting for background jobs to finish.
The HTTP client is now initialized later during server launch, after the network initialization. Error handling is added and the server stops if the HTTP client could not be initialized, same as the client.
The `RunBlocking` functions are removed, as they are not used anymore after curl-multi was added.
The function `IJob::Status` is renamed to `State` and `IJob::STATE_PENDING` is renamed to `STATE_QUEUED` for consistency with naming of the HTTP client.
The member variables of the engine interface are encapsulated and the `jobs.h` include is removed from `engine.h`, which removes transitive includes of `system.h`.
Documentation for all job and job pool API is added.
Instead of only building the switch entities when the server uses the DDNet/DDrace type. Fix switch entities not being shown anymore on servers which do not mask entities. Regression from #7979.
Call `EnvelopeEval` functions directly instead of passing them and their arguments to `CRenderTools::RenderTilemap` and `CRenderTools::RenderTileRectangle`.
Only evaluate color envelopes for tiles layers once instead of separately for the opaque and transparent passes.
Only evaluate relevant number of envelope channels instead of always evaluating all channels.
Avoid unnecessary calculations by only evaluating position envelopes for quads which are not fully transparent.
Fully ignore layer color and envelope color for entities layers, as these cannot be specified in the editor and should not be changeable.
Remove duplicate and insufficient checks for invalid envelope index before calling `EnvelopeEval`. Instead, set the correct default for all channels before calling `EnvelopeEval` and only change the result on success. Now, white color will consistently be assumed for invalid color envelopes, zero positions and rotations for invalid position envelopes, and full volume for invalid sound envelopes.
Validate number of envelope channels to prevent crashes. When loading maps containing envelopes with invalid number of channels (not equal to 1, 3 or 4), the number of channels of these envelopes is reset to 4 and an error message is displayed, so the mapper can examine all channels' data and transfer it to another envelope if necessary. Closes#7985.
Use consistent margins for all settings pages and titles (except the Appearance settings, which will be covered in the future).
Fix checkbox UI element ID variable `s_LowerRefreshRate` not being `static`.
Improve readability of layout code.
Matching the requester allows "reliable" and "unreliable" pings.
"Reliable" pings suffer from re-sends, thus might not accurately reflect
the latency, "unreliable" pings might not arrive at all.
If the own ghost is the last element in the vector and deleted due to using the `cl_race_ghost_save_best 1` setting then the following accesses with index `Own` were out-of-bounds. Closes#8003.
Add mandatory Boolean attribute `has_finishes` to every community info, which specifies whether finishes can be shown for the community, regardless of whether any finishes are currently available for the player.
The community info must be adjusted when/before merging this, by adding the attribute `has_finishes` to every community object, with the value `true` for DDNet and `false` for all other communities.
Closes#7957.
The check before calling `normalize` incorrectly excludes skins containing zero in any color component instead of excluding only skins with zero in all components. The check can be removed entirely, because it is already checked inside the `normalize` function whether the length of the `vec3` is zero, in which case a zero `vec3` will be returned.
For very large skins which use large color values in at least one component, the `int` used for calculating the blood color could overflow.
Prevent skins with invalid names from being loaded/downloaded.
Improve log messages when skins cannot be loaded.
Remove obsolete check for duplicate skins, as the storage handles duplicate files already.
Instead of adding the placeholder skin to the list of skins only when no skins have been loaded, always create the placeholder skin and use it only when no other skin is available.
Use reasonable values for skin metrics of placeholder skin to improve its rendering.
This adds highlighting color when hovering the main menu buttons (File, Tools, Settings), which was previously missing for these buttons. This also reduced duplicate code.
Extract font size for menu buttons into constant `MENU` in new namespace `EditorFontSizes`.
Use `DoButton_FontIcon` with `FONT_ICON_MINUS`/`FONT_ICON_PLUS` consistently instead of using `DoButton_ButtonDec/Inc` with text labels `+`/`-`.
For yes/no buttons, use `DoButton_Ex` and specify the button corners explicitly instead of using `DoButton_ButtonDec/Inc`.
Reset dragging operation when the dragged button has been unset and reset dragged button when operation has been reset, to prevent the editor from being stuck in either of two inconsistent states. This could be reproduced easily by clicking very quickly while moving the mouse up and down over the groups/layers list.
Do not call `CheckActiveItem` on a UI element (the dragged button) which might not be in view, as this prevents the UI from detecting the invalid active item, when groups/layers are dragged very far and quickly outside the scrollregion.
Closes#6855.
Immediately cancel new HTTP requests instead of enqueuing them when already shutting down `CHttp`. Otherwise, `CChooseMaster` may wait forever for HTTP requests to be completed while the client is shutting down, if `CHttp` was shutdown while `CChooseMaster` is not waiting for an existing HTTP request.
Not directly caused by #7962, but it made it more likely to happen.
Closes#7980.
The combined entities texture for all layers except the switch layer was only built when the gametype has a front, speedup, tele or tune layer, which is only the case for servers declaring DDNet or DDRace type in their gameinfo. Now this entities layer is always built, as the game layer is always present.
Expose UI element ID for the main map editor as member variable so the select layer by tile funtionality can correctly be enabled when the map editor is the hot item.
Add/update doxygen comments for all `shell_*`, `os_*`, `secure_*`, `cmdline_*`, `windows_*` and related functions.
Move `open_link` and `open_file` declarations to shell group, to the same relative position as their function definitions.
Instead of sending the message from the first client which is not ingame, choose the first client which is fully empty, to make it less likely that the message is sent from a currently connecting client.
Add a country "none" with tee flag (code `-1`) and a type "None" for all servers without community.
Previously, the country and type for these servers was unset and their handling special-cased, so any non-empty country/type filter would exclude servers without country/type information. Now individual countries/types can be excluded without also excluding all servers without community.
Closes#7961.
It was possible to create a community/country/type filter excluding all entries when the list only contains one entry and right-click is used to deselect it.
Shutdown `CHttp`, i.e. abort all HTTP requests, as early as possible when quitting/restarting the client, after the config has been saved but before shutting down the gameclient. Previously, this was delayed until the engine shutdown, so the requests would often finish entirely instead of being aborted by the callback.
Previously, HTTP requests being aborted due to `CHttp` shutting down were considered `EHttpState::ERROR`, which sometimes causes deadlocks during engine shutdown or destruction of `CHttp`. The state is now set to `ABORTED` by passing `CURLE_ABORTED_BY_CALLBACK` to `OnCompletionInternal`. The otherwise unused handling of internal errors is removed.
Previously, it was assumed `ResultJson` would return `nullptr` for HTTP requests which have been aborted or failed with an error, which would trigger the newly added assertion error "Request not done".
Directly use the bounding box which has already been calculated for the text containers, instead of calculating the text width again for every info message text.
Reduce duplicate code by moving screen remapping and validation of the info message into the `AddInfoMsg` function.
Check for valid kill message before creating text containers, so they are not created and deleted unnecessarily.
Assign info message type directly instead of passing it as a separate parameter.
Combine `CreateNamesIfNotCreated` and `CreateFinishTextContainersIfNotCreated` functions to `CreateTextContainersIfNotCreated` function to reduce duplicate code on usage.
Extract duplicate code for determining name color in info messages as lambda function.
For consistency between 0.6 and 0.7 clients, also show kill messages for 0.7 clients, now that DDNet client also supports finish messages.
Closes#5623.
Instead of preventing the client from quitting/restarting while a warning is shown in the menus, add warnings that should be shown after quitting/restarting (i.e. the warning when the config could not be saved) to a separate list and show these warnings using an OS message box after the client has been closed. Otherwise, the client is prevented from closing if a warning is shown without being automatically hidden, which causes the client to hang indefinitely in the CI.
The message boxes for warnings must be shown after the client has already been completely shutdown, otherwise the regular shutdown with the Vulkan backend crashes because showing the message box has already partially deinitialized the backend.
The quitting/restarting client state is now checked after updating the FIFO component, so quitting/restarting initiated via FIFO is effective immediately, although this should have little effect in practice.
For completeness, a log message is added also for the case that the config was saved successfully.
The GitHub CI seems to automatically confirm/disable OS message boxes, so they should not block workflows.
Add assertions to ensure that the HTTP request result data and SHA256 are available when getting them instead of returning `nullptr` and `SHA256_ZEROED` when they are not.
Rename `CHttpRequest::Sha256` function to `ResultSha256`.
Because of incorrect index/size math, two favorite communities were removed when exceeding the maximum number of three favorite communities instead of only one. The check can be simplified because the maximum number of favorite communities can never be exceeded, so at most the first element needs to be removed from the vector.
Closes#7935.
Avoid many writes to disk each time the DDNet info is downloaded, i.e. each time the refresh button is pressed in the server browser, by first loading the DDNet info into memory and only writing it to disk when it differs from the current DDNet info based on the SHA256 hash.
The SHA256 is now also used to track whether the DDNet info was modified for the community filters and icons instead of using the current time for this purpose.
Closes#3941.
Initialize the HTTP client (start the HTTP thread) after initializing the network client instead of during the client interface construction. This ensures that config variables have been loaded already and the log output level is set correctly. This also means all library initialization log messages appear together in one block when starting the client.
Show an error message box and stop launching when the HTTP client could not be initialized, instead of launching but not being able to perform any HTTP requests, which causes bug reports about the serverlist being empty.
Both console and chat commands are sent to clients dynamically with respective messages, so the static lists of commands were only used for servers not making use of these messages. Instead of assuming potentially incorrect console and chat commands on those servers, the lists will now be empty for those servers.
Move the command registration from the `ddracecommands.h` and `ddracechat.h` header files to the `CGameContext::RegisterDDRaceCommands` and `CGameContext::RegisterChatCommands` functions and delete the header files. The `CHAT_COMMAND` and `CONSOLE_COMMAND` macros are removed, because they only add unnecessary indirection now. The strings `CHAT_COMMAND` and `CONSOLE_COMMAND` are simply replaced with `Console()->Register` and semicolons are added at the end of the lines.
Closes#7665.
Support adding up to three communities as favorites in the server browser. Favorites can be changed with favorite buttons which are shown in the community filter on the Internet and Favorites tabs. The commands `add_favorite_community` and `remove_favorite_community` are added to change the favorite communities via the console and for saving the favorite communities to the config file. For the favorite communities, additional tabs using the communities' icons are shown in the server browser next to the Internet, LAN and Favorites tabs. Each community tab shows only the servers from the respective community, hence the community filters UI is not shown on the community tabs but only on the Internet and Favorites tabs. The country and type filters on community tabs cover only the countries and types from the respective community. Favorite communities are added from left to right. When more than three favorite communities are added, the oldest (leftmost) favorite community will be removed from the list.
When starting the client for the first time, i.e. with `cl_show_welcome 1`, the DDNet tab will be created as the only favorite community and selected initially. The community, country and type filters are unset when starting for the first time, so the Internet tab now shows all servers per default.
When starting with a `ui_page` for a favorite community that is not configured, the page is reset to the Internet tab. This also affects those who upgrade from versions with the old DDNet and KoG tabs. The server browser is now also correctly updated when changing `ui_page` via the console.
Track country and type filters for every community separately, to avoid filters resetting when switching between community tabs or changing the community filter. The commands `add_excluded_community`, `remove_excluded_community`, `add_excluded_country`, `remove_excluded_country`, `add_excluded_type` and `remove_excluded_type` are added to change the exclusion filters via the console and for saving the exclusion filters to the config file.
Render community filters above the toolbox (filter, info and friends) tabs when on the Internet and Favorites tab, so this setting is more visible and can be changed also when the other toolbox tabs are selected.
Add icon for the none community, based on the tee country flag color. This icon is hard-coded in the client, as the none community also is, so fetching the icon from the server would be inconvenient. Load community icons already when rendering the menu instead of only when rendering the server browser, so the icons are immediately available when using the start menu.
Find tutorial server by searching for community type "Tutorial" instead of searching for "(Tutorial)" in the server name.
Avoid cleaning favorite communities and filters when there are no communities, i.e. when the DDNet info failed to be loaded or does not contain any communities, to avoid losing all favorite communities and filters in this case.
Closes#7774.
When empty PNG files are loaded, the `std::vector` for the file contents is resized to size 0, which results in undefined behavior when it is accessed with `front`.
When `io_tell` fails, i.e. returns `-1`, this was incorrectly cast to an `unsigned` and therefore caused a very large allocation and potentially crashes due to lack of memory.
Add `CGameClient::RefreshSkins` function to refresh skins. This function reloads all skins by calling `CSkins::Refresh` and then notifies all gameclient components about the skins being refreshed by calling the new `CComponent::OnRefreshSkins` function, so the components can properly invalidate their current skin texture handles. The existing `RefindSkins` functions are changed to `OnRefreshSkins`.
Additionally, `OnRefreshSkins` is overridden in `CMenus` to set the flag so the skin list will be updated before it is rendered the next time, to fix the client crashing when changing skin related config variables via the console. Closes#7891.
Reduce height and empty space and make margins consistent. Slightly increase width for better round corner drawing and to support longer vote descriptions.
Auto-scale description and reason labels and show ellipsis if they don't fit.
Show circular progress indicator for remaining vote time.
The functionality to draw text on the voting bars is never used, so the parameter `Text` is always `false`.
Remove unused variable `PassArea`. The remaining area of the voting bars is unused.
Use error log level for error messages. Use info log level explicitly instead of using `dbg_msg`.
Log time taken for initialization in trace level message.
Use `IGraphics::CORNER_NONE` instead of `0`.
Use `nullptr` instead of `NULL`.
Remove arguments identical to defaults.
Use loops for tab bars with several tabs.
Add parameter to `shell_execute` to either start the process in the foreground or background on Windows. Previously, all processes were started in the background, because this is desired when starting the server from the client. However, this causes the graphics initialization to fail when restarting the client after updating or with the `restart` command when using Vulkan with windowed and windowed fullscreen mode.
Closes#6578.
Render an additional "Reset filter" button below the "No servers match your filter criteria" message to make it more obvious for new players how to restore the original view of servers, especially when the filter tab is not currently selected.
The `bool AllowResizing` parameter is only passed to other `SetWindowParams` functions but never used in the end, as whether or not resizing is allowed is separately determined based on the value of the `int FullscreenMode` parameter.
Instead of cleaning the countries/types filters based on all available communities' countries/types, only consider the countries/types of currently selected communities. Ensure countries/types are always cleaned when updating the server browser filter in the UI. This fixes that countries/types which are not available for the selected communities were still affecting the server filtering, causing no servers to be shown in some cases.
Closes#7847.
To simplify the usage of the demo recorder, parameters are added to the `IDemoRecorder::Stop` function to control whether the demo file is removed or renamed when stopping the recording.
Ensure demo files of the replay recorder are always removed (except on crashes). The temporary replay demos were previously not removed when being disconnected from a server.
Fix auto demos being stopped and restarted when enabling replays in the settings menu. Now only the replay recorder is stopped/started when necessary.
Fix replay recorder being restarted when setting `cl_replay` variable with console without changing the value.
Remove unnecessary `CClient::m_ButtonRender` variable. Demo rendering is already stopped correctly when reaching the end of the demo without this additional variable.
Remove unused `CDemoRecorder::m_pMapData` variable.
Instead of leaking the m_MoveRestrictions to all classes which needs to
adjust the Character velocity, add a setter which takes into account those
restrictions.
The shotgun bug replication requires an access to the Velocity without the
restrictions applied (so we have to have this dirty setter).
Expose only *const* CCharacterCore to force the setters usage and prevent
incorrect write.
Ensure that the `CHttpRequest::OnCompletion` function has finished before updating the state which is visible to other threads. Otherwise, threads may try to access the result of a completed HTTP request, before the result has been initialized/updated in the `OnCompletion` function.
Fixes warning due to invalid texture width/height being shown for downloaded skins, because the texture was loaded before the image data was initialized. Closes#7818.
Regression from #7683. Before this, the `OnCompletion` function could also modify the HTTP request's state, which was used to check if loading the skin PNG failed. Instead of doing this, a separate check is added for the skin download task to check that the PNG was loaded successfully.
Use [virtual terminal processing](https://learn.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences) for Windows console output when possible, using the existing async logger. Fallback to the old Windows logger when virtual terminal processing is not available. Do not set any logger when there is no console/file/pipe to output to.
This adds support for 24 bit RGB colors using ANSI escape codes (when Terminal emulator supports it) instead of only supporting 16 fixed colors.
Use the existing async logger also on Windows by converting the console standard output handle to an IOHANDLE. Avoid converting console output to UTF-16 by changing the console output codepage to UTF-8 and outputting that directly. Overall, this reduces the time until the loading screen first appears when launching the client from the console on Windows by around 70%, from 80-100ms down to 25ms.
Add support for `NO_COLOR` environment variable also on Windows.
Use the async logger also for writing output to files and pipes instead of using a Windows specific implementation for this, to reduce lags when redirecting the standard output.
Closes#3925. The client/server will not freeze anymore when entering selection mode in the console because we are using the async logger on Windows now.
Closes#1108. We were already using AIO on Windows to write the logfile, which was not causing any known issues. We are now also using AIO with the async loggers for console/files/pipes on Windows.
On Windows, the result of the `localtime` function is thread-local, hence our uses of this function are already thread-safe.
On non-Windows, the `localtime` function is potentially not thread-safe, so the reentrant `localtime_r` function is used instead with a thread-local buffer for the result.
Closes#7788.
When handling door and laser entities, all 8 adjacent tiles are checked for laser modifier tiles. When handling laser entities, the 8 tiles one further out are also checked for laser modifier tiles. If either of those 16 tiles is outside the map bounds, an error message was printed and the tiles are ignored. The error message is removed because it's unnecessary and misleading. Placing entities near or on the map border is not known to cause any issues.
Closes#7759.
Store X value (time) for all graph entries in addition to the Y value (FPS, prediction margin etc.). The `CGraph::Add` function adds values to the graph at the current time. The `CGraph::InsertAt` function allows specifying arbitrary X values, as long as the values are inserted in increasing order.
The entries are kept in a ringbuffer and old entries are recycled when it's full. The size of the ringbuffer is configurable for each graph, as the FPS graph needs significantly more buffer because values are added more often.
The scrolling speed of the graphs is fixed by specifying the maximum size of the window of values which should be displayed. For this purpose, a parameter is added to the `CGraph::Scale` function to specify the size of the window which should be rendered in the `CGraph::Render` function. For the FPS graph only the last second is rendered, so small spikes are still noticeable. For prediction and gametime margin graphs the last five seconds are rendered, which should result in a similar scrolling speed as before this change. The debug tuning graph is a special case, where the X values set manually and fixed to 0-127, same as before, instead of being based on the current time.
The graph rendering is made much more efficient by precalculating when the vertex colors need to be updated, to avoid all unnecessary calls to `SetColorVertex`. Additionally, line items are bundled together in an array to avoid calling `LinesDraw` for every individual line item.
Center the resize (V/H) cursors on the mouse position instead of aligning the top-left with the mouse position. This feels better to use and is consistent with the behavior for example on Windows.
Fixed crash when runing tool 'Remove unused envelopes' and trying to undo the addition of an envelope (#7738)
Fixed issues where undo/redo would not work properly when changing a layer's order or group property
Added missing action for changing the position of a sound source
The `CTextCursor::m_LineCount` variable is only updated after the cursor is rendered. For calculating the selection quads we need to check `LineCount` instead, which is updated while the cursor is being rendered. Regression from #7733.
Use yellow-orange star icon to mark favorite servers in the server browser instead of using the heart icon. The heart icon is now only used for servers with friends.
Tweak color of the star icon used for skin favorites so it's slightly more orange instead of pure yellow.
Free up space for up to three more server browser tabs of the same size (on 5:4 resolutions), i.e. tabs for configurable communities.
Remove the additional server browser icon tab on the right side. Remove the purely visual Demo and News tabs which were only shown when the respective page is already active. Instead, always show all server browser tabs when offline.
The `IClient::Disconnect` function is always called at the beginning of the `IClient::Connect` function. The `IClient::Disconnect` function calls `IClient::DisconnectWithReason`, if the client is not already offline, which performs cleanup of temporary commands and demo recorders already, so the same cleanup in the `Connect` function is unnecessary. An assertion is added to ensure that the client was properly disconnected before connecting. Connecting while the client is already quitting or restarting is now prevented, as the client state cannot be changed once set to quitting or restarting.
By adding `CDataFileWriter::ECompressionLevel` to replace usage of zlib internal compression levels in the `CDataFileWriter` API.
Use `std::numeric_limits<int>::max()` instead of `INT_MAX` in one case where the latter was only declared by the transitive zlib include. The `limits` header is already included and its use is more fitting for C++ code.
The server community was only initialized when receiving server info. This caused servers for which no server info is received to be hidden when using the community filter. Now the community filter also works correctly for servers for which no ordinary server info is received, by initializing the community already when adding server entries to the list.
Closes#7776.
Adapt the `CDataFileReader::GetItem` function so it optionally also returns a `CUuid` for UUID-based map items, including for items with unknown UUIDs where the item type will be `-1`. Adapt the `CDataFileWriter::AddItem` function so it optionally also accepts a `CUuid` which will be used if the item type is `-1`.
The additional checks for invalid map item types in the map tools are removed again and instead the new UUID parameters are used so map items with unknown UUIDs are written back to maps correctly.
Closes#7701.
Files were not being truncated on Windows anymore when using `io_open` with `IOFLAG_WRITE` due to a regression from #7254. Instead, the existing file contents were kept and the file pointer was set to the beginning of the file.
This caused broken demo files to be created (#7349) when recording a shorter demo with the same filename as an existing longer demo. It also caused the map tools to produce maps with additional junk data at the end, if an existing map is overridden by a smaller map.
This is fixed by using the creation disposition `CREATE_ALWAYS` instead of `OPEN_EXISTING` with [`CreateFileW`](https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew), which ensures that a file is always created and truncated.
A regression test case is added, which fails without this change.
Add separate `IGraphics::LoadTextureRawMove` function with non-`const` `void *pData` argument in addition to existing `LoadTextureRaw` function with `const void *pData` argument. The former function takes ownership of the data and avoids copying the texture data into an additional buffer, if the texture data is already in RGBA format. Non-RGBA texture data always needs to be converted and therefore also copied.
The `LoadTextureRaw` function is split into smaller functions to share common code with the `LoadTextureRawMove` function. Alternatively to this, a flag `TEXLOAD_MOVE_DATA` could have been added to the existing `LoadTextureRaw` function, which would have required the use of `const_cast` to free the texture data.
Unset the assertion handler before shutting down the kernel (including graphics) and client. Otherwise the assertion handler itself crashes when assertion errors happen after the graphics/client have been destroyed.
The selection in the kickvote/specvote menu was reset to the first player when the previously selected player leaves or if the filter string does not match it. Now the selection is cleared in these cases, to prevent players from accidentally starting kickvotes/specvotes on the wrong player.
Remove unnecessary `static` variable. Improve readability by not nesting post-increment in another statement.
Add color palette with up to 8 colors to editor toolbar. The palette colors work like regular color picker buttons, so they open a color picker popup on click and the value can be copied and pasted with Ctrl+Right click and Ctrl+Left click respectively. Less palette colors are shown when not enough space is available (with 5:4 resolutions).
Add color pipette which allows selecting any color displayed on the screen. Selecting a color with the pipette adds the new color to the palette and shifts the other colors to the right. The selected color is also copied to the clipboard immediately. The hotkey Ctrl+Shift+C is added to toggle the color pipette, which allows using the color pipette in popups and dialogs.
The implement this, the function `IGraphics::ReadPixel` and the command `SCommand_TrySwapAndReadPixel` are added to read a specified pixel's color from the backbuffer. Like the screenshot command, this command also requires a swap operation to be performed before the correct pixel color can be read with the Vulkan backend. The `ReadPixel` function therefore accepts a pointer to a `ColorRGBA` that will be filled after the next swap operation.
Closes#7430.
Replace linear search for free sample index with free list. This brings sound sample index allocation down to constant complexity independent of the number of allocated sounds. On average the time to allocate sound samples is reduced by around 75% (843µs down to 223µs). For perspective, the time to load all default sounds on client launch is reduced by around 15ms (although this does not significantly affect launch time due to threaded loading).
The lock needs to be owned when accessing the sound voices. Calling `IsPlaying` is redundant, as the loops effectively check whether the sample is playing.
Ensure that the UI mouse position stays inside the UI screen rect. Previously, the mouse was not considered to be inside the UI screen when all the way at the right or bottom edge. This caused the map editor tooltip to not be shown when the mouse is all the way at the right side of the map editor view. This is a cleaner fix for #4553 which reverts the previous workaround from #6423.
Add dropdown menus for changing animation and grid settings (i.e. animation speed and grid size) instead of conditionally showing more buttons for this directly in the menu bar. This frees up space in the menu bar, which is currently full on 5:4 resolutions.
The icons previously used for the default animation/grid buttons are now used for the main buttons that toggle animation/grid instead of using text.
Support setting lower animation speeds with the plus and minus buttons by adjusting the step size when the animation speed is low.
Support setting arbitrary animation speed by text input.
Render one quad for each line of the text selection instead of rendering one quad per selected character.
This increases the average FPS when the console is open and all text is selected by around 10% (from around 849 to around 943 FPS) (on my machine, in release mode).
When normalizing color components in the engine graphics, round the components to the nearest integer instead of rounding down. Otherwise the color that is rendered in color pickers may be off by 1 in any of its RGB components from the color that the color picker displays as text (hex string and individual components). The slightly incorrect color can be confirmed by creating a screenshot or otherwise reading the backbuffer (planned editor pipette feature).
This should not change map rendering, since maps already store quantized RGBA values on which the rounding mode should have no effect. It may however slightly change appearance of colors in all other places (at most +1 in every RGB component).
The individual color components were rounded down when being displayed on their own in color pickers, whereas the components are rounded to the nearest integer when packing the colors into the hex string. This was causing minor discrepancies between the color being displayed/saved as hex and the individual components. Rounding the components when packing is necessary to reduce the error when converting between color spaces.
Add tabs to player and tee settings pages to switch between player (main) and dummy settings instead of using a checkbox for this, to improve the usability.
Improve layout of tee settings page, especially with 5:4 resolutions, where the eye selector previously overlapped with other UI elements. Decrease size of eye selector and make better use of empty space overall. Use more space for skin selector instead of showing empty space when "Use custom colors" is disabled.
Show favorite skin buttons also when hovering list items to make it easier to discover the feature. Add small highlight color to favorite skin buttons when hovering them.
The text color was previously set and reset for every individual number being rendered for non-empty tele, speedup, switch and tune tiles. The color is the same for all tiles from each entities layer, so most of these `TextColor` calls are unnecessary. Now the text color is only set and reset once when rendering each entities layer.
Let's say you have this bind:
```bind x +toggle cl_dummy_hammer 1 0```
and you set cl_dummy_control to 1.
When you press the bind i mentioned above, and then release, the dummy will hammer where he is looking (not at you). So, in total, there will be two hammers. One hammer when you press down the button and the dummy hammers towards you, and then another hammer when you release the button and the dummy hammers where he is looking.
This fixes it, and also makes sure it does not conflict with cl_dummy_copy_moves (as if it is enabled and cl_dummy_control is enabled, the dummy will not copy fire, hook, or jump) so I made sure it keeps this functionality as it's pretty cool.
This does not fix any other bugs yet, maybe I will fix those in the future but we'll see. Any bug you may encounter with this change is also probably present in the main branch, such as resetonswitch not working perfectly with dummy_control, but if you do find something different then let me know.
According to the Vulkan specification, the struct `VkBufferImageCopy` is used only for `vkCmdCopyBufferToImage` and `vkCmdCopyImageToBuffer`. The variable `Region` is only initialized but not passed to either of those functions.
ChillerDragon
Edgar
