chippydip / HearthLog

Client application to log Hearthstone games and upload those logs to
www.hearthlog.com
Apache License 2.0
54 stars 18 forks source link

small changes to allow compiling & running on OS X [tested in Mavericks] #5

Closed danlangford closed 10 years ago

danlangford commented 10 years ago

I do not do a lot of C++ or OS X programming so help me improve if there are mistakes.

I do enjoy Hearthstone and want this logger running on my Mac. With these changes I can build the project via XCode 5 and run the logger. It successfully posts up to hearthlog.com

HOWEVER I have not yet properly implemented Helper::GetHearthstoneVersion() and I need to look around for other methods that needs to be customized for OS X. i just though that in the mean time your users might want this addition. I can supply compiled binaries if desired.

chippydip commented 10 years ago

(For some reason my previous comment seems to have disappeared. Hopefully I can remember what I wrote before.)

Thanks for getting this running on OS X! I definitely want to support macs, but I don't have access to one currently to test/build on. I'm glad that the required changes seem relatively simple.

I do want to get the Version thing fixed before providing an official OS X release to the public. A bogus version number is currently parsed as the latest version (the site assumes the game was patched and it hasn't been updated yet), but I'm working on adding stats that are aggregated by patch to track how the game changes over time, so I'd rather not have to scan all old games and try to guess what their real version number was supposed to be.

It looks like the version info should be stored in the info.plist file in the root of an app's bundle. For example, this code is supposed to get the version info for the current application:

NSString version = (NSString )CFBundleGetValueForInfoDictionaryKey(CFBundleGetMainBundle(), kCFBundleVersionKey);

Obviously we want the version of the Hearthstone bundle, so that first argument would have to change, and then the result is returned as a string, so some minor parsing would be required to turn it into the 64-bit packed value the rest of the code expect.

I'd love to work with you on getting the Version stuff figured out on the mac if you'd like to help. Once that's done I'll happily merge your changes and support an official mac version of the app.

danlangford commented 10 years ago

i am more than willing to help you maintain this project especially on the mac side of things. and its all thanks to you and your choice of libraries (pcap, wxWidgets) that made it so easy to port over.

i will get the proper Hearthstone version wired up. hopefully in the next day or so.

sidenote: do you want any help on hearthlog.com? @narfdre and i ran disunity on all the game assets and we want to construct the board and to try to rebuild cards on the client dynamically. if you would be interested in any of that on hearthlog.com just let me know

chippydip commented 10 years ago

Do you have an XCode project file you can include as well? It would be nice for people to be able to build this out of the box themselves.

There are a few minor issues that's I'd like to clean up as well (pull the .app location from the config file so that non-default locations will work, fix the CFRelease code so that memory is released on errors as well, etc), but I can merge these changes and do that myself. I think I need to change how the program prompts for the Hearthstone install location if it doesn't find it automatically to make things look reasonable on both platforms anyway.

danlangford commented 10 years ago

i do have the xcode project files. would you care if 2 or 3 files where mixed into the Hearth Log subdirectory? or would you rather i move those off into another folder?

also if you want to go full gambit we could use CMake to configure and build the project and then check into git NO IDE specific files. that way you could use travis.ci to automatically build windows and mac artifacts for you. we could have ide specific files on our machines of course but .gitignore them making the actual code in github pretty raw and striaghtforward

2cents?

chippydip commented 10 years ago

As far as I can tell, travis-ci supports neither Windows nor OS X build platforms (yet?). If/when they add that ability or if there is another CI site that does support them I'd definitely look into it.

In any case, i think having IDE specific files in the repo is fine. My main goal in making this code open source was to make it easy for people to see what they were going to be running on their system and build their own copy if they wanted. Having IDE files for the supported platforms should make it a little easier on people if they decide they want to go that route.

A few extra files in the Hearth Log directory are fine. For Visual Studio there's the .sln file in the root dir and then .vcxproj and .vcxproj.filters files in the project directory. Similar files for XCode should be fine.

danlangford commented 10 years ago

re:travis-ci here was something about os x http://about.travis-ci.org/docs/user/osx-ci-environment/ dont know if its a part of their free offering or if there is also a windows/visual studio option

all the project files should be committed now. just the minimum to have another user be able to open it in xcode. ill have @narfdre checkout the project himself and open it in xcode to make sure i got everything

all Macs already have libpcap installed, the easiest way to get wxMac is via homebrew like brew install wxmac

danlangford commented 10 years ago

also as it stands currently you need to run the binary within the resulting package as root. this is far less than ideal. i.e. sudo /Applications/Hearth\ Log.app/Contents/MacOS/Hearth\ Log

gaak

this is required to access the network interfaces to sniff. if you google this problem people say "just add your user to the access_bpf group" and that worked before mavericks (i think) but its not working now.

i am still researching the appropriate fix. it will either be to nail down the right user group and document it well OR to get the app to ask for elevated permissions.

AND i know that this build is by far not bug free but it has been working for me so far. we will be able to improve it

chippydip commented 10 years ago

That is annoying. WinPCap installs a driver that must be started as an admin, but then can be used by other users (as far as I'm aware).

Some searching around suggests that the "access_bpf" group is something setup by Wireshark on install, so if you haven't installed it you wouldn't have that group. Apparently Wireshark creates that group and then sets up a script that runs at startup to change the permissions on the bpf device, allowing this to work (see the first reply to the accepted answer here: http://stackoverflow.com/questions/13031601/how-do-you-debug-libpcap-code-as-a-normal-user-on-os-x-with-xcode).

Since the steps required to actually get this working seem a bit complex, the best work around at this point might be to just say "install Wireshark" and be done with it.

danlangford commented 10 years ago

ya my problem is that i have installed wireshark, that group was created, i was added to that group, the group ownership of the bpf devices have been changed to the necessary group and i still am having issues. ill work through them

cocoapacketanalyzer is very similar to wireshark only it uses cocoa and not x11 which mac users love. it doesnt mess with groups and device ownership it just prompts for an admins password which isnt TOO intrusive

chippydip commented 10 years ago

Re travis-ci: Interesting...I don't see the OS X stuff linked from the main docs section in the menu anywhere, but it does pop up via search. I'll have to look into this more at some point, but I don't think it adds a ton of value at this point unless/until there are tests to go along with the program (which would be great, but it's no small task to mock up a testing environment for game logging).

Re wxMac: The windows builds I do include the required wx dlls along side the exe. This would probably be a more convenient way to package the mac version as well. Being able to support drag+drop installation (outside of that pcap/Wireshark issue) would definitely be nice.

Edit: Actually, adding an installer that will setup the logger to run at startup is on my TODO list. Perhaps that's the best way to go on the mac as well to get around the permission issues?

danlangford commented 10 years ago

i just reinstalled all of wireshark and made sure i could actually get it up and running and listening to packets. after i did that Hearth Log was able to get access to everything it needed. maybe the wireshark install just didnt survive my Mavericks update

chippydip commented 10 years ago

Found this thread on the issue: https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=3760

There is a recent post complaining about problems with the Wireshark thing not working as well as a separate .pkg installer that sounds like it just does the chmod stuff. Perhaps that .pkg would be an alternative to a full Wireshark install.

In any case, glad you got it working with Wireshark as well.

danlangford commented 10 years ago

the wxMac libs should only needed for the developer, not the end user. however for that to be true the developer will either need to be sure that wxMac is compiled statically or that the necessary wx libs are included in the bundle. im researching which approach is best and which is easiest and how to automate/configure as much of it as possible.

danlangford commented 10 years ago

if you are ok with were the code stands currently then you could approve the pull request and we could put issues in your issue tracker for some of the other Mac improvements we want to make down the road. maybe on your website explain that if you want to compile a Mac version yourself you can. and a Mac download is pending. that might also help drum up some interest and support from people who have more experience than i.

chippydip commented 10 years ago

Sounds like a plan!