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

Add plugin for Final Fantasy XIV #2653

Merged
merged 1 commit into from Nov 27, 2016

Conversation

@EmperorArthur
Copy link
Contributor

commented Nov 25, 2016

This is the first draft of a positional audio plugin for Final Fantasy XIV. It probably requires more work before being merge ready. I used the l4d2 plugin as a base, with a few patterns found in the wow plugin.

I'd appreciate some feedback, especially since I'm pretty sure I've messed up the coordinate system. I couldn't find any documentation on how to debug this thing aside from littering the code with cout statements.

The plugin does compile on Linux, though it's not set to in this pull request. I'm in the process of setting up a Windows build environment, so I haven't had a chance to do any testing.

What do you all like about this code? What do you dislike, and what should I change?

@mkrautz

This comment has been minimized.

Copy link
Member

commented Nov 25, 2016

Hi @EmperorArthur,
Do you run FFXIV on Wine?

I haven't yet reviewed the PR, but here's my WIP branch for wine plugin support, if you're interested:
https://github.com/mkrautz/mumble/commits/wine-plugins-v5

@@ -12,7 +12,7 @@ SUBDIRS = link
DIST = plugins.pri

win32 {
SUBDIRS += aoc arma2 bf1942 bf2 bf3 bf2142 bfbc2 bfheroes bf4_x86 blacklight borderlands borderlands2 breach cod2 cod4 cod5 codmw2 codmw2so cs css dods dys etqw tf2 gmod gtaiv gw hl2dm insurgency jc2 l4d l4d2 lol lotro ql rl sr sto ut2004 ut3 ut99 wolfet wow
SUBDIRS += aoc arma2 bf1942 bf2 bf3 bf2142 bfbc2 bfheroes bf4_x86 blacklight borderlands borderlands2 breach cod2 cod4 cod5 codmw2 codmw2so cs css dods dys etqw tf2 gmod gtaiv gw hl2dm insurgency jc2 l4d l4d2 lol lotro ql rl sr sto ut2004 ut3 ut99 wolfet wow ffxiv

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Nov 25, 2016

Member

ffxiv should be after etqw, to keep the alphabetical order.

include(../plugins.pri)

TARGET = ffxiv
SOURCES = ffxiv.cpp

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Nov 25, 2016

Member

Wrong indentation.

This comment has been minimized.

Copy link
@EmperorArthur

EmperorArthur Nov 25, 2016

Author Contributor

This looks like a github issue. If you view the file directly, the problem disappears.

This comment has been minimized.

Copy link
@mkrautz

mkrautz Nov 25, 2016

Member

It's fine.

avatar_front[2] = 0;

if(camera_is_free) {
camera_front[0] = -std::cos(camera_heading) ;

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Nov 25, 2016

Member

Space after the bracket.

const procptr32_t identity_offset = 48; // Name
const procptr32_t avatar_pos_offset = 160; // X, Z, Y
const procptr32_t avatar_heading_offset = 176; // Heading (-pi to pi)
//Camera struct offsets

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Nov 25, 2016

Member

This comment should be "// Camera struct offsets".

peekProc(avatar_address + avatar_pos_offset, &avatar_pos_corrector, 12) && // Avatar Position values (X, Z and Y).
peekProc(avatar_address + avatar_heading_offset, &avatar_heading, 4) && //Avatar heading float.
// peekProc(pModule, servername) && // Server name.
peekProc(pModule + map_id_address, &map_id,4) && // Map id.

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Nov 25, 2016

Member

Missing space after the comma.

const procptr32_t camera_ptr = 0x1045C40;
const procptr32_t avatar_ptr = 0x10468EC;
const procptr32_t state_address = 0x1048C60;
//const procptr32_t servername_address = 0;

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Nov 25, 2016

Member

If the server name isn't available I would just remove it.

peekProc(camera_address + camera_is_free_offset, &camera_is_free, 1) && // Camera is in first person mode
peekProc(avatar_address + avatar_pos_offset, &avatar_pos_corrector, 12) && // Avatar Position values (X, Z and Y).
peekProc(avatar_address + avatar_heading_offset, &avatar_heading, 4) && //Avatar heading float.
// peekProc(pModule, servername) && // Server name.

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Nov 25, 2016

Member

If the server name isn't available I would just remove it.

std::ostringstream ocontext;
ocontext << "{";

// Server name

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Nov 25, 2016

Member

If the server name isn't available I would just remove it.

std::wostringstream oidentity;
oidentity << "{";

// Server name

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Nov 25, 2016

Member

If the server name isn't available I would just remove it.

@davidebeatrici

This comment has been minimized.

Copy link
Member

commented Nov 25, 2016

Hi @EmperorArthur,

You're not obliged to setup a full environment to compile plugins, see the Wiki page for a small guide on how to do it.

To test plugins we have a dedicated program called MumblePAHelper, you can download it from here or build it yourself if you want the latest version; we still have to setup a builder for it. 😉

avatar_front[1] = std::sin(avatar_heading);
avatar_front[2] = 0;

if(camera_is_free) {

This comment has been minimized.

Copy link
@mkrautz

mkrautz Nov 25, 2016

Member

Should be if (camera_is_free) {

// that can be found in the LICENSE file at the root of the
// Mumble source tree or at <https://www.mumble.info/LICENSE>.

#ifdef WIN32

This comment has been minimized.

Copy link
@mkrautz

mkrautz Nov 25, 2016

Member

No need to ifdef. This plugin is only for Windows for now. Just replace this block with the Windows include.

We'll handle Wine inside mumble_plugin_win32_32bit.h/mumble_plugin_win32_64bit.h when it lands.

// X is East/West, West is increasing values
// Heading is in radians, 0 faces south

// Memory offsets

This comment has been minimized.

Copy link
@mkrautz

mkrautz Nov 25, 2016

Member

Please use tabs for indentation, but spaces for lining things up.

In this case, the whitespace between camera_ptr, avatar_ptr, etc. should be spaces, not tabs.

This comment has been minimized.

Copy link
@EmperorArthur

EmperorArthur Nov 26, 2016

Author Contributor

Ok, i was worried about it after @davidebeatrici mentioned them being mixed. It's the same in the l4d2 plugin.

Will work on fixing.

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Nov 26, 2016

Member

I was referring to the peekProc and variables parts. 😉

// Create containers to stuff our raw data into, so we can convert it to Mumble's coordinate system
float avatar_pos_corrector[3], camera_pos_corrector[3], avatar_heading, camera_heading;
// Values for extra features
// char servername[50];

This comment has been minimized.

Copy link
@mkrautz

mkrautz Nov 25, 2016

Member

Delete?

include(../plugins.pri)

TARGET = ffxiv
SOURCES = ffxiv.cpp

This comment has been minimized.

Copy link
@mkrautz

mkrautz Nov 25, 2016

Member

It's fine.

@EmperorArthur

This comment has been minimized.

Copy link
Contributor Author

commented Nov 26, 2016

Good news, this compiles successfully on MSVC 2010, 2013, and 2015. I know because I installed all three of them yesterday.

Bad news (for me), MumblePAHelper and Mumble snapshot don't recognize any compiled plugin. At this point I'm stuck.

@davidebeatrici

This comment has been minimized.

Copy link
Member

commented Nov 26, 2016

You must use the same compiler (and architecture) for Mumble/MumblePAHelper and the plugins.

@mkrautz

This comment has been minimized.

Copy link
Member

commented Nov 26, 2016

For Mumble 1.3.x, that's MSVC 2013, for Mumble 1.2.x, it's MSVC2010.

@EmperorArthur

This comment has been minimized.

Copy link
Contributor Author

commented Nov 26, 2016

Ok, the problem was everything, including MumblePAHelper, is compiled for X64, but the wiki didn't mention that. That should probably be fixed.

One final question before this thing is ready. Does this sound correct?

With the character facing South, Avatar Front is X: 1, Y: 0, Z: 0
With the character facing East, Avatar Front is X: 0, Y: 0, Z: 1
With the character facing South Avatar_Pos_X increases while going West.
With the character facing South Avatar_Pos_Z increases while going South.
With the character facing South Avatar_Pos_Y increases while going Up.

South was chosen as the reference direction since heading (in radians) is 0 pointing South.

@mkrautz

This comment has been minimized.

Copy link
Member

commented Nov 27, 2016

Seems wrong that the coordinate system for avatar_front is different from avatar_pos.. (South is X is one, in the other?)

Also, how is avatar_front's Z ever anything but 0 when it's set to 0 here? https://github.com/mumble-voip/mumble/pull/2653/files#diff-be1da379944da5c11f9a33c6658fcc3aR128

@EmperorArthur

This comment has been minimized.

Copy link
Contributor Author

commented Nov 27, 2016

I was confused about the coordinate system. Looking at the code for the manual placement plugin helped straighten things out.

actor_front Y is always zero. The player can not tilt up and down. There may be motions that look like it, but those animations do not affect the player's coordinates.

This code works, but I believe I can move things to North aligned instead of South aligned. Also, would you like me to squash all commits and rebase to master?

side note: I really wish the manual placement plugin was an actual plugin. It would have made experimenting, and understanding the coordinate system so much easier.


/*
Mumble | Game (in L hand coordinate system)
X | -X

This comment has been minimized.

Copy link
@EmperorArthur

EmperorArthur Nov 27, 2016

Author Contributor

I may be able to swap this to X, -Y, Z and save all the negatives in the azimuth calculations.

This comment has been minimized.

Copy link
@mkrautz

mkrautz Nov 27, 2016

Member

I have no opinion. As long as the coordinate system is mapped sensibly onto the Mumble coordinate system, I'm happy.

if (!camera_address) return false;

// Peekproc and assign game addresses to our containers, so we can retrieve positional data
ok = peekProc(pModule + state_offset, &state, 1) && // Magical state value: 0 or 255 when in main menu and 1 when in-game.

This comment has been minimized.

Copy link
@EmperorArthur

EmperorArthur Nov 27, 2016

Author Contributor

Tabs or Spaces in aligning things here?

This comment has been minimized.

Copy link
@mkrautz

mkrautz Nov 27, 2016

Member

Tabs are OK here. It's once you start aligning things you should use spaces.

This comment has been minimized.

Copy link
@EmperorArthur

EmperorArthur Nov 27, 2016

Author Contributor

Ok, I'm aligning all the && // so I'll use tabs for the indentation, and spaces to align those.

@mkrautz

This comment has been minimized.

Copy link
Member

commented Nov 27, 2016

I was confused about the coordinate system. Looking at the code for the manual placement plugin helped straighten things out.

actor_front Y is always zero. The player can not tilt up and down. There may be motions that look like it, but those animations do not affect the player's coordinates.

This code works, but I believe I can move things to North aligned instead of South aligned.

Sounds like a good idea to me. I'm not sure what other plugins do, though!
Not a top priority, as long as the coordinate system is consistent.

Also, would you like me to squash all commits and rebase to master?

Yes, please!

side note: I really wish the manual placement plugin was an actual plugin. It would have made experimenting, and understanding the coordinate system so much easier.

It was, until very recently. We made it part of Mumble because of the Qt UI... Perhaps we can do something similar in MumblePAHelper, but use the link plugin?

ffxiv Plugin: Add a plugin for Final Fantasy XIV
This only supports the DirectX 9 version of the game.

@EmperorArthur EmperorArthur force-pushed the EmperorArthur:ffxiv branch from 0b6c6fd to 07308dc Nov 27, 2016

@EmperorArthur

This comment has been minimized.

Copy link
Contributor Author

commented Nov 27, 2016

Here's, what should be, the final version. Ready to merge, hopefully.

There is a 64 bit version of the game, but I will submit that as a separate pull request.

@davidebeatrici

This comment has been minimized.

Copy link
Member

commented Nov 27, 2016

Looks good to me. 😉

Does the DirectX 11 version of the game use a different executable?

@mkrautz mkrautz changed the title Add a plugin for Final Fantasy XIV (feedback needed) Add plugin for Final Fantasy XIV Nov 27, 2016

@mkrautz mkrautz merged commit 8bbb34d into mumble-voip:master Nov 27, 2016

@mkrautz

This comment has been minimized.

Copy link
Member

commented Nov 27, 2016

Landed. Thanks a bunch! :-)

@EmperorArthur EmperorArthur deleted the EmperorArthur:ffxiv branch Nov 27, 2016

@EmperorArthur

This comment has been minimized.

Copy link
Contributor Author

commented Nov 27, 2016

Thanks for all your help. I enjoy Mumble, and appreciate everything you all do.

@davidebeatrici

This comment has been minimized.

Copy link
Member

commented Nov 27, 2016

You're welcome!
Thank you very much for your contribution! 😄

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