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

Fix Left 4 Dead 2 plugin and clean up plugin code #2248

Closed
wants to merge 1 commit into from

Conversation

@Joeliam
Copy link
Contributor

commented May 7, 2016

Thanks to @mrzchuck for answering system based programming questions
and helping me with my first use of github.

This plugin supports the latest version of L4D2 (version 2.1.4.5)

Most of my code changes are based on the mumble wiki plugin guide
styling and/or the Counter Strike (CS) plugin (both of which are more
standardized and easier to read). I removed the calcout function by
pulling in the front and top vectors directly from L4D2. I simplified
the trylock function by calling fetch from it (previously trylock and
fetch contained duplicate code). The style for context support seemed
convoluted so I changed it to emulate the style of the CS plugin (the
logic is still the same). Lastly, I added many helpful comments
describing the variables, functions, and general principles of mumble
plugins.

I have performed extensive testing and found excellent results.

Fix Left 4 Dead 2 plugin and clean up plugin code
Thanks to @mrzchuck for answering system based programming questions
and helping me with my first use of github.

This plugin supports the latest version of L4D2 (version 2.1.4.5)

Most of my code changes are based on the mumble wiki plugin guide
styling and/or the Counter Strike (CS) plugin (both of which are more
standardized and easier to read). I removed the calcout function by
pulling in the front and top vectors directly from L4D2. I simplified
the trylock function by calling fetch from it (previously trylock and
fetch contained duplicate code). The style for context support seemed
convoluted so I changed it to emulate the style of the CS plugin (the
logic is still the same). Lastly, I added many helpful comments
describing the variables, functions, and general principles of mumble
plugins.

I have performed extensive testing and found excellent results.
@@ -1,6 +1,12 @@
/* Copyright (C) 2005-2012, Thorvald Natvig <thorvald@natvig.com>
/* Copyright (C) 2012-2016, Joel Kees <joelkees@gmail.com>

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 7, 2016

Member

Hi,

We're in the process of changing our license headers.
Ideally, we want all files to carry the simpler one from LICENSE.header that you have included below.
But we aren't there yet. In this case, we'd need d-rez's permission, because of the BSD license.

Here's what I suggest we do in this file, for now:

  • You can add yourself to the copyright line, no problem. (But please change "2012-2016" to be just "2016" -- that's correct, right?)
  • Please remove the "Mumble Developers" bit.
  • Can we get a statement from you that you allow us to remove your name from ANY headers in Mumble, in order to replace the license header in the file with the simplified header? Instead of being in the header, you'll be added to the AUTHORS file, automatically generated added based on Git history. The AUTHORS file will soon be visible in both Murmur (murmur --authors for AUTHORS, murmur --license for LICNESE) and Mumble (in the about dialog).

If we have your statement on file, we'll take care of migrating the headers in the future, once people have agreed. :-)

Personal note: In practice, we can change most of our headers already. There are only a few throughout the tree where we cannot.

@mkrautz

This comment has been minimized.

Copy link
Member

commented May 7, 2016

My only comment is the license bit, otherwise LGTM.

@mkrautz

This comment has been minimized.

Copy link
Member

commented May 7, 2016

@davidebeatrici You've told me you also found updated values for L4D2. Can you review the values in this patch, and vouch for them?

@mkrautz

This comment has been minimized.

Copy link
Member

commented May 7, 2016

@Joeliam Also, thank you very much for the contribution.

One more thing I have noticed is that your Git client is set up not to include your real name.

This will affect attribution of your changes. Right now, you would be credited as Joeliam <joelkees@gmail.com>, but in the license header, you've put Copyright (C) 2012-2016, Joel Kees <joelkees@gmail.com>. Please fix that!

@davidebeatrici

This comment has been minimized.

Copy link
Member

commented May 7, 2016

Hi,

Thank you for your contribution, I really appreciate it. I didn't update the plugin yet because I coded it from scratch and now I'm adding identity support to it. Happy to see another person supporting Left 4 Dead 2 😄

@mkrautz I tested the code, the values are correct 😉

position tuple: client.dll+0x774B40 float (x, y, z location in inches)
front tuple: client.dll+0x774BA0 float (x, y, z as unit vector in direction you are facing)
top tuple: client.dll+0x774BD0 float (x, y, z as unit vector pointing out top of head)
state: client.dll+0x772ACC bye (0 in menu; non-zero in game)

This comment has been minimized.

Copy link
@mkrautz

mkrautz May 7, 2016

Member

Typo: byte

@Joeliam

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2016

Thanks for all the inputs guys! I will fix the header as soon as I can. I will need to converse with my friend @mrzchuck again (still learning the github submission process 😃).

@davidebeatrici

This comment has been minimized.

Copy link
Member

commented May 8, 2016

You're welcome! 😉
I see that you're also a Portal fan 😃

@Joeliam

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2016

Yes @davidebeatrici, Portal is my all time favorite game ❤️.

@mkrautz, you have my permission to remove my name from any header in Mumble 😄.

@mkrautz

This comment has been minimized.

Copy link
Member

commented May 8, 2016

@Joeliam Thanks!

I can fix the typo and your name in the commit when landing.
You want your name in the commit to be

Joel Kees <joelkees@gmail.com>

and not

Joeliam <joelkees@gmail.com>

correct?

@Joeliam

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2016

That is correct. So do I need to fix anything? Or are you going to fix the name, date, and remove the mumble developer section. I do not mind recommitting. I'm just finding a time when my buddy can walk me through the process again.

@mkrautz

This comment has been minimized.

Copy link
Member

commented May 8, 2016

I can fix it for you, no worries! We just needed to agree on what should be done, and we do now.

@mkrautz

This comment has been minimized.

Copy link
Member

commented May 8, 2016

Landed via c8d136f.

@mkrautz mkrautz closed this May 8, 2016

mkrautz added a commit to mkrautz/mumble that referenced this pull request May 10, 2016
Add Joeliam to .mailmap and sync AUTHORS.
In PR mumble-voip#2248, I promised to change the commit's
author before landing. I didn't.

This adds an entry to the .mailmap to make up
for that.
mkrautz added a commit to mkrautz/mumble that referenced this pull request May 16, 2016
Add Joeliam to .mailmap and sync AUTHORS.
In PR mumble-voip#2248, I promised to change the commit's
author before landing. I didn't.

This adds an entry to the .mailmap to make up
for that.
mkrautz added a commit to mkrautz/mumble that referenced this pull request May 17, 2016
Add Joeliam to .mailmap and sync AUTHORS.
In PR mumble-voip#2248, I promised to change the commit's
author before landing. I didn't.

This adds an entry to the .mailmap to make up
for that.
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.