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

Added additional gun types and stat fields. #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

enragedginger
Copy link

Hi, thanks for putting this repository together. I was toying around with building my own replay parser in Clojure and parts of your library were helpful to have as a reference. I found a few more gun type values and post-game stat values in the replay file and decided to contribute those back to your project. I don't have C# installed, so I haven't compiled and tested this exact implementation, but it should give you a good starting point if you pick this project up again. I'd love to see what you come up with for doing parsing of the checkpoint and replay-data chunk types.

@fredimachado
Copy link
Owner

Hi Stephen,
Thanks for the PR!
I'll pull your changes and give it a try when I have a chance, before merging the PR.
Cheers,
Fredi

@Shiqan
Copy link

Shiqan commented Oct 18, 2018

Hi guys,
I actually cloned this repo recently as well to create a Python version. I have more or less the same findings as Stephen but I'll properly fork this repo over the weekend to add some additional gun types (and add the ones I didnt find 👍 ) and one of the missing values in match stats.

@enragedginger I see you have added accuracy, I couldnt verify that property. I just find some big number. Do I need to do something with that number to get accuracy?

@enragedginger
Copy link
Author

@Shiqan Accuracy is a float value. The code that I've put in this PR is trying to read it as reader.ReadUInt32(); which is definitely wrong. Have you tried reading it as a float in your code?

@Shiqan
Copy link

Shiqan commented Oct 18, 2018

Yep, you are right and you actually specified that in the model. I should have paid more attention ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants