Androz2091 / discord-giveaways

🎉 Complete framework to facilitate the creation of giveaways using discord.js
https://discord-giveaways.js.org
MIT License
334 stars 127 forks source link

Drop ends with 0 winners if participant triggers exemptMembers #429

Closed GeistFighter closed 2 years ago

GeistFighter commented 2 years ago

Describe the bug

In fact, the problem is that when a person who is normally part of the exclusions reacts, he is counted in the drop as a person who reacts, but is not counted in the fact that he can win, so if there is only one winner on the drop, if the 1st to add the reaction is a person who is part of the exclusions, the drop stops because it takes him into account, but indicates the "NoWinner" because he is excluded, so it would be necessary that the exclusions are also applied to the counter of the Drop reaction.

On the other hand, if there are several winners in the drop, if among the winners there are people excluded, the number of winners will not be equal to the number of winners set.

Error

The error is that it sets the "noWinner" variable directly, so the Giveaway stops, the error is the "noWinner", i don't have a real error, I don't know if that is clear?

To Reproduce

Steps to reproduce the behavior: Example 1:

Example 2:

Expected behavior
That users excluded from the rewards do not count against the Giveaways Drop counter.

Screenshots
I can make a video if necessary.

Additional context

Thanks you !

GeistFighter commented 2 years ago

A video as an example: 😄 https://user-images.githubusercontent.com/42804270/151698488-78676ea6-99a8-4ddf-a867-6d169a5d6147.mp4

imranbarbhuiya commented 2 years ago

213 this can solve your issue

GeistFighter commented 2 years ago

Hi, yes indeed, putting an option that checks the number of winners required would solve the problem, but it would be like adding more steps, in this case, it would be easier to add the check of exemptMembers directly to the addition of the reaction on the Giveaway Drop ?

Nico105 commented 2 years ago

Hi, yes indeed, putting an option that checks the number of winners required would solve the problem, but it would be like adding more steps, in this case, it would be easier to add the check of exemptMembers directly to the addition of the reaction on the Giveaway Drop ?

exemptMembers was meant as a barrier directly before ending, e.g. when the bot was offline and users reacted.

What you mean is technically the reactionAdded event. But it won't really be "fast" enough (isn't awaited) if there is only 1 winner/when 2/3 reacted and the 3rd one reacts

Nico105 commented 2 years ago

This will get solved when I somewhen have the time to do #107 in the form of a endRequirement (name not final I guess) function. I started working on it some time ago but delayed it for now.

213 is limited in it's use case. So ^^^ would give more options.

imranbarbhuiya commented 2 years ago

I believe this can be fixed by adding giveaway.checkUserEntry in this filter

https://github.com/Androz2091/discord-giveaways/blob/08b0a4152571bb57be314df98ce34562d43a5e1f/src/Manager.js#L284

Nico105 commented 2 years ago

what should be done about the case, that a drop "never" reaches it's winnerCount. cause that could potentially lead to dead drops littering the system. should there be a data deletion after e.g. 1 week, when a drop hasn't ended yet?

imranbarbhuiya commented 2 years ago

should there be a data deletion after e.g. 1 week, when a drop hasn't ended yet?

Yeah it'll be cool

Nico105 commented 2 years ago

v5.2.0 was released. (Should fix the issue, if I didn't forget smth XD)