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

✨ requiredParticipationCount, requiredWinnercount, noValidEndingInterval #213

Closed
wants to merge 3 commits into from

Conversation

Nico105
Copy link
Collaborator

@Nico105 Nico105 commented Feb 10, 2021

Thoughts?
As far as I tested it, it works.
Better variable names are of course accepted.
Better implementation of course accepted.

End Command

Currently requiredParticipationCount will only be trigger by auto-ending, an end command with the end() function ignores it.
But requiredWinnercount will always be triggered (= even with an end command)

2nd sentence because otherwise we would have to move requiredWinnercount handling into the roll() function and then a reroll command would also be affected by requiredWinnercount and I don't think that that is really wanted. The only thing that we could do if we should do it like that (lmk your opinion) is adding an argument to the end and reroll function where people can set useRequiredWinnercount to true if they like but otherwise nothing will be affected.

@Nico105 Nico105 added the enhancement New feature or request label Feb 10, 2021
Copy link
Owner

@Androz2091 Androz2091 left a comment

Choose a reason for hiding this comment

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

So, first of all thank you so much for your work 😁
I think that we should remove noValidEndingInterval. Yes. I'm sorry. 😬
I will do the changes, don't worry.

@Nico105
Copy link
Collaborator Author

Nico105 commented Feb 23, 2021

As a thought, we could scrap requiredParticipationCount and requiredWinnercount and just use the requirements option that you already added to the giveaway data
EDIT: lol, that line was from #93 and it was the former name of extraData

===

endRequirement: (giveaway) => giveaway.message.reactions.cache.get(giveaway.reaction).count - 1 >= 100 && giveaway.winnerIDs.length === giveaway.winnerCount

#107

But in order to use giveaway.winnerIDs.length we would need to check endRequirements in the end() function, but so that would also affect a manually executed "end" command = we would need to do something about that (maybe force: true option for end())

@Nico105 Nico105 mentioned this pull request Jul 21, 2021
@imranbarbhuiya
Copy link
Contributor

Why it's still WIP?

@Nico105
Copy link
Collaborator Author

Nico105 commented Dec 19, 2021

Why it's still WIP?

Waiting on Androz to tell the direction where this should go.

@Nico105
Copy link
Collaborator Author

Nico105 commented Jul 24, 2022

The future approach should be like mentioned in: #213 (comment)
I already started working on it a little, but it will take its time

@Nico105 Nico105 closed this Jul 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants