Skip to content
This repository was archived by the owner on Oct 10, 2024. It is now read-only.

Conversation

@cddjr
Copy link
Contributor

@cddjr cddjr commented Mar 26, 2022

closes #260

@cddjr cddjr force-pushed the feature/reveal_votes branch 2 times, most recently from efe92dc to 8cfacfa Compare March 26, 2022 12:49
@cddjr

This comment was marked as outdated.

@v0idp
Copy link
Member

v0idp commented Mar 26, 2022

I see you took that function I mentioned in the issue. Is this finished or still WIP ?

@cddjr
Copy link
Contributor Author

cddjr commented Mar 27, 2022

I see you took that function I mentioned in the issue. Is this finished or still WIP ?

I have finished.

@cddjr cddjr changed the title [WIP] #260: Reveal Votes #260: Reveal Votes Mar 27, 2022
Copy link
Contributor

@DarkPenguin24 DarkPenguin24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one issue with this logic:
If someone votes while RevealVotes is disabled and u enable it at a later point u won't see the votes that were made already and they will also not appear at the end of the meeting which leads into false behaviour (e.g. only 2 people voting on person A which gets ejected while 4 people voted on person B).

@v0idp
Copy link
Member

v0idp commented Mar 27, 2022

There is one issue with this logic: If someone votes while RevealVotes is disabled and u enable it at a later point u won't see the votes that were made already and they will also not appear at the end of the meeting which leads into false behaviour (e.g. only 2 people voting on person A which gets ejected while 4 people voted on person B).

maybe adding a second check on the hooked blipvote function and if that person didn't vote yet execute the original function

@DarkPenguin24
Copy link
Contributor

this will fix it for "end of meeting". would prefer to fix it in a way it is always visible or not (depending on the state of RevealVotes. Like it is for the roles.

@v0idp
Copy link
Member

v0idp commented Mar 27, 2022

wouldn't that mean that if it's disabled it wouldn't show at all anymore ? that's what I get from what you're saying. or do you mean also remove the previously drawn ones ?

@puscher
Copy link

puscher commented Mar 27, 2022

Ideally:
Enabled -> show all votes that were made and all votes that will come in
Disabled -> show nothing at all (remove already drawn icons)

Same behaviour as we have for Roles:
Enabled -> shown
Disabled -> not shown at all (except of Red if you are imposter)

@cddjr cddjr changed the title #260: Reveal Votes [WIP] #260: Reveal Votes Mar 28, 2022
@cddjr cddjr force-pushed the feature/reveal_votes branch from df5f157 to 7d75406 Compare March 29, 2022 07:12
@cddjr cddjr changed the title [WIP] #260: Reveal Votes #260: Reveal Votes Mar 29, 2022
@cddjr cddjr requested a review from DarkPenguin24 March 29, 2022 07:13
@cddjr
Copy link
Contributor Author

cddjr commented Mar 29, 2022

Ideally: Enabled -> show all votes that were made and all votes that will come in Disabled -> show nothing at all (remove already drawn icons)

Same behaviour as we have for Roles: Enabled -> shown Disabled -> not shown at all (except of Red if you are imposter)

I have changed the code, please review.

@cddjr cddjr force-pushed the feature/reveal_votes branch from 7d75406 to f5cbc12 Compare March 29, 2022 15:00
@DarkPenguin24 DarkPenguin24 merged commit b99ea4e into BitCrackers:main Mar 30, 2022
@cddjr cddjr deleted the feature/reveal_votes branch March 31, 2022 02:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Live vote count on voting screen

4 participants