FEAT(positional-audio): Plugin for Teardown#7142
FEAT(positional-audio): Plugin for Teardown#7142sqwishy wants to merge 1 commit intomumble-voip:masterfrom
Conversation
Positional audio support for the 2026 multiplayer release of Teardown. I think it's game version 2.0. (That's what it says in the bottom right corner of the screen in the main menu.)
WalkthroughA new Mumble plugin for the game Teardown is being introduced across three files. The 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
plugins/teardown/teardown.cpp (1)
286-292: Use a session-specific context instead of a constant string.Line 291 sets one global context for all in-game users, which can cause positional sharing between different Teardown multiplayer sessions on the same Mumble server.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/teardown/teardown.cpp` around lines 286 - 292, Currently *contextPtr is assigned a constant literal "tearing it down with your buddies", which makes the context identical across sessions; instead build a session-unique string (e.g., include the current session id, server port, or a GUID) and assign that to *contextPtr so each multiplayer session gets its own context. Locate the place using contextPtr in teardown.cpp and replace the literal with a dynamically-created string formatted with the session identifier (using snprintf/std::string + strdup or equivalent) and ensure the allocated memory lifetime matches the API expectations (free or clear it when the session ends or when context is replaced) so no dangling pointer or leak occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/teardown/CMakeLists.txt`:
- Around line 6-17: The CMake targets currently add "../ProcessWindows.cpp"
unconditionally which breaks Linux builds and omits "../ProcessLinux.cpp";
update the teardown target_sources for the library named teardown so that
"../ProcessWindows.cpp" and "../HostWindows.cpp" are only added inside the
if(WIN32) branch and "../ProcessLinux.cpp" and "../HostLinux.cpp" are added in
the else() branch, i.e. move the ProcessWindows.cpp entry into the WIN32 block
and add ProcessLinux.cpp to the else() block while keeping
HostWindows.cpp/HostLinux.cpp selection as-is to guard platform-specific backend
sources.
In `@plugins/teardown/teardown.cpp`:
- Around line 42-61: The TeardownPositional struct is read raw from another
process so add compile-time layout guards: use static_assert with
offsetof(TeardownPositional, playerPos) == 0x58, offsetof(..., cameraPos) ==
0x1cc, and static_assert(sizeof(TeardownPositional) == 0x???) (replace ??? with
the actual expected total size) to catch layout drift; also assert sizeof(Vec3)
and sizeof(Quat) if needed and ensure the struct's packing/alignment matches the
remote process (e.g., pragma pack or attribute if the project uses it) so
offsets are reliable.
---
Nitpick comments:
In `@plugins/teardown/teardown.cpp`:
- Around line 286-292: Currently *contextPtr is assigned a constant literal
"tearing it down with your buddies", which makes the context identical across
sessions; instead build a session-unique string (e.g., include the current
session id, server port, or a GUID) and assign that to *contextPtr so each
multiplayer session gets its own context. Locate the place using contextPtr in
teardown.cpp and replace the literal with a dynamically-created string formatted
with the session identifier (using snprintf/std::string + strdup or equivalent)
and ensure the allocated memory lifetime matches the API expectations (free or
clear it when the session ends or when context is replaced) so no dangling
pointer or leak occurs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c4ef2ad8-860f-476e-8d7b-4a22bcc9a0f5
📒 Files selected for processing (3)
plugins/CMakeLists.txtplugins/teardown/CMakeLists.txtplugins/teardown/teardown.cpp
| add_library(teardown SHARED | ||
| "teardown.cpp" | ||
|
|
||
| "../Module.cpp" | ||
| "../ProcessBase.cpp" | ||
| "../ProcessWindows.cpp") | ||
|
|
||
| if(WIN32) | ||
| target_sources(teardown PRIVATE "../HostWindows.cpp") | ||
| else() | ||
| target_sources(teardown PRIVATE "../HostLinux.cpp") | ||
| endif() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify process backend files and current plugin backend patterns.
fd -t f 'Process.*\.(cpp|h)' plugins
rg -n -C2 'add_library\(|ProcessWindows\.cpp|ProcessLinux\.cpp|HostLinux\.cpp|HostWindows\.cpp' plugins/**/CMakeLists.txt
rg -n -C2 'class ProcessWindows|#include <windows\.h>|_WIN32' plugins/ProcessWindows.cpp plugins/ProcessWindows.hRepository: mumble-voip/mumble
Length of output: 16908
🏁 Script executed:
head -30 plugins/ProcessWindows.cpp plugins/ProcessWindows.hRepository: mumble-voip/mumble
Length of output: 1740
🏁 Script executed:
head -30 plugins/ProcessLinux.cppRepository: mumble-voip/mumble
Length of output: 818
🏁 Script executed:
grep -A 5 "add_library(se SHARED" plugins/se/CMakeLists.txtRepository: mumble-voip/mumble
Length of output: 158
🏁 Script executed:
grep -A 5 "else()" plugins/se/CMakeLists.txt | head -3Repository: mumble-voip/mumble
Length of output: 145
Guard process backend sources by platform.
Line 11 unconditionally includes ../ProcessWindows.cpp, which contains Windows-specific code (including mumble_positional_audio_win32_internals.h and Windows PE parsing logic) that cannot compile on Linux. Meanwhile, ../ProcessLinux.cpp—which uses ELF-specific structures—is never included, breaking Linux builds.
Move ProcessWindows.cpp into the if(WIN32) block and add ProcessLinux.cpp to the else() block:
Suggested CMake adjustment
add_library(teardown SHARED
"teardown.cpp"
"../Module.cpp"
- "../ProcessBase.cpp"
- "../ProcessWindows.cpp")
+ "../ProcessBase.cpp")
if(WIN32)
+ target_sources(teardown PRIVATE "../ProcessWindows.cpp")
target_sources(teardown PRIVATE "../HostWindows.cpp")
else()
+ target_sources(teardown PRIVATE "../ProcessLinux.cpp")
target_sources(teardown PRIVATE "../HostLinux.cpp")
endif()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| add_library(teardown SHARED | |
| "teardown.cpp" | |
| "../Module.cpp" | |
| "../ProcessBase.cpp" | |
| "../ProcessWindows.cpp") | |
| if(WIN32) | |
| target_sources(teardown PRIVATE "../HostWindows.cpp") | |
| else() | |
| target_sources(teardown PRIVATE "../HostLinux.cpp") | |
| endif() | |
| add_library(teardown SHARED | |
| "teardown.cpp" | |
| "../Module.cpp" | |
| "../ProcessBase.cpp") | |
| if(WIN32) | |
| target_sources(teardown PRIVATE "../ProcessWindows.cpp") | |
| target_sources(teardown PRIVATE "../HostWindows.cpp") | |
| else() | |
| target_sources(teardown PRIVATE "../ProcessLinux.cpp") | |
| target_sources(teardown PRIVATE "../HostLinux.cpp") | |
| endif() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/teardown/CMakeLists.txt` around lines 6 - 17, The CMake targets
currently add "../ProcessWindows.cpp" unconditionally which breaks Linux builds
and omits "../ProcessLinux.cpp"; update the teardown target_sources for the
library named teardown so that "../ProcessWindows.cpp" and "../HostWindows.cpp"
are only added inside the if(WIN32) branch and "../ProcessLinux.cpp" and
"../HostLinux.cpp" are added in the else() branch, i.e. move the
ProcessWindows.cpp entry into the WIN32 block and add ProcessLinux.cpp to the
else() block while keeping HostWindows.cpp/HostLinux.cpp selection as-is to
guard platform-specific backend sources.
| struct TeardownPositional { | ||
| /* This information isn't perfect, the quaternion flops around when the | ||
| * player is seated in a vehicle. But the position is pretty good! When | ||
| * in a lift vehicle, for example, your body is in the upper part that | ||
| * moves vertically, and playerPos will report that position | ||
| * consistently in both first and third person perspectives. That is | ||
| * what we want because other players will always see you on the | ||
| * upper/vertical part of the lift, so your voice should always come | ||
| * from there. */ | ||
| Vec3 playerPos; /* 0x58 */ | ||
| Quat playerRot; | ||
| /* There is a third-person camera position and quaternion immediately | ||
| * after the player above, but it doesn't work right when you get into | ||
| * a vehicle. At 0x1cc there is a more accurate camera structure that | ||
| * works in first-person, third-person, in a vehicle, and in the bird's | ||
| * eye view when hitting tab. */ | ||
| uint8_t _[0x158]; | ||
| Vec3 cameraPos; /* 0x1cc */ | ||
| Quat cameraRot; | ||
| }; |
There was a problem hiding this comment.
Add compile-time layout guards for remote-memory structs.
Line 246 reads TeardownPositional as a raw block from another process; without static_assert offset/size checks, compiler/layout drift can silently corrupt positional data.
Suggested hardening patch
+#include <cstddef>
+
struct Vec3 {
float x;
float y;
float z;
};
@@
struct TeardownPositional {
@@
Quat cameraRot;
};
+
+static_assert(sizeof(Vec3) == 0x0C, "Vec3 layout changed");
+static_assert(sizeof(Quat) == 0x10, "Quat layout changed");
+static_assert(offsetof(TeardownPositional, playerPos) == 0x00, "playerPos offset changed");
+static_assert(offsetof(TeardownPositional, playerRot) == 0x0C, "playerRot offset changed");
+static_assert(offsetof(TeardownPositional, cameraPos) == 0x174, "cameraPos offset changed");
+static_assert(offsetof(TeardownPositional, cameraRot) == 0x180, "cameraRot offset changed");Also applies to: 246-251
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/teardown/teardown.cpp` around lines 42 - 61, The TeardownPositional
struct is read raw from another process so add compile-time layout guards: use
static_assert with offsetof(TeardownPositional, playerPos) == 0x58,
offsetof(..., cameraPos) == 0x1cc, and static_assert(sizeof(TeardownPositional)
== 0x???) (replace ??? with the actual expected total size) to catch layout
drift; also assert sizeof(Vec3) and sizeof(Quat) if needed and ensure the
struct's packing/alignment matches the remote process (e.g., pragma pack or
attribute if the project uses it) so offsets are reliable.
|
No time to review this fully, just dropping in to say: Don't forget to add your .dll file to the ClientInstaller.cs as install target this time :3 |
2 participants
Positional audio support for the 2026 multiplayer release of Teardown. I think it's game version 2.0. (That's what it says in the bottom right corner of the screen in the main menu.)
Hi. I left a lot of notes in comments in the source. I hope it's not too much. A couple things came up while writing the plugin that I wanted to make obvious and explain in the comments.
The game uses quaternions for rotations so there is some math to calculate the forward and top vectors. Normally this involves a couple cross products but the implementations are simplified because the forward and top vectors have two zero components each. (I thought I could just write a function for a general cross product and the compiler would optimize
vec3_cross(quat, Vec3 { 0.f, 1.f, 0.f })into a special case but, when I checked on godbolt, that didn't seem to actually happen. So I wrote the special case implementations and added comments to show the math and make up for them being less readable.)The player/camera positional information can be read in main menu but we don't want positional audio to start there yet. So the plugin tries to read the game state, compares it to known states where positional audio doesn't make sense to apply, and does not read positional audio in that case. Otherwise, if it can't read the state or isn't in a single-player/menu/lobby state, it will try to read positional audio and set the context string so that in-game players don't hear positional data from in-menu players located at zero.
There are also some remarks about why certain memory regions were used. I haven't been able to test this much but I think it does a pretty okay job and supports third-person.
The player's rotation is a little suspect at times. I almost wonder if it should use the camera's rotation in place of the player rotation. But I'd have to test more to see how that changes the perception of things.
The game is modable with a lua API and custom characters. You can query player model bones from lua and get their transforms in lua scripts. (I assume you can make characters with any number of heads too, since they're just labels/tags on bones/bodies in the game?) It would be really hekin' cool to get the head's global transform and share that with mumble but I haven't come up with a way to do that from lua yet. It's all pretty locked down just like with factorio.
Finally, it uses a pattern to search for a base pointer to start the pointer chain. I'm not too sure if this will survive game updates because there haven't been a lot of versions of Teardown with multiplayer to test on.
Also, I tested this plugin with one other person but only in a limited amount. We're waiting for our friend group to all get the game when it goes on sale :3
Checks