-
-
Notifications
You must be signed in to change notification settings - Fork 127
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
✨ Button support #482
base: develop
Are you sure you want to change the base?
✨ Button support #482
Conversation
This looks absolutely amazing. Will definitely test this 👍🏻 |
@Androz2091, could you maybe take a look at the questions stated above |
Co-authored-by: Andromeda <[email protected]>
Hi, is it fully operational now or not yet ? |
Think it should add also others entry as component array, so user can add some info buttons they need like link to some page for giveaways, or similar buttons that they find useful? |
Is this PR complete? |
Nice idea, but if a different database than json file is used to store the giveaways, it doesn't work yet |
will be fixed |
not yet, last changes have to be made |
Great, thank you for the hard work you are putting into it, will it show the number of entries on the button? |
it will be possible to do that |
Have you finished it yet? |
I have made modifications to the discord-giveaways forked by @Nico105 and have been successful in creating my own working button giveaways package. However, I am unsure of the potential effects of merging my changes back into the original package i.e |
Yeah it is, but consider giving credits to the original one. |
Is this ready for use yet? |
Changes
Adds an option in the default manager or
start()
options to provide a join and optionally a leave button. (if only join is provided, it will also act as leave, if clicked twice)Usage test
Questions
What should happen withendedGiveawayReactionAdded
? It was added because some people want to prevent users from joining in case of a reroll. This one would a little bit be the odd one out since it is reaction only, but emitting it for buttons too doesn't really make sense cause of its specific use case.LMK
If there should be a 3rd option which takes an array for any other buttons that should get added to the message and should get checked/filled in.Tho I have to say thats not a high priority cause that could get added later anyway.
LMK
The buttons are currently calledjoin
andleave
.I leave it up for debate if they should be called differently, like instead of join --> enter or smth.
LMK
I currently just named the button eventsgiveawayJoined
andgiveawayLeft
cause the reaction ones wouldn't really fit.But so I would like to ask if we should rename the reaction ones to these names too, cause the only event argument difference is
messageReaction
vsButtonInteraction
, which can be detected easily viaif (giveaway.buttons)
.What i also leave open is the event names themselves, it doesn't have to be those I used on a whim.
LMK
Important: Currently the interactions are not deferred, so they just fail.I did this so the dev can decide in the events on what to do.
But lmk if we should do something by default
Stuff to do
Proper priority handling of reaction vs button.Cause it is possible to provide both as the default manager option and then you could set one or the other as
null
in thestart()
options to indicate which default should be used.But also atm i left it open what to do when a button is provided but it is invalid. Should it automatically default to the reaction and start or reject, or whatever. Vice versa.
Note: I left some time pass by, so Im not remembering the current behavior/priority stuff completely = the paragraph above is probably worded wrong/describes it falsly but Im too lazy atm to check it again. Still this has to get properly handled.
Priority (if I'm not mistaken):
start()-reaction > start()-buttons > default-reaction > default-buttons
Status
Tested: