Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Positional Audio support for Battlefield 4 (x64) #2255

Closed
wants to merge 6 commits into from

Conversation

@davidebeatrici
Copy link
Member

commented May 8, 2016

Context based on Server ID.
Identity data: IP address + port, team, squad and squad leader.

Positional Audio support for Battlefield 4 (x64)
Context based on Server ID.
Identity data: IP address + port, team, squad and squad leader.
if (squad_leader)
oidentity << std::endl << "\"Squad leader\": \"Yes\""; // If squad leader value is 1, set squad leader state to "Yes" as squad leader in identity
else
oidentity << std::endl << "\"Squad leader\": \"No\""; // If squad leader value is 1, set squad leader state to "No" as squad leader in identity

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Do you mean if the value is 0?

oidentity << std::endl << "\"Squad\": \"Zulu\"";

if (squad_leader)
oidentity << std::endl << "\"Squad leader\": \"Yes\""; // If squad leader value is 1, set squad leader state to "Yes" as squad leader in identity

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Should this be a boolean instead of a string?

So

"Yes" -> true

and

"No" -> false?


std::string ServerID(serverid);
std::ostringstream ocontext;
ocontext << " {\"Server ID\": \"" << ServerID << "\"}"; // Set context with server ID

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

If ServerID contains the " character, the context will contain invalid JSON.

You probably need to add a function to escape your strings to be JSON-safe.

For example:

std::string jsonStringEscape(const std::string &input) {
   std::string output(input);
   std::replace(output.begin(), output.end(), '"', ' '); // replace '"' character with ' ' (space)
   return output;
}
std::string Host(host);
if (Host.find("bot") == std::string::npos) // Set host string as empty if "bot" is found in it
if (!Host.empty()) {
oidentity << std::endl << "\"Host\": \"" << host << "\""; // If it's not empty, set Host (IP:Port) in identity

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Also needs to be escaped.


return true; // This tells Mumble to ignore all vectors.
}

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

In the code below, you are using the C strings ServerID, host and team as-is.

However, if they (for some reason) do not contain a NUL byte, you'll crash Mumble.

Instead, you should probably convert them to std::string before using them.

std::string sserverid(serverid, sizeof(serverid));
std::string shost(host, sizeof(host));
std::string steam(team, sizeof(team));

and use those names in the code below.

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Ah, sorry... I didn't see the constructor JUST BELOW here.

Anyway, the point is: please use the std::string constructor with an explicit size.
Then the NUL bytes do not matter!

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Actually, we can't use my suggestion above, so don't do that. I'll explain...

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

If we use the std::string(char *, size_t) constructor, the string will end up with NUL bytes in it.
Technically that should be OK, but a lot of things break when that's the case (for example, Ice RPC doesn't like having NUL bytes in its strings for some languages)

Instead, you should make sure the strings are zero-terminated.

So, you need to do:

// NUL terminate queried C strings.
// We do this to ensure the strings from the game
// are NUL terminated. They should be already, but
// we can't take any chances.
serverid[sizeof(serverid)-1] = 0;
host[sizeof(host)-1] = 0;
team[sizeof(team)-1] = 0;
@davidebeatrici

This comment has been minimized.

Copy link
Member Author

commented May 8, 2016

Yes, thanks for the corrections 😄

First correction:
Yes, I didn't change 1 to 0 when I copied the line, sorry.

Second correction:
I always interpreted boolean as
Yes -> 1
and
No -> 0
This is why I wrote that in the comment 😉

@mkrautz

This comment has been minimized.

Copy link
Member

commented May 8, 2016

To be clear, what I mean by comment regarding the booleans is that JSON has a native boolean values true and false. It would make sense to use those, instead of your string types "Yes" and "No". Just so we're clear. :-)

@mkrautz

This comment has been minimized.

Copy link
Member

commented May 8, 2016

Done with my first round of comments.

I'm thinking it might also make sense to split the code that generates the identity string out into a separate function. (Context seems small enough to me to not have its own function).

Changed Squad Leader boolean values comment
Changed to "true" and "false" to be clear.
Thanks to mkrautz.
@mkrautz

This comment has been minimized.

Copy link
Member

commented May 8, 2016

Added new comments about the NUL terminating issues (just so you don't miss 'em).

@davidebeatrici

This comment has been minimized.

Copy link
Member Author

commented May 8, 2016

I didn't see your last comments when I replied, I'm fixing all of the things you mentioned step by step; I will reply again when I have done everything 😉

Added QT Project file
I didn't add it at first because I didn't know if the libs would have changed or not with a 64 bit plugin.
Thanks to mkrautz.
Fix jsonStringEscape mistake
I accidentally passed the char values to jsonStringEscape function instead of the strings. This resulted in a compile error (cannot convert char to string).

@mkrautz mkrautz self-assigned this May 8, 2016

}

