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

Windows Powershell readme update + Decimal vs * 10 w/ early return #3

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Luffles
Copy link

@Luffles Luffles commented Aug 22, 2021

Probably moot w/ git bash or wsl, but might be useful for people who forget about it until it's done (me)

might be useless as you can just do it from git bash or wsl, but i realized that after bothered to write it up.
Windows SymLink readme addition
Didn't test on OSX, just Windows
@Luffles Luffles changed the title Windows Powershell readme update Windows Powershell readme update + Decimal vs * 10 w/ early return Aug 22, 2021
@Luffles
Copy link
Author

Luffles commented Aug 22, 2021

sorry i didn't test this on my macbook, i assume everything is fine and it will Just Work ™

maybe decimal isn't what you intended for output, but i think it looks better for win rate %, imo.

apologies if you weren't looking for PRs to this @jdb8 .

hs

@jdb8
Copy link
Owner

jdb8 commented Aug 22, 2021

@Luffles always happy to accept PRs, thanks for taking the time!

Regarding the percentage display, the reason I went with 3-digit displays for win rate (like 568 for a card with a 56.8% deck win rate) is because the macOS version of HSTracker seems to only be able to display numbers, not decimals (and only supports up to 3 digits). So in order to make sure we have the full percentage visible, it needs to be formatted that way unfortunately. Example:

image

This limitation might not apply to the Windows version, but unfortunately I don't have a Windows machine to test on at the moment. I'm actually surprised this repo is even useful for Windows, since I thought that HSReplay win rates were already implemented outside of macOS.

If this repo is also useful for Windows then I'd be open to supporting a flag to choose whether the script outputs percentages or 3-digit numbers, but I'd want to keep the default as 3-digits to ensure that the macOS usecase works out of the box.

@Luffles
Copy link
Author

Luffles commented Aug 22, 2021

@Luffles always happy to accept PRs, thanks for taking the time!

Cool, thanks for taking a look.

If this repo is also useful for Windows

I ran into Overwolf being incredibly resource intensive while trying to multitask with arena on the side -- and maybe I'm missing something with deck tracker -- but I get the almost entirely N/A value on cards using the latest repo release, after the source of the value stopped updating.

@jdb8
Copy link
Owner

jdb8 commented Oct 11, 2021

@Luffles sorry for neglecting this PR. The README updates look fine to me, although I don't have a Windows machine to test them on (but they look correct and I'm assuming you've run them on your machine).

I'm less sure about the code change though: it seems like the Windows deck tracker is fine with decimals, but Mac isn't. I'd be happy to add conditional behaviour here if you can easily detect whether we're running on Windows or Mac - as long as the default Mac behaviour is to maintain the 3-digit display.

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.

2 participants