-
Notifications
You must be signed in to change notification settings - Fork 123
Add on-board counters for recording progress toward goal #141
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
base: main
Are you sure you want to change the base?
Conversation
This reverts commit 48a2959.
|
Neat, thanks for having a go a this :) I agree with the approach of having the counters as in-browser state, I don’t think this needs to be synced and I’d rather avoid the resulting complexity and data size. If the lack of persistence is a problem for people, I’d prefer to use browser local storage. |
|
My only feedback from a design/ux perspective is to consider reducing the space between the numbers and the buttons. With them at the edge of the square like that a user might misclick a button in a neighboring square which could get annoying. Counters reaching triple digits will be rare and anything more than that would be very surprising so I think having the buttons just outside the width of "999" (or whatever number is the widest) might be good. This and the images PR are looking really awesome :) |
|
Nice work on both parts of this. I think the counters a great but I do think that having them in sessionStorage or localStorage would be a nice feature in case a refresh is needed for whatever reason. I also wonder if maybe a flag for whether counters applicable on the entries would be nice. For instance "Kill a Black Hinox" doesn't need a counter so why clutter the screen with it. I did a review of the code also and it looks nice and clean. |
|
I think the implementation as-is is a good minimum viable product to send out. Local storage might be nice to have in the future, but stars don't have that today either and people make do. I'm not sure that I'm on board with only showing the counters for certain goals. I like that this approach doesn't require any changes to the goal list (or heuristics to detect whether something needs a counter). I think it's also more flexible in case someone wants to use the counters in an unconventional way. If this approach feels too visually cluttered right now, then I think it'd be better to address that directly, e.g. by only showing the |
|
As far as a lighter visual treatment, we could narrow it down to just showing the |
|
@kbuzsaki - just want to make sure you're not waiting on me for anything in these 2 PRs. I'm unfamiliar with your policies on merging, comment period, etc. Please let me know if there's anything else you'd like from me, and if not then what I should expect as far as merging. Thanks again. |
|
@rpierce99 thanks for checking in and thanks for your patience :) |
|
No worries, hope the life and work things haven't been bad things! Appreciate the work you do for the community. |
|
Hi rpierce, were you hoping to have one or both of these PRs merged in time for use with Bingothon? I was just reminded that that's coming up soon. Trying to balance the benefit gained from deploying the features beforehand versus the risk of added instability that may not have been debugged yet. |
|
@kbuzsaki thanks for asking, I don't personally run bingo and am not associated with bingothon, but I posted the question to the BOTW discord to let them know the question was asked. I expect we'll see one or more comments here with an answer, but I don't personally have an opinion either way. |
I am not the main dev when it comes to Bingosync features so take this with some grain of salt, however I am slightly scared of this potentially breaking some stuff in our system, depending on how this is implemented, especially when it comes to images. If image goals still have an alternate text attribute then it should probably be fine. |
I think you said bingosync and meant bingothon? Either way you can see the changes in #140 here |
|
I just did a normal Korok blackout on stream yesterday, and keeping count of everything I had done was the most troublesome part of the experience. I pulled up a notepad to try and keep track of the types and regions I had done, but an in-browser counter would be a much more elegant and less time-consuming solution. |
|
There is now an instance of bingosync using this branch live at https://bingosync.bingothon.com, if anyone wants to test it out and give feedback |

This pull request contains changes that are also in #140 since it builds on those changes, hopefully once that is merged this PR will be narrower in scope to just the counter part.
I'm by no means a designer, but hopefully this isn't too terrible looking

When counters are disabled the board looks normal, when counters are enabled they take up the lower 20% of each space and refit the text to accommodate.
The counters do not persist and are not shared with other players, they are an in-browser state only and would be lost after a refresh (similar to stars).
Thanks!