FEAT/REFAC(positional-audio): Rewrite GTAV plugin, define "procid_t" to "uint64_t"

[davidebeatrici]

It should now work flawlessly with all variants (Rockstar, Steam, Epic Games).

[davidebeatrici]

@vimpostor Could you test the plugin, please?

[vimpostor]

This awesome, thank you for doing this! I will definitely test this, but don't have time to do that before Monday unfortunately.

[davidebeatrici]

No problem, thanks!

[vimpostor]

Confirmed working on Linux + Wine + GTA V Steam edition.
This is really awesome, finally no more looking through hardcoded memory addresses.

I only noticed two minor problems:

• Player top vector seems to be locked to one value forever
• When I first start Mumble, then start GTA, Mumble receives a MUMBLE_PDEC_ERROR_PERM once Rockstar launcher is about to close and start the real game (If I start Mumble after GTA is running already, then everything is fine)

[davidebeatrici]

Player top vector seems to be locked to one value forever

The player character is almost always perpendicular to the floor.

Enter a vehicle and you will see it changes, unless you're on a level terrain.

When I first start Mumble, then start GTA, Mumble receives a MUMBLE_PDEC_ERROR_PERM once Rockstar launcher is about to close and start the real game (If I start Mumble after GTA is running already, then everything is fine)

Confirmed, I'm pretty sure it happens because at that point the executable is not "unpacked" yet.
In fact, in order to be able to reverse engineer it you have to take a dump of the process memory as the game is running.

As a workaround we could return MUMBLE_PDEC_ERROR_TEMP if a signature is not found, until we find a proper solution.

[vimpostor]

The player character is almost always perpendicular to the floor. Enter a vehicle and you will see it changes, unless you're on a level terrain.

You are right.

> When I first start Mumble, then start GTA, Mumble receives a > `MUMBLE_PDEC_ERROR_PERM` once Rockstar launcher is about to close and > start the real game (If I start Mumble after GTA is running already, > then everything is fine) Confirmed, I'm pretty sure it happens because at that point the executable is not "unpacked" yet. In fact, in order to be able to reverse engineer it you have to take a dump of the process memory as the game is running. As a workaround we could return `MUMBLE_PDEC_ERROR_TEMP` if a signature is not found, until we find a proper solution.

I think that would be an acceptable workaround. How often does Mumble retry plugins that returned temporary error though?

[davidebeatrici]

A few seconds if I recall correctly.

@Krzmbrzl

[Krzmbrzl]

Every second - provided that there is currently no plugin that has established a link

mumble/src/mumble/PluginManager.h

Line 105 in dd5df45

static constexpr int POSITIONAL_DATA_CHECK_INTERVAL = 1000;

[davidebeatrici]

What do you think? Should we return MUMBLE_PDEC_ERROR_TEMP?

[Krzmbrzl]

Just marking all my previous review comments as "resolved" without actually addressing them does not seem like a productive strategy.
Should this become the norm, I will no longer provide code reviews in the first place. But for now I'll assume that it was merely some sort of oversight...

[Krzmbrzl]

What do you think? Should we return MUMBLE_PDEC_ERROR_TEMP?

Maybe add a counter. The first x times you fail to resolve the structs, you return a temp error but if you aren't able to resolve them after a bunch of attempts (that should cover the usual loading times of the game), then assume something is wrong and return a permanent error.
As soon as you are able to properly resolve the structs, reset the counter.

[davidebeatrici]

Just marking all my previous review comments as "resolved" without actually addressing them does not seem like a productive strategy.
Should this become the norm, I will no longer provide code reviews in the first place. But for now I'll assume that it was merely some sort of oversight...

I resolved them locally, I wanted to know whether I should return MUMBLE_PDEC_ERROR_TEMP before pushing.

Maybe add a counter. The first x times you fail to resolve the structs, you return a temp error but if you aren't able to resolve them after a bunch of attempts (that should cover the usual loading times of the game), then assume something is wrong and return a permanent error.
As soon as you are able to properly resolve the structs, reset the counter.

Not a good idea, the time it takes the launcher to load the game is not constant.

A proper solution would probably consist in getting a snapshot of the process memory at the time the plugin returns MUMBLE_PDEC_ERROR_PERM and check what kind of pattern we can use to detect such a situation.

But, for the time being returning MUMBLE_PDEC_ERROR_TEMP is a viable workaround.

[vimpostor]

But, for the time being returning MUMBLE_PDEC_ERROR_TEMP is a viable workaround.

I think it is a great workaround even. If the process called "GTA5.exe" is not running at all, we take the early exit with MUMBLE_PDEC_ERROR_TEMP in mumble_initPositionalData() anyway, so we only take the expensive search through the memory page every second, while the Rockstar launcher is running, which I guess is fine.

One unrelated optimization would be possible where each plugin tells Mumble what process name it is looking for, so that only Mumble itself iterates over all program names instead of n positional audio plugins iterating themself, but right now that does hardly matter because n is low (and this has nothing to do with this PR).

[davidebeatrici]

That's an excellent idea!

Could you open an issue for it, please?

[vimpostor]

Looks good to me now, thanks! :)
(I don't have an "Approve" button anywhere)

[davidebeatrici]

Thank you very much for testing!

[Krzmbrzl]

💚 All backports created successfully

Status	Branch	Result
✅	1.4.x

Questions ?

Please refer to the Backport tool documentation