The client crashes when trying to unpack a packet that has the sixup flag set, as `CNetClient` does not pass pointers for the output parameters `pSecurityToken` and `pResponseToken` to `CNetBase::UnpackPacket`.
Since the client does not handle sixup packets, checks are added to return an error and ignore the packet instead of crashing due to a null pointer access.
This was found by fuzzing the data returned by `net_udp_recv` with radamsa.
```
==6200==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f8d0fb8ba56 bp 0x7ffcbf67c7f0 sp 0x7ffcbf67c7a8 T0)
==6200==The signal is caused by a WRITE memory access.
==6200==Hint: address points to the zero page.
0 0x7f8d0fb8ba56 (/lib/x86_64-linux-gnu/libc.so.6+0xc4a56)
1 0x563a7e250fbe in mem_copy src/base/system.cpp:208
2 0x563a7e1bc6b6 in CNetBase::UnpackPacket(unsigned char*, int, CNetPacketConstruct*, bool&, int*, int*) src/engine/shared/network.cpp:263
3 0x563a7e1bf57e in CNetClient::Recv(CNetChunk*) src/engine/shared/network_client.cpp:100
4 0x563a7cfa76a2 in CClient::PumpNetwork() src/engine/client/client.cpp:2546
5 0x563a7cfb7cf6 in CClient::Update() src/engine/client/client.cpp:2838
6 0x563a7cfcfe47 in CClient::Run() src/engine/client/client.cpp:3214
7 0x563a7d04c631 in main src/engine/client/client.cpp:4702
8 0x7f8d0faf0d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
9 0x7f8d0faf0e3f in __libc_start_main_impl ../csu/libc-start.c:392
10 0x563a7cb28754 in _start (build-asan/DDNet+0x2472754)
==8315==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f4accfe5a56 bp 0x7ffcf1318530 sp 0x7ffcf13184e8 T0)
==8315==The signal is caused by a WRITE memory access.
==8315==Hint: address points to the zero page.
0 0x7f4accfe5a56 (/lib/x86_64-linux-gnu/libc.so.6+0xc4a56)
1 0x560413603200 in mem_copy src/base/system.cpp:208
2 0x56041356d9c7 in CNetBase::UnpackPacket(unsigned char*, int, CNetPacketConstruct*, bool&, int*, int*) src/engine/shared/network.cpp:224
3 0x5604135717c0 in CNetClient::Recv(CNetChunk*) src/engine/shared/network_client.cpp:104
4 0x5604123597e2 in CClient::PumpNetwork() src/engine/client/client.cpp:2546
5 0x560412369e36 in CClient::Update() src/engine/client/client.cpp:2838
6 0x560412381f87 in CClient::Run() src/engine/client/client.cpp:3214
7 0x5604123fe771 in main src/engine/client/client.cpp:4702
8 0x7f4accf4ad8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
9 0x7f4accf4ae3f in __libc_start_main_impl ../csu/libc-start.c:392
10 0x560411eda894 in _start (build-asan/DDNet+0x2472894)
```
Demo seeking for percent positions and relative time was choppy, when the first and last ticks of the demo are very large but close together (e.g. with 1308908156 to 1308905658, which are close to integer limit).
During the calculation of `WantedTick` both operands were promoted to `float`s, which caused the information of the smaller operand, i.e. the seeked percentage or relative time, to be mostly lost, so seeking was very inaccurate.
This is fixed by rounding the `float` operand to `int` before adding it to another `int`.
The types are translated to `TYPE_ALL`/`TYPE_SAVE` respectively if a given path is relative and to `TYPE_ABSOLUTE` if a path is absolute.
These types are only supported with the `OpenFile`, `ReadFile`, `ReadFileStr` and `GetCompletePath` methods.
This reduces duplicate code when calling the methods.
5917: report extra player info to master http r=def- a=edg-l
couldn't test yet
this doesnt show extra info on the client server list yet, just makes the server report it
<!-- What is the motivation for the changes of this pull request? -->
<!-- Note that builds and other checks will be run for your change. Don't feel intimidated by failures in some of the checks. If you can't resolve them yourself, experienced devs can also resolve them before merging your pull request. -->
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Edgar Luque <git@edgarluque.com>
Fix warnings with `-fstrict-aliasing` and `-Wstrict-aliasing=2` by using char array instead of array of char pointers:
```
src/engine/client/client.cpp: In member function 'virtual const char* CClient::DemoPlayer_Play(const char*, int)':
src/engine/client/client.cpp:3858:123: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
3858 | m_aapSnapshots[g_Config.m_ClDummy][SNAP_CURRENT]->m_pSnap = (CSnapshot *)m_aaapDemorecSnapshotData[SNAP_CURRENT][0];
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
src/engine/client/client.cpp:3859:126: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
3859 | m_aapSnapshots[g_Config.m_ClDummy][SNAP_CURRENT]->m_pAltSnap = (CSnapshot *)m_aaapDemorecSnapshotData[SNAP_CURRENT][1];
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
src/engine/client/client.cpp:3864:117: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
3864 | m_aapSnapshots[g_Config.m_ClDummy][SNAP_PREV]->m_pSnap = (CSnapshot *)m_aaapDemorecSnapshotData[SNAP_PREV][0];
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
src/engine/client/client.cpp:3865:120: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
3865 | m_aapSnapshots[g_Config.m_ClDummy][SNAP_PREV]->m_pAltSnap = (CSnapshot *)m_aaapDemorecSnapshotData[SNAP_PREV][1];
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
```
Snapshot data during demo playback was being stored in an array of `char *` instead of an array of `char`, which caused above aliasing warnings and used 8 times more memory for the snapshot storage than being necessary.
6017: Minor refactoring of demo related code r=def- a=Robyt3
Extracted from #6016.
## Checklist
- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Robert Müller <robytemueller@gmail.com>
5599: Add support for Rust code in DDNet r=def- a=heinrich5991
The glue is done using the [cxx crate](https://cxx.rs/) on the Rust side.
As a proof-of-concept, only a small console command (`rust_version`) printing the currently used Rust version was added.
You can generate and open the Rust documentation using `DDNET_TEST_NO_LINK=1 cargo doc --open`.
You can run the Rust tests using `cmake --build <build dir> --target run_rust_tests`, they're automatically included in the `run_tests` target as well.
Rust tests don't work on Windows in debug mode on Windows because Rust cannot currently link with the debug version of the C stdlib on Windows: https://github.com/rust-lang/rust/issues/39016.
---
The stuff in `src/rust-bridge` is generated using
```
cxxbridge src/engine/shared/rust_version.rs --output src/rust-bridge/engine/shared/rust_version.cpp --output src/rust-bridge/engine/shared/rust_version.h
cxxbridge src/engine/console.rs --output src/rust-bridge/cpp/console.cpp --output src/rust-bridge/cpp/console.h
```
Co-authored-by: heinrich5991 <heinrich5991@gmail.com>
This function searches in all subfolder of the given folder and fills the given `std::vector` with all files with the given filename.
A `std::set` is used to prevent duplicate entries when a file with the same path is present in multiple storage locations. In that case we only need the path once, as we only use it against the highest priority storage location anyway.
The `Append` method was returning `0` on success while `Load` and `Save` were returning `1`.
Now all three methods use a `bool` as return value and return `true` on success.
The call `SortImages()` is moved inside the `Append` method, as it should always be called when appending succeeded.
Since the vulkan loader drops support for useful extensions (or the extension loading process changed between 1.0 & 1.1, not sure)
Which causes OBS not to work
5770: Fix physics change by weak hook fix (fixes#5769) r=def- a=fokkonaut
<!-- What is the motivation for the changes of this pull request -->
fixes#5769
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: fokkonaut <35420825+fokkonaut@users.noreply.github.com>
Co-authored-by: Jupeyy <jupjopjap@gmail.com>
Using `str_format(aBuf, sizeof(aBuf), "%s", pStr)` is equivalent to `str_copy(aBuf, pStr, sizeof(aBuf))`. Using `str_copy` is more readable and also more efficient as there is no overhead from parsing the format string and from passing varargs.
Slicing a demo opened from command line with an absolute path did not work, as the game only tried to load the source demo from storage instead of using the given absolute path.
Previously the IME was not deactivated when the editor is closed, so `SDL_TEXTINPUT` events where still being reported ingame after exiting the editor.
When entering the editor with a custom bind (e.g. `bind e "cl_editor 1"`) or when opening it from the console while also immediately closing the console (e.g. with `cl_editor 1; toggle_local_console`), the IME state was not properly set to active, so SDL did not report any `SDL_TEXTINPUT` events, leading to editboxes in the editor not receiving any text.
The is fixed by always enabling the IME state when entering the editor, which was previously only done when using the Ctrl+Shift+E hotkey or coincidentally when the editor is activated while the IME is already active, i.e. when the menu or console is open.
Closes#5095.
The glue is done using the [cxx crate](https://cxx.rs/) on the Rust
side.
As a proof-of-concept, only a small console command (`rust_version`)
printing the currently used Rust version was added.
You can generate and open the Rust documentation using
`DDNET_TEST_NO_LINK=1 cargo doc --open`.
You can run the Rust tests using `cmake --build <build dir> --target
run_rust_tests`, they're automatically included in the `run_tests`
target as well.
Rust tests don't work on Windows in debug mode on Windows because Rust
cannot currently link with the debug version of the C stdlib on Windows:
https://github.com/rust-lang/rust/issues/39016.
---
The stuff in `src/rust-bridge` is generated using
```
cxxbridge src/engine/shared/rust_version.rs --output src/rust-bridge/engine/shared/rust_version.cpp --output src/rust-bridge/engine/shared/rust_version.h
cxxbridge src/engine/console.rs --output src/rust-bridge/cpp/console.cpp --output src/rust-bridge/cpp/console.h
```
5933: Inline `Is(GameType)` functions and remove support for legacy 64 player info protocol r=def- a=heinrich5991
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
5941: Get away from vector for skins r=def- a=Jupeyy
most of the time it uses the index just to get the skin, downloaded skins change the index. Now its simply a heap object and downloaded skins load directly. Also the loading might be a bit faster bcs it had a loop lookup .Also O(1) lookup
not 100% tested. also fixes a bug with favorite skins hopefully
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: heinrich5991 <heinrich5991@gmail.com>
Co-authored-by: Jupeyy <jupjopjap@gmail.com>
It was only being used for "Leak IP" favorites and LAN servers. Since
KoG has upgraded to latest DDNet, there aren't much servers left that
don't understand the new 64 player info protocol.
Keywords: fstd, dtsf
They are used for two purposes, coloring gametypes in the serverbrowser
and enabling backward compatibility. These are independent, we shouldn't
add more stuff to the backward compatibility, hence I split them up and
inlined them.
5918: Constify snap more (inspired by upstream) r=Robyt3 a=ChillerDragon
Incorporate const added in those upstream commits:
d86d576217e6b8518b49
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [x] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: ChillerDragon <ChillerDragon@gmail.com>
Pass the original string to the unknown command callback instead of the parsed command, as the latter ends at the first whitespace, which breaks for unknown commands (filenames) containing spaces.
Closes#5902.
5867: Use ddnet.org on manpages r=heinrich5991 a=def-
<!-- What is the motivation for the changes of this pull request? -->
<!-- Note that builds and other checks will be run for your change. Don't feel intimidated by failures in some of the checks. If you can't resolve them yourself, experienced devs can also resolve them before merging your pull request. -->
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Dennis Felsing <dennis@felsin9.de>
This works around, i.e. fixes#5858 for curl versions < 7.77.0, as long
as the servers are only registered via IPv4 **OR** IPv6.
The bug that's being worked around is this:
84d2839740.
The bug makes curl reuse IPv6 connections when it is being requested to
connect via IPv4, making the registering fail. This commit works around
that by letting the server only register after having completely read
the config, so any `sv_register ipv4` should already be in effect.
5848: Remove cl_http_map_download r=heinrich5991 a=def-
Causes problems with GER3
## Checklist
- [x] Tested the change ingame
- [x] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
5851: Respect reserved slots in old serverinfo r=heinrich5991 a=def-
Noticed in https://github.com/ddnet/ddnet/pull/5850 that reserved slots were not respected
<!-- What is the motivation for the changes of this pull request? -->
<!-- Note that builds and other checks will be run for your change. Don't feel intimidated by failures in some of the checks. If you can't resolve them yourself, experienced devs can also resolve them before merging your pull request. -->
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: def <dennis@felsin9.de>
5829: Add HTTPS map download URL field for game servers r=def- a=heinrich5991
This allows every game server to provide its own HTTPS server for map
downloads. Since the ingame protocol for downloading map data is very
inefficient, this is desirable. Previously, only servers hosted by DDNet
could benefit from this.
Security concerns:
- Attackers can find out whether a given HTTPS GET request matches a
known answer.
This isn't deemed to be problematic as no cookies for authentication
are sent and only the whole response can be matched.
- Sending requests to honeypot URLs to get people in legal trouble.
This seems to be already possible with HTML image embeds, so it can't
be that bad™.
- Downloading huge files, filling up a player's disk. The players might
cancel when seeing huge files.
There's a generous limit of 1 GiB per map file.
- Downloading huge files transparently compressed with gzip. See above.
Fixes#5812.
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: heinrich5991 <heinrich5991@gmail.com>
This allows every game server to provide its own HTTPS server for map
downloads. Since the ingame protocol for downloading map data is very
inefficient, this is desirable. Previously, only servers hosted by DDNet
could benefit from this.
Security concerns:
- Attackers can find out whether a given HTTPS GET request matches a
known answer.
This isn't deemed to be problematic as no cookies for authentication
are sent and only the whole response can be matched.
- Sending requests to honeypot URLs to get people in legal trouble.
This seems to be already possible with HTML image embeds, so it can't
be that bad™.
- Downloading huge files, filling up a player's disk. The players might
cancel when seeing huge files.
There's a generous limit of 1 GiB per map file.
- Downloading huge files transparently compressed with gzip. See above.
Fixes#5812.
Add a non-saved config variable `http_allow_insecure` to re-enable HTTP
support; this is mostly useful for testing stuff.
Disallowing HTTP by default allows fewer insecure data to be sent and
received by the DDNet client.
Merging of DoLaserPreview and static int fix
Replace if else statement with switch function
Adjusted RenderDropDown header to match
static int cbuttoncontainer fix, potential bugs
5822: Move skin info after the settings (settings can cause reload -> skin … r=heinrich5991 a=Jupeyy
…is invalid)
Also added an assert now, we dont want to render invalid textures ever since its always a logic bug
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Jupeyy <jupjopjap@gmail.com>
Previously, close messages were entirely ignored during the connection
process, this meant that ban messages weren't shown to players. Instead,
they'd see the standard "no answer from server yet" message.
Fixes#5792.
5763: Fix png error handling by setting jmpbuf r=def- a=Jupeyy
else libpng will kill the thread on an error bcs it's a fatal error.
broken skin can be found in discord bugs channel weeb_okayu
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Jupeyy <jupjopjap@gmail.com>
Add `IConsole::SetUnknownCommandCallback` to set a callback for unknown commands. The callback is used to handle connect links, .demo and .map files when parsing command line arguments.
This will allow paths/links to be passed at any argument position instead of only the first one.
And this fixes the command `play xyz.demo` not working due to `play ` being considered part of the path.
When passing .demo or .map paths as command line arguments, first check if the path can be found in the storage and then try resolving an absolute path.
Also delay execution of the `play` command until the client is ready so the command can be used from the command line. Although it doesn't work if there is only one argument, as this interferes with the .demo file handling.
By updating and rendering the current tick again when changing the spectator mode while the demo playback is paused.
Refactoring: Extract `IDemoPlayer::ETickOffset`, `IDemoPlayer::SeekTick` and `CMenus::DemoSeekTick`.
- Approximate index of wanted keyframe to decrease linear search time.
- Only apply the magic `-5` to the Keyframe calculation, to make seeking with the mouse more accurate.
- After the method returns, the specified tick will be the _next_ tick being played instead of being the current tick.
Previously the playback did not start and no error messages was shown, when the map file cannot be found and no map data is embedded in the demo file.
Minor refactoring: Replace instances of `return pError` with semantically identical `return nullptr` to improve readability.
/home/deen/isos/ddnet/ddnet-source/src/tools/map_create_pixelart.cpp: In function ‘int InsertPixelArtQuads(CQuad*, int&, const CImageInfo&, const int*, const int*, const bool*)’:
/home/deen/isos/ddnet/ddnet-source/src/tools/map_create_pixelart.cpp:110:9: error: ‘memset’ was not declared in this scope
110 | memset(aVisitedPixels, 0, sizeof(bool) * Img.m_Height * Img.m_Width);
| ^~~~~~
/home/deen/isos/ddnet/ddnet-source/src/tools/map_create_pixelart.cpp:8:1: note: ‘memset’ is defined in header ‘<cstring>’; did you forget to ‘#include <cstring>’?
7 | #include <game/mapitems.h>
+++ |+#include <cstring>
8 |
- use `CORNER_*` constants and add missing constants
- fix array variable names
- use `size_t`
- extract duplicate computation into constant
- use `ColorRGBA` instead of `vec4`
Make the member variables private and add `SetMin` to replace a usage of the member variables in `CDebugHud`. For completeness/symmetry, `SetMax` is also added.
The return value of `CGraph::InsertAt` was not checked. All uses of the function pass a correct index, so the return value is replaced with an assertion.
As the methods are always called at the same time, they can be combined. This also improves the performance, as the array only needs to be iterated once.
`PossibleCommands` now passes the item index to the callback and returns the total number of items.
Add `EmptyPossibleCommandCallback` as default parameter.
5642: Option to remove weak hook & bounce r=def- a=Jupeyy
Fixes#5641
server side only
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Jupeyy <jupjopjap@gmail.com>
5715: Use djb2 for snapshot item hashlist r=def- a=Robyt3
The previous hash function was heavily biased towards the hash buckets 64-79, making those buckets overflow faster, which results in snapshot CRC errors and lags.
Using the djb2 hash yields an almost even distribution over the entire range of values.
Source for djb2: http://www.cse.yorku.ca/~oz/hash.html
(we incidentally use the same implementation for `str_quickhash`)
Sample which compares the usage of hash buckets in the test map from #5454 with the old and new hash function: [SnapshotHash.csv](https://github.com/ddnet/ddnet/files/9285148/SnapshotHash.csv) (this table also differentiates between the source and target of the snapshot delta)
Closes#4379.
## Checklist
- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Robert Müller <robytemueller@gmail.com>
The previous hash function was heavily biased towards the hash buckets 64-79, making those buckets overflow faster, which results in snapshot CRC errors and lags.
Using the djb2 hash yields an almost even distribution over the entire range of values.
http://www.cse.yorku.ca/~oz/hash.html
How this works: parallax values configure perceived distance from camera
when it's moving along x and y axes. Assume that zoom is moving the
camera away and scale layers accordingly, with background layers
(furtherst away) changing the least.
New per-ItemGroup (LayerGroup) setting allows to set the new parallax
value independently from the other two. This can be used to do tricks
like on Time Shop zoom correctly or make it feel like the camera is
changing the field of view at the same time as moving in space.
The `CServer::GetClientVersion` method needs the `version.h` include, so it's moved from the header to the source file, so the include can be removed from the header.
The `GetClientVersion` method is often called with the same `Client != SERVER_DEMO_CLIENT ? GetClientVersion(Client) : CLIENT_VERSIONNR` expression, which also needs the `version.h` include. This expression is moved inside the method, so the include can be removed from all the server entities' and player code.
The `CGameContext::GetClientVersion` method is made a delegate to reduce duplicate code.
The includes of the server entities are also organized further.
/media/ddnet/src/engine/server.h:135:3: error: Address of stack memory associated with local variable 'aBuf' is still referred to by the stack variable 'tmp' upon returning to the caller. This will be a dangling reference [clang-analyzer-core.StackAddressEscape,-warnings-as-errors]
return SendPackMsgOne(pMsg, Flags, ClientID);
^
/media/ddnet/src/game/server/gamecontext.cpp:4084:5: note: Assuming the condition is false
if(pFilter[0])
^~~~~~~~~~
/media/ddnet/src/game/server/gamecontext.cpp:4084:2: note: Taking false branch
if(pFilter[0])
^
/media/ddnet/src/game/server/gamecontext.cpp:4088:2: note: Calling 'CGameContext::SendChatTarget'
SendChatTarget(ClientID, aBuf);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/media/ddnet/src/game/server/gamecontext.cpp:401:5: note: Assuming field 'm_SvDemoChat' is not equal to 0
if(g_Config.m_SvDemoChat)
^~~~~~~~~~~~~~~~~~~~~
/media/ddnet/src/game/server/gamecontext.cpp:401:2: note: Taking true branch
if(g_Config.m_SvDemoChat)
^
/media/ddnet/src/game/server/gamecontext.cpp:402:3: note: Calling 'IServer::SendPackMsg'
Server()->SendPackMsg(&Msg, MSGFLAG_VITAL | MSGFLAG_NOSEND, -1);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/media/ddnet/src/engine/server.h:71:3: note: Taking true branch
if(ClientID == -1)
^
/media/ddnet/src/engine/server.h:73:19: note: Assuming the condition is true
for(int i = 0; i < MaxClients(); i++)
^~~~~~~~~~~~~~~~
/media/ddnet/src/engine/server.h:73:4: note: Loop condition is true. Entering loop body
for(int i = 0; i < MaxClients(); i++)
^
/media/ddnet/src/engine/server.h:74:8: note: Assuming the condition is true
if(ClientIngame(i))
^~~~~~~~~~~~~~~
/media/ddnet/src/engine/server.h:74:5: note: Taking true branch
if(ClientIngame(i))
^
/media/ddnet/src/engine/server.h:77:15: note: Calling 'IServer::SendPackMsgTranslate'
Result = SendPackMsgTranslate(&tmp, Flags, i);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/media/ddnet/src/engine/server.h:118:6: note: Assuming field 'm_ClientID' is >= 0
if(pMsg->m_ClientID >= 0 && !Translate(pMsg->m_ClientID, ClientID))
^~~~~~~~~~~~~~~~~~~~~
/media/ddnet/src/engine/server.h:118:6: note: Left side of '&&' is true
/media/ddnet/src/engine/server.h:118:31: note: Assuming the condition is true
if(pMsg->m_ClientID >= 0 && !Translate(pMsg->m_ClientID, ClientID))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/media/ddnet/src/engine/server.h:118:3: note: Taking true branch
if(pMsg->m_ClientID >= 0 && !Translate(pMsg->m_ClientID, ClientID))
^
/media/ddnet/src/engine/server.h:125:6: note: Assuming the condition is false
if(IsSixup(ClientID))
^~~~~~~~~~~~~~~~~
/media/ddnet/src/engine/server.h:125:3: note: Taking false branch
if(IsSixup(ClientID))
^
/media/ddnet/src/engine/server.h:135:3: note: Address of stack memory associated with local variable 'aBuf' is still referred to by the stack variable 'tmp' upon returning to the caller. This will be a dangling reference
return SendPackMsgOne(pMsg, Flags, ClientID);
^
5658: Fix game freezing up on duplicate snapshot r=def- a=Fireball-Teeworlds
If a duplicate snapshot is received (for the same tick), we add both to the SnapshotStorage and end up with the same snapshot as both Cur and Prev. This results in GameInfraTick returning "inf" and results in "NaN" downstream in a few places, getting the CollLine logic stuck.
Some debug info (tcpdump, gdb, perf): https://gist.github.com/Fireball-Teeworlds/ad0016d2551a2e4d4cb5691023493856
Apparently this doesn't really happen in the wild, unless you have a buggy network stack. In which case it happens frequently enough to pinpoint the issue :D
(fixes#5657)
## Checklist
- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [X] Considered possible null pointers and out of bounds array indexing
- [X] Changed no physics that affect existing maps
- [X] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Fireball <fireball.teeworlds@gmail.com>
If a duplicate snapshot is received (for the same tick), we add both to the SnapshotStorage and end up with the same snapshot as both Cur and Prev. This results in GameInfraTick returning "inf" and results in "NaN" downstream in a few places, getting the CollLine logic stuck.
Apparently this doesn't really happen in the wild, unless you have a buggy network stack. In which case it happens frequently enough to pinpoint the issue :D
Add `CSnapshot::IsValid` to check if a snapshot unpacked from a snapshot delta or demo is valid:
- ensure number of items and data size are not negative
- ensure that the actual size of the snapshot matches the size derived from its member variables
- ensure item offsets are within the valid range
- ensure item sizes are not negative
Add `CSnapshot::TotalSize` and `CSnapshot::OffsetSize` utility functions.
Minor improvements to related error messages.
Fixes buffer overflow:
```
==47744==ERROR: AddressSanitizer: global-buffer-overflow on address 0x558618e3767f at pc 0x558614b9bdfb bp 0x7ffe58a32cd0 sp 0x7ffe58a32cc0
READ of size 4 at 0x558618e3767f thread T0
0x558614b9bdfa in CSnapshotItem::Type() const src/engine/shared/snapshot.h:16
0x558615c3c911 in CSnapshot::GetItemType(int) const src/engine/shared/snapshot.cpp:29
0x558614aebaba in CClient::UnpackAndValidateSnapshot(CSnapshot*, CSnapshot*) src/engine/client/client.cpp:2264
0x558614af87cb in CClient::OnDemoPlayerSnapshot(void*, int) src/engine/client/client.cpp:2598
0x558615b9db1a in CDemoPlayer::DoTick() src/engine/shared/demo.cpp:659
0x558615babd3f in CDemoPlayer::Update(bool) src/engine/shared/demo.cpp:1007
0x558614afb08b in CClient::Update() src/engine/client/client.cpp:2686
0x558614b1d9eb in CClient::Run() src/engine/client/client.cpp:3296
0x558614b8e64f in main src/engine/client/client.cpp:4761
```
And fixes a buffer overflow that manifests itself as an internal ASan error:
```
=================================================================
==4755==AddressSanitizer CHECK failed: ../../../../src/libsanitizer/asan/asan_descriptions.cc:79 "((0 && "Address is not in memory and not in shadow?")) != (0)" (0x0, 0x0)
0x7f0bf5f368be in AsanCheckFailed ../../../../src/libsanitizer/asan/asan_rtl.cc:72
0x7f0bf5f54eee in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) ../../../../src/libsanitizer/sanitizer_common/sanitizer_termination.cc:77
0x7f0bf5e4cb6f in GetShadowKind ../../../../src/libsanitizer/asan/asan_descriptions.cc:79
0x7f0bf5e4cb6f in __asan::GetShadowAddressInformation(unsigned long, __asan::ShadowAddressDescription*) ../../../../src/libsanitizer/asan/asan_descriptions.cc:95
0x7f0bf5e4cb6f in __asan::GetShadowAddressInformation(unsigned long, __asan::ShadowAddressDescription*) ../../../../src/libsanitizer/asan/asan_descriptions.cc:92
0x7f0bf5e4e386 in __asan::AddressDescription::AddressDescription(unsigned long, unsigned long, bool) ../../../../src/libsanitizer/asan/asan_descriptions.cc:440
0x7f0bf5e50e94 in __asan::ErrorGeneric::ErrorGeneric(unsigned int, unsigned long, unsigned long, unsigned long, unsigned long, bool, unsigned long) ../../../../src/libsanitizer/asan/asan_errors.cc:380
0x7f0bf5f35f4d in __asan::ReportGenericError(unsigned long, unsigned long, unsigned long, unsigned long, bool, unsigned long, unsigned int, bool) ../../../../src/libsanitizer/asan/asan_report.cc:460
0x7f0bf5e86f5e in __interceptor_memset ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:762
0x558234873f1d in mem_zero src/base/system.cpp:213
0x55823481fc27 in CSnapshotBuilder::NewItem(int, int, int) src/engine/shared/snapshot.cpp:675
0x55823481be65 in CSnapshotDelta::UnpackDelta(CSnapshot*, CSnapshot*, void const*, int) src/engine/shared/snapshot.cpp:380
0x558234776641 in CDemoPlayer::DoTick() src/engine/shared/demo.cpp:631
0x5582347861a9 in CDemoPlayer::Update(bool) src/engine/shared/demo.cpp:1007
0x5582336d4c7d in CClient::Update() src/engine/client/client.cpp:2695
0x5582336f75dd in CClient::Run() src/engine/client/client.cpp:3305
0x558233768241 in main src/engine/client/client.cpp:4770
```
5634: Fix clang tidy accessing data pointer r=def- a=ChillerDragon
ddnet/src/engine/client/favorites.cpp:229:23: error: 'data' should be
used for accessing the data pointer instead of taking the address of the
0-th element [readability-container-data-pointer,-warnings-as-errors]
int Index = pEntry - &m_aEntries[0];
^~~~~~~~~~~~~~
(m_aEntries).data()
<!-- What is the motivation for the changes of this pull request -->
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: ChillerDragon <ChillerDragon@gmail.com>
ddnet/src/engine/client/favorites.cpp:229:23: error: 'data' should be
used for accessing the data pointer instead of taking the address of the
0-th element [readability-container-data-pointer,-warnings-as-errors]
int Index = pEntry - &m_aEntries[0];
^~~~~~~~~~~~~~
(m_aEntries).data()
5520: Warn about pnglite-incompatible PNGs on load r=def- a=heinrich5991
This allows a larger range of PNGs to be loaded while still maintaining
backward compatibility with older clients by annoying the user.
This warning can be enabled by the `warn-pnglite-incompatible-images`
key in the https://info2.ddnet.tw/info JSON, if the key is not there or
the JSON hasn't been obtained yet, the warning is disabled. Since the
JSON is cached across restarts, it'll be effective for initially loaded
images from the second start.
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
5624: Move message copy inside `SendPackMsgTranslate` r=def- a=Robyt3
Fix clang warning: Address of stack memory associated with local variable 'aBuf' is still referred to by the stack variable 'tmp' upon returning to the caller. This will be a dangling reference [clang-analyzer-core.StackAddressEscape]
Mark the input parameters as const pointers and mark the protocol message `Pack` methods as const.
And remove a null check that only serves to hide programmer errors.
## Checklist
- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: heinrich5991 <heinrich5991@gmail.com>
Co-authored-by: Robert Müller <robytemueller@gmail.com>
This allows a larger range of PNGs to be loaded while still maintaining
backward compatibility with older clients by annoying the user.
This warning can be enabled by the `warn-pnglite-incompatible-images`
key in the https://info2.ddnet.tw/info JSON, if the key is not there or
the JSON hasn't been obtained yet, the warning is disabled. Since the
JSON is cached across restarts, it'll be effective for initially loaded
images from the second start.
5511: Rename cl_show_quads -> cl_showquads r=heinrich5991 a=def-
To reenable quads for everyone who accidentally disabled it or has the
bind to accidentally disable them
<!-- What is the motivation for the changes of this pull request -->
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: def <dennis@felsin9.de>
Fix clang warning: Address of stack memory associated with local variable 'aBuf' is still referred to by the stack variable 'tmp' upon returning to the caller. This will be a dangling reference [clang-analyzer-core.StackAddressEscape]
Mark the input parameters as const pointers and mark the protocol message `Pack` methods as const.
5205: Allow multiple addresses per server in the serverbrowser r=def- a=heinrich5991
Support is incomplete for `leak_ip_address_to_all_servers` (will only
ping the first address of each server) and for the `leak_ip` setting
(which will also only ping the first address of each server).
Fixes#5158.
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: heinrich5991 <heinrich5991@gmail.com>
5613: Search in $PATH for server executable (fixes#5594) r=heinrich5991 a=def-
<!-- What is the motivation for the changes of this pull request -->
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: def <dennis@felsin9.de>
This allows the client to connect to servers that have both an IPv4 and
an IPv6 address, even if the client is only connected to one of the two.
The one faster to answer will be picked.
Timeout codes are now generated based on *all* of the server's
addresses.
Fixes#5158.
Support is incomplete for `leak_ip_address_to_all_servers` (will only
ping the first address of each server) and for the `leak_ip` setting
(which will also only ping the first address of each server).
5591: (A bit) safer interface for text containers r=def- a=Jupeyy
For #5143
Maybe it helps identifying the problem earlier, maybe not^^
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Jupeyy <jupjopjap@gmail.com>
Fix pointer and pointer array variable naming
Huge renaming to match our rules
Used regex: (?!(return|delete)\b)\b\w+ (m_|ms_|g_|gs_|s_)[^a]\w+\[
(?!(return|delete)\b)\b\w+ (?!(m_|ms_|g_|gs_|s_))[^a]\w+\[
Further format static variables
Format almost all pointer names accordingly
Used regex: (?!(return)\b)\b\w+
\*(?!(m_p|p|s_p|m_ap|s_ap|g_p|g_ap|ap|gs_ap|ms_ap|gs_p|ms_p))\w+\b[^:\(p]
clang-format
Fix CI fail
Fix misnamed non pointer as pointer and non array as array
Used regex: (?!(return|delete)\b)\b\w+ (m_|ms_|g_|gs_|s_)p\w+\b
(?!return\b)\b\w+ (ms_|m_|g_|gs_|s_)a\w+\b[^\[]
clang-format
Revert to SCREAMING_SNAKE_CASE and reinstate dead code
5544: Replaced float array with appropriate struct (vec or color) r=def- a=Chairn
I mostly touched the code in the graphic part for array of 2 floats into vec2.
For color, i replaced array of 4 floats with ColorRGBA where it made sense.
I had to change the logic order in `src/engine/client/graphics_threaded.cpp:1113` due to operator= from vec2 to vec3. I didn't see any visual change.
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [x] Considered possible null pointers and out of bounds array indexing
- [x] Changed no physics that affect existing maps
- [x] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Chairn <chairn.nq@hotmail.fr>
5345: Add Integration test to pr template r=heinrich5991 a=def-
5519: Library updates (2022-06-27) r=heinrich5991 a=def-
Should probably wait until after 16.2 release with them.
Vulkan 1.3.204.1 -> 1.3.216.0
curl 7.81.0 -> 7.84.0
FFmpeg 5.0 -> 5.0.1
freetype 2.11.1 -> 2.12.1
SQLite3 3.37.2 -> 3.39.0
<!-- What is the motivation for the changes of this pull request -->
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
5572: Also make gameclient aware that snapshots have been purged (fixes#5571) r=heinrich5991 a=def-
Recreates SnapEntities internally
<!-- What is the motivation for the changes of this pull request -->
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: def <dennis@felsin9.de>
Co-authored-by: Dennis Felsing <dennis@felsin9.de>
5547: Minor tweaks to keep ODR across translation units r=def- a=Jupeyy
<!-- What is the motivation for the changes of this pull request -->
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Jupeyy <jupjopjap@gmail.com>
The sixup translated player flags were previously applied in `OnDirectInput` directly to the player input data.
However, the input data has already been copied at that point, so predicted inputs were using the incorrect flags.
This is fixed by moving the player flags translation to `OnClientPrepareInput`, which is called first and only once on each new player input before it is copied.
The flags were only being updated based on rather arbitrary conditions, so they could never reset to zero or reduced.
The flags are now always updated. The flags are only used in the output of the `status` command.
5486: Switch to loading screen, when map creation takes too long r=def- a=Jupeyy
Same as #4941
It doesn't directly fix the issue described in #5478, it does however not trigger it anymore. it's still a bug in our code unrelated to this. This just triggered the UB from the issue as `@ardadem` confirmed, SDL received a SDL_QUIT (`[2022-06-23 20:26:31][test]: sdl closed my client, but why xd`)
Now we have a confirmed case that making the window unresponsive can create weird behavior. Similar to the other pr switch to a loading screen after 500ms (this time without menu background map tho, since the menu background map calls the same code).
We really need some fancy loading screen for such situations, even tho they are rare xD
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Jupeyy <jupjopjap@gmail.com>
5514: Update menu music state when the config variables change via console, various refactoring r=heinrich5991 a=Robyt3
Also update the background music when `snd_enable` or `snd_enable_music` change via console or bind. Closes#2911.
For this purpose, add `IsPlaying` method to engine sound and client sound component to check whether a specific sound sample is already playing.
Various refactoring in engine sound.
## Checklist
- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Robert Müller <robytemueller@gmail.com>
5210: Move ddnet character display info in ddnet character r=def- a=C0D3D3V
- I also added the default value mechanic for Net Objects that have set `validate_size=False`
- I removed the ramp value from the snap, because currently all maps use the default tuning parameters for it
- I renamed `m_FreezeTick` to `m_FreezeStart`
- Added Extended Net Objects to the debug HUD (Also added a Headline):
(out of range) is in the screenshot the old DDNetCharacterDisplayInfo
fixes#5455
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [x] Considered possible null pointers and out of bounds array indexing
- [x] Changed no physics that affect existing maps
- [x] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: c0d3d3v <c0d3d3v@mag-keinen-spam.de>
5420: Improvments for new HUD r=def- a=C0D3D3V
for testing set `#define CLIENT_VERSIONNR 16020`
Some of the changes are only available in the next version
Impressions: https://youtu.be/E770vGp4KKY
Implements a few of the recommendations from https://github.com/ddnet/ddnet/issues/5159 and Discord
- removes all player capability messages send by the server, that are now displayed in the HUD
- ~~remove `cl_ddrace_hud` and cl_show_freeze_bars~~,
- Added option `cl_freezebars_alpha_inside_freeze` setting to change the opacity if the player is inside a freeze tile (default visible 100%)
- Added option `cl_show_jumps_indicator`
- We send now the real weapon the player is holding if he is frozen
-> for that we added a new particle "snow flakes" or frozen flakes :D that are spawned over the tees body, to indicate he is frozen.
-> new extras.png asset (+ corresponding asset tab, that can potentially be used for all sorts of extras for our mod, e.g. material particles, but in the future maybe also for other mods using a similar structure like it is for entities)
- stars are no longer send to new clients, because it is duplication to the freezebar, and we decided to not allow to disable it
- also ninja stars are no longer send. They are not vanilla, and we now have a ninja bar in the new HUD.
- display target angle from 0 to 360 (looks mirrored on the x-axis to the normal trigonometry circle, but that is just how the world coordinates are)
- dummy action display is increased in size ( but only the border margin :D not the icons. so the icons are still the same size as the icons on the left side)
- allow the server to also show old HUD elements (see https://github.com/ddnet/ddnet/pull/5427 for screenshots)
fixes#5149fixes#198closes#5159fixes#2252 ~~(But not clear if we should also display the weapon, still investigating)~~
- we do no longer display a weapon inside freeze, instead we have nice freeze particles
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [x] Considered possible null pointers and out of bounds array indexing
- [x] Changed no physics that affect existing maps
- [x] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: c0d3d3v <c0d3d3v@mag-keinen-spam.de>
5504: Remove useless typedef in c++ r=Jupeyy a=Chairn
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Chairn <chairn.nq@hotmail.fr>
5499: Use [[fallthrough]] attribute rather than comment r=def- a=Chairn
We could even add implicit fallthrough warning, but it is triggered in external json.c
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Chairn <chairn.nq@hotmail.fr>
5502: Add missing capability for GLES backend r=def- a=Jupeyy
<!-- What is the motivation for the changes of this pull request -->
## Checklist
- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: Jupeyy <jupjopjap@gmail.com>
```
Direct leak of 216320 byte(s) in 3 object(s) allocated from:
#0 0x7f5e05924808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
#1 0x559b8ca66278 in CCommandProcessorFragment_OpenGL2::Cmd_CreateBufferObject(CCommandBuffer::SCommand_CreateBufferObject const*) src/engine/client/backend/opengl/backend_opengl.cpp:1901
#2 0x559b8ca47fe7 in CCommandProcessorFragment_OpenGL::RunCommand(CCommandBuffer::SCommand const*) src/engine/client/backend/opengl/backend_opengl.cpp:1107
#3 0x559b8cdf811b in CCommandProcessor_SDL_GL::RunBuffer(CCommandBuffer*) src/engine/client/backend_sdl.cpp:248
#4 0x559b8cdf4262 in CGraphicsBackend_Threaded::ThreadFunc(void*) src/engine/client/backend_sdl.cpp:77
#5 0x559b8dfc58a5 in thread_run src/base/system.cpp:721
#6 0x7f5e03bf6608 in start_thread /build/glibc-SzIz7B/glibc-2.31/nptl/pthread_create.c:477
```
4829: Added map_replace_area tool r=def- a=sctt
Motivation:
sometimes mappers need to copy a certain area from a map to another (but also to a different position of the same map).
that's very time consuming on complex maps, as tiles and quads layers have to be manually copied one by one.
map_replace_area is designed to automatically replace all the tiles and quads from a specific area to another.
`Usage: map_replace_area <from_map> <from_x> <from_y> <to_map> <to_x> <to_y> <width> <height> <output_map>`
PoW screenshots attached below.
note: for the moment map_replace_area suppose that you are working on the same map (that's what i needed), that means source and destination maps must have the same layers structure, or else an error is returned.
but i understand it might be useful to also work on totally different maps, in that case users must be able specify layers they want to consider by associating source map layers to destination map layers (by using a config file maybe).
i'm probably going to make another PR to add this enhancement in the future.
PoW:
5343: Add rcon and chat to integration test r=def- a=ChillerDragon
Add more action to the integration test script: chat messages, chat commands, rcon commands.
This unlocks more code coverage at runtime to catch asan issues.
It adds a very restrictive regex on the log format. But I am happy to maintain that in case changes to the log output happen.
The current version is already supporting heinrichs refactor https://github.com/ddnet/ddnet/pull/5036
Also ensure chat messages arrive in the correct format. Would have catched the following issues:
https://github.com/ddnet/ddnet/issues/5342https://github.com/ddnet/ddnet/issues/5340https://github.com/ddnet/ddnet/issues/5302https://github.com/ddnet/ddnet/pull/5126
DEPENDS ON:
https://github.com/ddnet/ddnet/issues/5342https://github.com/ddnet/ddnet/issues/5340
5465: Change from pnglite to libpng for PNG reading r=def- a=heinrich5991
This is desirable mainly because libpng is maintained and pnglite is
not. pnglite was last updated in 2007 (15 years ago) and probably has a
lot of security vulnerabilities.
libpng is an actively maintained library also used by browsers like
Firefox or Chromium, so it's less likely to contain security
vulnerabilities, also it's more likely to be packaged by Linux
distributions.
This also refuses to load images of types not supported by pnglite,
which allows us to think about backward compatibility while also
introducing libpng.
## Checklist
- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)
Co-authored-by: sctt <scottistefano91@gmail.com>
Co-authored-by: term <term@term.sinervis.pri>
Co-authored-by: f <scottistefano91@gmail.com>
Co-authored-by: heinrich5991 <heinrich5991@gmail.com>
Co-authored-by: ChillerDragon <ChillerDragon@gmail.com>
Co-authored-by: def <dennis@felsin9.de>
Co-authored-by: Jupeyy <jupjopjap@gmail.com>
This is desirable mainly because libpng is maintained and pnglite is
not. pnglite was last updated in 2007 (15 years ago) and probably has a
lot of security vulnerabilities.
libpng is an actively maintained library also used by browsers like
Firefox or Chromium, so it's less likely to contain security
vulnerabilities, also it's more likely to be packaged by Linux
distributions.
Instead of closing the joysticks manually, use `SDL_QuitSubSystem(SDL_INIT_JOYSTICK)` to quit the entire subsystem, which will also close all joysticks correctly.
The engine input destructor is replaced with a `Shutdown` method so we can control when it is called, i.e. before calling `SDL_Quit`, which forcefully quits all subsystems.
`CJoystick::Close` is removed as we don't need to close joysticks manually anymore.
Edgar
ChillerDragon
