FEAT(positional-audio): Plugin for Grounded #6491

sqwishy · 2024-07-03T16:42:57Z

Positional audio support for the Steam release of Grounded 1.4.3.4578.

I don't really know what I'm doing but I gave it a shot. I'm open to feedback here or on matrix.

One thing I struggled with was moving ProcessWindows into a uniqe_ptr without closing the handle (which happens in the destructor). The default copy constructor for HostWindows doesn't really work with the RAII it's trying to do. I've deleted it and added a move constructor instead that should be valid. It's possible that what I'm doing with std::move is not a good approach, but I feel strongly that HostWindows's default copy constructor is invalid and deleting it is an improvement.

Also, I haven't tested this version with another person. I did test a similar program last year that used the Link plugin and that worked with another player. The memory layout has changed a bit since then; but I think the structures and xyz-mapping from the game to mumble are the same. So this should work.

I haven't built mumble on Linux yet, I can't promise that this doesn't break Linux builds. I thought I installed the dependencies for tumbleweed but CMake said something about not being able to find Qt5 Core. I'm really bad at CMake and debugging it always takes so much of my time so I don't want to go down that road unless it's necessary.

Caveats

No context or identity support.
The game's camera is used for both the avatar and camera position in mumble. The game is mostly first person. There are a few circumstances when the game temporarily switches to third person, like if the player opens their inventory menu. In that case, their voice will come from their camera behind their character model. I haven't found separate positional information for the character nearby, though I haven't looked very hard either. If this is a noticable problem I am open to resolving this. But when I played this the effect wasn't obvious.
Mumble's UI is a bit laggy and unresponsive while the game is open at the main menu, before loading in to a world.

The lookup this does to read positional information fails until after the user loads a world and selects their character. This implementation of mumble_initPositionalData will return MUMBLE_PDEC_ERROR_TEMP until that lookup is successful once. So, while the game is open, but before loading in, mumble_initPositionalData will open a handle to the process and attempt the lookup and fail. It feels like this is happening in the main/UI thread and opening a handle to the process or something takes a long enough time that the interface feels unresponsive as mumble is calling it repeatedly.

Alternatively, when the process is detected in mumble_initPositionalData, it could open a handle and return MUMBLE_PDEC_OK. In that case, the fetch function could attempt the lookup instead. The reason I didn't start with that approach was that if the lookup will never succeed, as a result a game update or something, it seemed preferable that the initialization fails rather than mumble reporting to the user that the game has linked.

Looking forward to your feedback.

Krzmbrzl · 2024-07-04T07:02:46Z

plugins/grounded/CMakeLists.txt

@@ -0,0 +1,18 @@
+
+# Copyright 2020-2023 The Mumble Developers. All rights reserved.


Suggested change

# Copyright 2020-2023 The Mumble Developers. All rights reserved.

# Copyright 2024 The Mumble Developers. All rights reserved.

Krzmbrzl · 2024-07-04T07:03:54Z

plugins/grounded/grounded.cpp

@@ -0,0 +1,177 @@
+#include "ProcessWindows.h"
+#define MUMBLE_PLUGIN_NO_DEFAULT_FUNCTION_DEFINITIONS


why? Unless you are including the plugin header multiple times, having the default definitions saves you the trouble of having to implement some functions yourself 🤔

I think I just copied this from elsewhere to try to be conventional, but I think you're right in that it doesn't seem appropriate here.

Krzmbrzl · 2024-07-04T07:04:40Z

plugins/grounded/grounded.cpp

+	uint8_t _unused1[4];
+	/* 950 */ float front[3];
+	uint8_t _unused2[4 * 40];


Suggested change

uint8_t _unused1[4];

/* 950 */ float front[3];

uint8_t _unused2[4 * 40];

std::uint8_t _unused1[4];

/* 950 */ float front[3];

std::uint8_t _unused2[4 * 40];

Krzmbrzl · 2024-07-04T07:05:41Z

plugins/grounded/grounded.cpp