if (squad == 0) {
oidentity << std::endl << "\"Squad\": \"No\""; // If squad value is 0, set squad state to "No" in identity.

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Maybe null instead of "No"? Would make more sense in JSON, I suppose.

else if (squad == 26)
oidentity << std::endl << "\"Squad\": \"Zulu\"";

if (squad_leader)

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Still not boolean true/false.

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Should be if (squad_leader == 1) {.

@mkrautz

This comment has been minimized.

Copy link
Member

commented May 8, 2016

LGTM with my latest comments are addressed.

Update: no, I have more comments.

peekProc((BYTE *) pModule + 0x21B80C0, host) && // Host value: "IP:Port" when in a server, "bot" when loading and empty when it's hidden.
peekProc((BYTE *) pModule + 0x24AFAE5, team) && // Team value: US (United States); RU (Russia); CH (China).
peekProc((BYTE *) squad_offset_2 + 0x230, squad) && // Squad value: 0 (not in a squad); 1 (Alpha); 2 (Bravo); 3 (Charlie)... 26 (Zulu).
peekProc((BYTE *) squad_offset_2 + 0x234, squad_leader); // Team value: US (United States); RU (Russia); CH (China).

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

The comment here is wrong.

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Also, don't read squad_leader into a bool. Read it into a BYTE, like squad.

oidentity << std::endl << "\"Squad\": \"Zulu\"";

if (squad_leader)
oidentity << std::endl << "\"Squad leader\": \"Yes\""; // If squad leader value is true, set squad leader state to "Yes" as squad leader in identity.

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Should be: oidentity << std::endl << "\"Squad leader\": true";

(NOTE: NO trailing comma, as this is the last key/value pair in the JSON)

if (squad_leader)
oidentity << std::endl << "\"Squad leader\": \"Yes\""; // If squad leader value is true, set squad leader state to "Yes" as squad leader in identity.
else
oidentity << std::endl << "\"Squad leader\": \"No\""; // If squad leader value is false, set squad leader state to "No" as squad leader in identity.

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Should be: oidentity << std::endl << "\"Squad leader\": false";

(NOTE: NO trailing comma, as this is the last key/value pair in the JSON)

oidentity << "\"Team\": \"Russia\""; // If team value is RU, set "Russia" as team in identity.
}

if (squad == 0) {

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

I think this code should be restructured.

Remove this check here and unindent the lines below it.

So the code becomes:

if (squad == 1) {
    oidentity << std::endl << "\"Squad\": \"Alpha\",";
} else if (squad == 2)
    oidentity << std::endl << "\"Squad\": \"Bravo\",";
[...]
} else {
    oidentity << std::endl << "\"Squad\": null,"
}

This makes the code handle invalid values (values that are 0, or > 26) consistent, and simplifies the code.

Host = jsonStringEscape(Host);
if (Host.find("bot") == std::string::npos) // Set host string as empty if "bot" is found in it.
if (!Host.empty()) {
oidentity << std::endl << "\"Host\": \"" << host << "\""; // If it's not empty, set Host (IP:Port) in identity.

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Seems like the JSON needs a comma after the last ", so it becomes:

oidentity << std::endl << "\"Host\": \"" << host << "\",";
if (!Team.empty()) {
oidentity << std::endl;
if (Team == "US")
oidentity << "\"Team\": \"United States\""; // If team value is US, set "United States" as team in identity.

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Needs comma at the end, like was done for "Host".

if (Team == "US")
oidentity << "\"Team\": \"United States\""; // If team value is US, set "United States" as team in identity.
else if (Team == "CH")
oidentity << "\"Team\": \"China\""; // If team value is CH, set "China" as team in identity.

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Needs comma at the end, like was done for "Host".

else if (Team == "CH")
oidentity << "\"Team\": \"China\""; // If team value is CH, set "China" as team in identity.
else if (Team == "RU")
oidentity << "\"Team\": \"Russia\""; // If team value is RU, set "Russia" as team in identity.

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Needs comma at the end, like was done for "Host".

} else {
// If squad value is in a value between 1 and 26, set squad name in identity using NATO Phonetic alphabet.
if (squad == 1)
oidentity << std::endl << "\"Squad\": \"Alpha\"";

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 8, 2016

Member

Needs comma at the end, like was done for "Host".

Same for all the following Squad values.

@mkrautz

This comment has been minimized.

Copy link
Member

commented May 8, 2016

OK, done with this round of reviewing. Please address my comments and ping me. :-)

Thanks.

Various fixes
Declared "squad_leader" variable as BYTE instead of Boolean for size and compatibility reasons.
Fixed "squad_leader" peekProc comment.
Added commas for correct JSON format.
Improved Squad/Squad Leader system using better IF Statements.
@mkrautz

This comment has been minimized.

Copy link
Member

commented May 8, 2016

Thanks!

Landed via a30f1dd.

@mkrautz mkrautz closed this May 8, 2016

@davidebeatrici

This comment has been minimized.

Copy link
Member Author

commented May 9, 2016

You're welcome 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.