+	/* 9fc */ float pos[3];
+};
+
+#define unreal_to_mumble_units(unreal) (unreal / 100.f)


Suggested change

#define unreal_to_mumble_units(unreal) (unreal / 100.f)

constexpr float unreal_to_mumble_units(float unreal) { return unreal / 100.0f; }

Krzmbrzl · 2024-07-04T07:06:53Z

plugins/grounded/grounded.cpp

+
+#define unreal_to_mumble_units(unreal) (unreal / 100.f)
+
+static_assert(sizeof(struct GroundedCam) == 200, "");


Suggested change

static_assert(sizeof(struct GroundedCam) == 200, "");

static_assert(sizeof(struct GroundedCam) == 200, "GroundedCam struct has unexpected size");

Krzmbrzl · 2024-07-04T07:09:44Z

plugins/grounded/grounded.cpp

+
+uint8_t mumble_initPositionalData(const char *const *programNames, const uint64_t *programPIDs, size_t programCount) {
+	for (size_t i = 0; i < programCount; ++i) {
+		if (strcmp(programNames[i], "Maine-Win64-Shipping.exe") != 0) {


I would define Maine-Win... as a std::string constant at the beginning of the function. Then you can simply do a programNames[i] != myConstant and also reuse the constant in other places where you need the process name (retyping it every time introduces the potential of typos)

plugins/grounded/grounded.cpp

Krzmbrzl · 2024-07-04T07:11:14Z

plugins/grounded/grounded.cpp

+			continue;
+		}
+
+		handle = std::make_unique< GroundedHandle >(GroundedHandle{ std::move(proc), p });


Suggested change

handle = std::make_unique< GroundedHandle >(GroundedHandle{ std::move(proc), p });

handle = std::make_unique< GroundedHandle >(std::move(proc), p);

Krzmbrzl · 2024-07-04T07:12:54Z

plugins/grounded/grounded.cpp

+	avatarAxis[0] = cameraAxis[0] = -cam.top[0];
+	avatarAxis[1] = cameraAxis[1] = cam.top[2];
+	avatarAxis[2] = cameraAxis[2] = -cam.top[1];
+
+	avatarDir[0] = cameraDir[0] = -cam.front[0];
+	avatarDir[1] = cameraDir[1] = cam.front[2];
+	avatarDir[2] = cameraDir[2] = -cam.front[1];


why don't we need a unit conversion for these fields?

Trigonometry. The range of values goes from -1 to 1.

Let's take cam.front[2] for example: if you look at the sky it will be ~1, if you look at the ground it will be ~-1.

They're unit vectors. They have a length of one in both the game and in mumble.

I see. In that case adding an explicit assert that checks their magnitude to not differ significantly from one would make the code more self-explanatory.
Also, a short comment could be added that these are unitless (direction only)

Yeah I can add comments that these should be unit vectors.

I couldn't find any other plugins to reference that do runtime assertions for these values. My understanding is a failed assertion typically causes the program to exit. If that happens, because the memory we read from the game isn't what we expect, we probably don't want mumble to quit. So I'm not sure an assert is the best choice here?

Instead, I could add a sanity check to initPositionalData that checks that the top and front vectors are of length zero (at the main menu) or one (in a game)?

Alternatively, in the general case, mumble could check that the vectors from fetch functions in positional audio plugins are of valid lengths and possibly respond by disabling the plugin if they appear to be misbehaving.

I couldn't find any other plugins to reference that do runtime assertions for these values. My understanding is a failed assertion typically causes the program to exit. If that happens, because the memory we read from the game isn't what we expect, we probably don't want mumble to quit. So I'm not sure an assert is the best choice here?

That's exactly what happens and that's also why it's a good thing (the crash happens right at the place where an assumption is violated - including information about where exactly the error occurred). Note however, that asserts are only active in debug builds.

I put in an assert.

But I think is worth mentioning that, depending on how memory is written and read, it seems maybe possible to me that mumble could read a copy of memory while these vectors are being updated by the game and partially written for a particular frame. My guess is that each dimension in the three-dimensional vectors are updated as separate instructions. And, though the timing would need to be very unfortunate, it's possible mumble could read a copy of this memory in the middle of an update. So one dimension might be accurate for the current frame and the others dimensions for the previous frame. In that case, the vector that mumble reads might not be a unit vector since it isn't accurate to one particular game state. Here, there isn't really anything mumble can do except maybe discard that fetch and/or try to read the memory again. I don't know that exiting is super useful, but ultimately that's not my decision.

On the other hand, it could make it very obvious if the plugin is attempting to be used but is out of date. So that could be nice.

Since it will only be an issue in debug builds that will be used by developers, I wouldn't consider this to be an issue. It's true that the assertion could produce a false-positive under very specific circumstances but I think it is still better to have the assumptions encoded into the code explicitly.

Krzmbrzl · 2024-07-04T07:19:23Z

One thing I struggled with was moving ProcessWindows into a uniqe_ptr without closing the handle (which happens in the destructor). The default copy constructor for HostWindows doesn't really work with the RAII it's trying to do. I've deleted it and added a move constructor instead that should be valid. It's possible that what I'm doing with std::move is not a good approach, but I feel strongly that HostWindows's default copy constructor is invalid and deleting it is an improvement.

So now that you are using the move ctor, things work as expected? Making the HostWindows class move-only sounds reasonable to me though 👍

Also, I haven't tested this version with another person. I did test a similar program last year that used the Link plugin and that worked with another player. The memory layout has changed a bit since then; but I think the structures and xyz-mapping from the game to mumble are the same. So this should work.

With Mumble 1.5 you can use the positional data viewer inside Mumble to see whether the coordinates are updated when you walk around in the game. If they seem correct, I don't see a reason why positional audio shouldn't work as expected.

The game's camera is used for both the avatar and camera position in mumble.

No problem

The lookup this does to read positional information fails until after the user loads a world and selects their character. This implementation of mumble_initPositionalData will return MUMBLE_PDEC_ERROR_TEMP until that lookup is successful once. So, while the game is open, but before loading in, mumble_initPositionalData will open a handle to the process and attempt the lookup and fail. It feels like this is happening in the main/UI thread and opening a handle to the process or something takes a long enough time that the interface feels unresponsive as mumble is calling it repeatedly.

Interesting 🤔
I might have to look into that at some point... The approach you chose is definitely the intended way to do things though.

davidebeatrici · 2024-07-04T13:08:07Z

So now that you are using the move ctor, things work as expected? Making the HostWindows class move-only sounds reasonable to me though 👍

Agreed, I would do the same with HostLinux in a separate commit.

Interesting 🤔
I might have to look into that at some point... The approach you chose is definitely the intended way to do things though.

Please note that this is a problem we were already aware of: #5728

Krzmbrzl · 2024-07-04T17:23:03Z

@davidebeatrici the issue being closed seems to imply that the underlying problem in Mumble has been fixed, no? But apparently that's not the case, unless OP is using an older version of Mumble 👀

davidebeatrici · 2024-07-04T22:42:20Z

The plugin for Left 4 Dead 2 was updated, but the underlying issue was not fixed.

davidebeatrici · 2024-07-10T13:43:14Z

@Krzmbrzl Unfortunately the issue is unacceptable, I would consider it a blocker. When the game is running and you're not in a game the entire client becomes laggy (audio included).

Krzmbrzl · 2024-07-10T14:00:46Z

How long exactly do calls to mumble_initPositionalData take and what is the frequency of these calls?

Looking at Mumble's implementation it seems like in

mumble/src/mumble/PluginManager.cpp

Lines 254 to 256 in b75fe54

    
           case MUMBLE_PDEC_ERROR_TEMP: 
        
           	// The plugin encountered a temporary error -> skip it for now (that is: do nothing) 
        
           	break;

we should probably just return false and set the active plugin to nullptr instead of continue with the looping. However, that shouldn't cause the lag as different processes should be filtered out by the plugin super fast (it should only need one string comparison).

I would expect the frequency of calls to mumble_initPositionalData to be 1 Hz:

mumble/src/mumble/PluginManager.cpp

Lines 127 to 130 in b75fe54

    
           m_positionalDataCheckTimer.setInterval(POSITIONAL_DATA_CHECK_INTERVAL); 
        
           m_positionalDataCheckTimer.start(); 
        
           QObject::connect(&m_positionalDataCheckTimer, &QTimer::timeout, this, 
        
           				 &PluginManager::checkForAvailablePositionalDataPlugin);

mumble/src/mumble/PluginManager.h

Lines 104 to 105 in b75fe54

    
           // How often the manager should check for available positional data plugins 
        
           static constexpr int POSITIONAL_DATA_CHECK_INTERVAL = 1000;

davidebeatrici · 2024-07-10T15:51:19Z

On Linux the issue doesn't appear to be present, in fact:

Begin:  1720626602201   ms
End:    1720626602201   ms
Begin:  1720626603160   ms
End:    1720626603160   ms
Begin:  1720626604160   ms
End:    1720626604160   ms
Begin:  1720626605159   ms
End:    1720626605159   ms
Begin:  1720626606160   ms
End:    1720626606160   ms
Begin:  1720626607158   ms
End:    1720626607158   ms
Begin:  1720626608159   ms
End:    1720626608159   ms
Begin:  1720626609159   ms
End:    1720626609159   ms
Begin:  1720626610160   ms
End:    1720626610160   ms
Begin:  1720626611167   ms
End:    1720626611167   ms
Begin:  1720626612160   ms
End:    1720626612160   ms
Begin:  1720626613159   ms
End:    1720626613159   ms
Begin:  1720626614160   ms
End:    1720626614160   ms
Begin:  1720626615159   ms
End:    1720626615159   ms
Begin:  1720626616160   ms
End:    1720626616160   ms
Begin:  1720626617159   ms
End:    1720626617159   ms
Begin:  1720626618162   ms
End:    1720626618162   ms
Begin:  1720626619160   ms
End:    1720626619160   ms
Begin:  1720626620159   ms
End:    1720626620159   ms
Begin:  1720626621159   ms
End:    1720626621159   ms
Begin:  1720626622160   ms
End:    1720626622160   ms
Begin:  1720626623159   ms
End:    1720626623159   ms
Begin:  1720626624159   ms
End:    1720626624159   ms
Begin:  1720626625165   ms
End:    1720626625165   ms
Begin:  1720626626160   ms
End:    1720626626160   ms
Begin:  1720626627159   ms
End:    1720626627159   ms
Begin:  1720626628159   ms
End:    1720626628159   ms
Begin:  1720626629161   ms
End:    1720626629161   ms
Begin:  1720626630160   ms
End:    1720626630160   ms
Begin:  1720626631160   ms
End:    1720626631160   ms
Begin:  1720626632159   ms
End:    1720626632159   ms
Begin:  1720626633160   ms
End:    1720626633160   ms
Begin:  1720626634161   ms
End:    1720626634161   ms
Begin:  1720626635160   ms
End:    1720626635160   ms
Begin:  1720626636159   ms
End:    1720626636159   ms
Begin:  1720626637159   ms
End:    1720626637159   ms
Begin:  1720626638157   ms
End:    1720626638157   ms
Begin:  1720626639159   ms
End:    1720626639159   ms
Begin:  1720626640160   ms
End:    1720626640160   ms
Begin:  1720626641159   ms
End:    1720626641159   ms
Begin:  1720626642159   ms
End:    1720626642159   ms
Begin:  1720626643159   ms
End:    1720626643159   ms
Begin:  1720626644160   ms
End:    1720626644160   ms
Begin:  1720626645160   ms
End:    1720626645160   ms
Begin:  1720626646160   ms
End:    1720626646160   ms
Begin:  1720626647159   ms
End:    1720626647159   ms
Begin:  1720626648158   ms
End:    1720626648158   ms
Begin:  1720626649160   ms
End:    1720626649160   ms
Begin:  1720626650159   ms
End:    1720626650159   ms

sqwishy · 2024-07-10T16:58:14Z

How long exactly do calls to mumble_initPositionalData take and what is the frequency of these calls?

On my machine in a debug build, the constructor to ProcessWindows takes about 380ms for Grounded. It's largely from HostWindows::modules() and I think that is mostly time spent in VirtualQueryEx.

davidebeatrici · 2024-07-10T17:14:50Z

I had a suspicion that was the culprit, embarrassing for Microsoft to say the least.

I wonder if this is a bug that was introduced in a recent build of Windows 11, since I'm pretty sure it was never encountered when we first wrote our interface.

Krzmbrzl · 2024-07-10T18:19:00Z

constructor to ProcessWindows takes about 380ms for Grounded

Ouch 👀

davidebeatrici · 2024-07-10T19:12:38Z

To be fair, VirtualQueryEx() is called several times (at least once per module) but it has no business taking so long just to retrieve some info.

And I'm not aware of another way to read the info we need...

sqwishy · 2024-07-12T15:42:28Z

And I'm not aware of another way to read the info we need...

I'm not aware how this plugin needs the information from VirtualQueryEx() anyway. It just needs the base address of the executable module. I think that tends to be the first module, but even if it looks modules up by name, the name is part of the MODULEENTRY32 from iterating with Module32First and Module32Next. So, at least for this plugin, it seems like HostWindows::modules() is doing a lot more work than it needs.

Nevertheless, it's probably not a bad idea to run this plugin initialization stuff in another thread instead of the main/GUI thread. But I haven't looked into how much work that would be.

davidebeatrici · 2024-07-12T18:59:43Z

You're absolutely right. We simply never noticed the issue until the new plugin framework became a thing (HostWindows already existed prior to that).

In my opinion plugin initialization should happen in a separate thread in any case, especially because the plugins that may be installed by a user are not guaranteed to be programmed correctly.

As for the modules: only a few games I've encountered hold memory areas of interest in libraries rather than the (main) executable. Refactoring the interface so that it doesn't iterate through all modules by default would make fully sense.

sqwishy · 2024-10-02T19:12:26Z

Just to recap. Previously, mumble would load all of the process modules and memory regions each time it tried to initialize the plugin. But the plugin would only succeed in initializing when a game world was loaded in. So mumble's UI thread would freeze every second as it tried and failed to initialize the Grounded plugin while Grounded was running on the computer but only at the main menu.

I changed the plugin a bit so that should initialize and link even at the main menu. And it should work better for future versions of the game. It can still lag for about a second in system calls while initializing, but in my testing it succeed to link the first try.

So arguably the problem preventing merging earlier might not be a problem anymore. Further down I explain the changes an what I found.

It uses findPattern to find code that helps start the pointer chain. The pointer it finds points to valid memory at the main menu, but it encounters a null pointer until the game world loads in. If we see this in fetching, the plugin considers it okay and expects the pointer chain to resolve to useful positional data later on.

I found the earlier implementation broke between minor game updates. This use of findPattern works across the most recent three patches. So I hope it is unlikely to break in the future. (It might be useful to clarify that in the plugin description somehow? The plugin isn't necessarily for a particular version but it is for the Steam release as opposed to the Microsoft XBox Game Pass release which has a different exe name last I checked.)

I'll copy some details about the slowness from the matrix thread:

HostWindows::modules() takes about 400ms to run on my machine in a release build. That function has an inner loop and outer loop:

The outer loop calls Module32First and Module32Next to collect Modules, a unordered map of Modules.

For each 'Module', an inner loop collects a set of MemoryRegions. A call to VirtualQueryEx is made for each MemoryRegion.

In Grounded, HostWindows::modules() finds 196 modules and 173,737 MemoryRegions across all of them. Most of the time spent in that function in VirtualQueryEx and adding MemoryRegions, both of these are part of the inner loop.

However, the only information I need in my plugin is the base address of the first module. Which shouldn't need VirtualQueryEx or the inner loop at all because the module address is provided by Module32First.

Since using findPattern now, using VirtualQueryEx is more useful than before as it can tell us which memory regions are executable and worth using findPattern in. But still it doesn't need to be loaded for every module.

With this change, HostWindows::modules() (400ms each) is called twice and findPattern() (<30ms) once in the plugin's positional data information. This is still pretty bad so it would be nice to improve this and do something similar to help L2D2 in #6558.

The only other concern is that if initialization fails, it will retry frequently and can cause performance issues. Even with the performance improvements planned, findPattern in another process's memory might not be a quick enough for the UI thread to do every second. I was wondering how other plugins do this and I noticed a TODO about this in the GTA 5 plugin.

One option is for the plugin to track how many times it has failed and return at some point with MUMBLE_PDEC_ERROR_PERM. On the other hand, this disables positional audio for the plugin entirely rather than just for the process that it saw. So it seems a bit heavy handed. I'm not sure if anybody else agrees with that, but, if we do, an option is for the plugin to record the pid along with its failure count and just skip the pid if it has failed to many times in the initialization. (I am not sure if pid re-use is a concern on windows.)

So all this is to say that there is room for improvement to handle failures with more grace. Not sure how necessary or important we feel that is?
And I'm still intent on changing how modules are enumerated in Windows so it doesn't take so long. I'd like to do that in a separate pull request. But I wanted to check in since the latest commits here do change the performance characteristics of the plugin and wanted to get your thoughts.

edit: I had profiled findPattern at taking about 300ms and I must have been testing a debug build, a release build with optimizations takes under 30ms.

davidebeatrici · 2024-10-03T03:25:36Z

Thank you very much for the extensive explanation and testing! Also, I really appreciate the use of the pattern matching technique as it makes the plugin much more robust to game updates (when done properly).

The pointer it finds points to valid memory at the main menu, but it encounters a null pointer until the game world loads in. If we see this in fetching, the plugin considers it okay and expects the pointer chain to resolve to useful positional data later on.

Perfectly fine, this matches the behavior of other plugins.

I found the earlier implementation broke between minor game updates. This use of findPattern works across the most recent three patches. So I hope it is unlikely to break in the future. (It might be useful to clarify that in the plugin description somehow?

I think specifying the minimum version that is known to be compatible is fine, for example:

Supports Grounded version >= 1.4.3.4578

That way if the plugin ever breaks the description will still be in part true.

The plugin isn't necessarily for a particular version but it is for the Steam release as opposed to the Microsoft XBox Game Pass release which has a different exe name last I checked.)

In theory the only difference, aside from the exe name, is the missing Steam integration.

Without checking we cannot be sure though and unfortunately I don't know anyone with that variant of the game.

Please rebase against master, squash the commits and then we're good. On that note, I strongly recommend to use a dedicated branch when creating a PR or when working on something in general. Ideally the local master should be synced with the upstream one.

Feel free to open an issue for VirtualQueryEx() and related performance issues, I will leave my opinion there.

Thank you again!

davidebeatrici · 2024-10-03T19:09:55Z

/home/vsts/work/1/s/plugins/grounded/grounded.cpp:16:13: error: ‘unique_ptr’ in namespace ‘std’ does not name a template type
   16 | static std::unique_ptr< GroundedHandle > handle;
      |             ^~~~~~~~~~
/home/vsts/work/1/s/plugins/grounded/grounded.cpp:13:1: note: ‘std::unique_ptr’ is defined in header ‘<memory>’; did you forget to ‘#include <memory>’?
   12 | #include <cstring>
  +++ |+#include <memory>
   13 | 
/home/vsts/work/1/s/plugins/grounded/grounded.cpp: In function ‘uint8_t mumble_initPositionalData(const char* const*, const uint64_t*, size_t)’:
/home/vsts/work/1/s/plugins/grounded/grounded.cpp:173:17: error: ‘handle’ was not declared in this scope
  173 |                 handle = std::make_unique< GroundedHandle >(std::move(proc), addr);
      |                 ^~~~~~
/home/vsts/work/1/s/plugins/grounded/grounded.cpp:173:31: error: ‘make_unique’ is not a member of ‘std’
  173 |                 handle = std::make_unique< GroundedHandle >(std::move(proc), addr);
      |                               ^~~~~~~~~~~
/home/vsts/work/1/s/plugins/grounded/grounded.cpp:173:31: note: ‘std::make_unique’ is defined in header ‘<memory>’; did you forget to ‘#include <memory>’?
/home/vsts/work/1/s/plugins/grounded/grounded.cpp:173:59: error: expected primary-expression before ‘>’ token
  173 |                 handle = std::make_unique< GroundedHandle >(std::move(proc), addr);
      |                                                           ^
/home/vsts/work/1/s/plugins/grounded/grounded.cpp:173:70: warning: ignoring return value of ‘constexpr typename std::remove_reference<_Tp>::type&& std::move(_Tp&&) [with _Tp = ProcessWindows&; typename std::remove_reference<_Tp>::type = ProcessWindows]’, declared with attribute ‘nodiscard’ [-Wunused-result]
  173 |                 handle = std::make_unique< GroundedHandle >(std::move(proc), addr);
      |                                                             ~~~~~~~~~^~~~~~
In file included from /usr/include/c++/11/bits/stl_pair.h:59,
                 from /usr/include/c++/11/bits/stl_algobase.h:64,
                 from /usr/include/c++/11/bits/stl_tree.h:63,
                 from /usr/include/c++/11/set:60,
                 from /home/vsts/work/1/s/plugins/Module.h:10,
                 from /home/vsts/work/1/s/plugins/HostLinux.h:9,
                 from /home/vsts/work/1/s/plugins/ProcessBase.h:13,
                 from /home/vsts/work/1/s/plugins/ProcessWindows.h:9,
                 from /home/vsts/work/1/s/plugins/grounded/grounded.cpp:6,
                 from /home/vsts/work/1/b/plugins/grounded/CMakeFiles/grounded.dir/Unity/unity_0_cxx.cxx:4:
/usr/include/c++/11/bits/move.h:104:5: note: declared here
  104 |     move(_Tp&& __t) noexcept
      |     ^~~~
In file included from /home/vsts/work/1/b/plugins/grounded/CMakeFiles/grounded.dir/Unity/unity_0_cxx.cxx:4:
/home/vsts/work/1/s/plugins/grounded/grounded.cpp: In function ‘void mumble_shutdownPositionalData()’:
/home/vsts/work/1/s/plugins/grounded/grounded.cpp:182:9: error: ‘handle’ was not declared in this scope
  182 |         handle.reset();
      |         ^~~~~~
/home/vsts/work/1/s/plugins/grounded/grounded.cpp: In function ‘bool mumble_fetchPositionalData(float*, float*, float*, float*, float*, float*, const char**, const char**)’:
/home/vsts/work/1/s/plugins/grounded/grounded.cpp:227:53: error: ‘handle’ was not declared in this scope
  227 |         const ProcessWindows &proc = std::get< 0 >(*handle);
      |                                                     ^~~~~~

Could you also make the first letter of "plugin" uppercase in the commit's title? Otherwise everything looks good to me.

sqwishy · 2024-10-03T20:39:15Z

Fixed commit title and that compiler error should be gone. I closed this on accident, sorry for the churn. I'm bad at windows and using git from windows is half broken and using anything in wsl is half broken so my setup is a recipe for mistakes but I did check that I could compile it in on fedora 40 in wsl.

davidebeatrici · 2024-10-04T00:21:33Z

No worries, it's fine.

davidebeatrici · 2024-10-04T20:56:28Z

Found 2 files with CRLF endings.
./plugins/grounded/grounded.cpp
./plugins/grounded/CMakeLists.txt

Positional audio support for the Steam release of Grounded. Tested on 1.4.3.4578, 1.4.4.4634, and 1.4.5.4679.

sqwishy · 2024-10-04T22:05:25Z

Found 2 files with CRLF endings.

oops my bad, I thought I had configured git right but apparently not. windows really does keep things surprising and interesting

Hartmnt added client positional audio feature-request This issue or PR deals with a new feature labels Jul 3, 2024

Krzmbrzl reviewed Jul 4, 2024

View reviewed changes

sqwishy force-pushed the master branch from 24cb894 to d381bae Compare July 4, 2024 18:34

sqwishy force-pushed the master branch 2 times, most recently from e8ed42d to b4b3122 Compare July 6, 2024 18:51

davidebeatrici mentioned this pull request Sep 21, 2024

Mumble lags and freezing when Left 4 Dead 2 is launched. PA not working #6558

Closed

sqwishy force-pushed the master branch from 02cd9ee to 89be65b Compare October 1, 2024 18:45

sqwishy requested a review from davidebeatrici October 2, 2024 19:12

davidebeatrici changed the title ~~FEAT(positional-audio): plugin for Grounded~~ FEAT(positional-audio): Plugin for Grounded Oct 3, 2024

sqwishy force-pushed the master branch from 89be65b to e24addd Compare October 3, 2024 05:13

sqwishy closed this Oct 3, 2024

sqwishy force-pushed the master branch from e24addd to 2b64b2c Compare October 3, 2024 20:20

sqwishy reopened this Oct 3, 2024

davidebeatrici mentioned this pull request Oct 4, 2024

FIX(plugins): Load correct pages for modules #6598

Merged

1 task

FEAT(positional-audio): Plugin for Grounded

2fa25dd

Positional audio support for the Steam release of Grounded. Tested on 1.4.3.4578, 1.4.4.4634, and 1.4.5.4679.

sqwishy force-pushed the master branch from 9b8b426 to 2fa25dd Compare October 4, 2024 21:54

		@@ -0,0 +1,18 @@

		# Copyright 2020-2023 The Mumble Developers. All rights reserved.

	# Copyright 2020-2023 The Mumble Developers. All rights reserved.
	# Copyright 2024 The Mumble Developers. All rights reserved.

		@@ -0,0 +1,177 @@
		#include "ProcessWindows.h"
		#define MUMBLE_PLUGIN_NO_DEFAULT_FUNCTION_DEFINITIONS

	#define unreal_to_mumble_units(unreal) (unreal / 100.f)
	constexpr float unreal_to_mumble_units(float unreal) { return unreal / 100.0f; }


		#define unreal_to_mumble_units(unreal) (unreal / 100.f)

		static_assert(sizeof(struct GroundedCam) == 200, "");

	handle = std::make_unique< GroundedHandle >(GroundedHandle{ std::move(proc), p });
	handle = std::make_unique< GroundedHandle >(std::move(proc), p);

FEAT(positional-audio): Plugin for Grounded #6491

Are you sure you want to change the base?

FEAT(positional-audio): Plugin for Grounded #6491

Conversation

sqwishy commented Jul 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Krzmbrzl commented Jul 4, 2024

davidebeatrici commented Jul 4, 2024

Krzmbrzl commented Jul 4, 2024

davidebeatrici commented Jul 4, 2024

davidebeatrici commented Jul 10, 2024

Krzmbrzl commented Jul 10, 2024

davidebeatrici commented Jul 10, 2024

sqwishy commented Jul 10, 2024

davidebeatrici commented Jul 10, 2024

Krzmbrzl commented Jul 10, 2024

davidebeatrici commented Jul 10, 2024

sqwishy commented Jul 12, 2024

davidebeatrici commented Jul 12, 2024

sqwishy commented Oct 2, 2024 • edited Loading

davidebeatrici commented Oct 3, 2024 • edited Loading

davidebeatrici commented Oct 3, 2024

sqwishy commented Oct 3, 2024 • edited Loading

davidebeatrici commented Oct 4, 2024

davidebeatrici commented Oct 4, 2024

sqwishy commented Oct 4, 2024

sqwishy commented Oct 2, 2024 •

edited

Loading

davidebeatrici commented Oct 3, 2024 •

edited

Loading

sqwishy commented Oct 3, 2024 •

edited

Loading