-
Notifications
You must be signed in to change notification settings - Fork 4
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
20+ reaction support #86
base: main
Are you sure you want to change the base?
Conversation
This is a very promising start. I simplified I think it would be more beneficial to try to add as many additional roles to the main message as possible. Normally the main message can fit quite a few additional roles before needing extra space, with this current setup adding even a single additional role will result in an extra message. I think the basic idea here is good, but instead of always splitting the Additional group into a separate message, we'd have to count the number of roles/reactions and create a new message only if that number exceeds 20. Notes about my changes: I commented out some functionality related to the message handling so that I could make the bot display the embeds generated by |
This is much better than my start. I didn't really have time to improve the code myself. BTW I might not have too much time until next week, because of another project. So if you want to continue this, feel free to do so. I don't have any local changes atm. I will look at any changes though so I can continue or help if necessary |
I re-added the emojis - they dont have a function yet again but theyre added correctly. And added a TODO for the updateReactions function because I scrapped the more efficient part for now. Needs to be implemented again BTW I havent got the Linter plugin yet, because it doesnt work yet. Ill fix the errors in the next commit |
This pull request needs in-depth testing. I haven't found anything so far. But there might be a few things that I haven't seen. There are 2 TODOs in messageFunctions.py. The one for syncMessages is obsolete but I forgot to remove the comment. Just saw that. The other one needs improvement but works, |
c6b148d
to
ce5efad
Compare
This feature is slowly starting to take shape. Creating and sorting multi-message events works now. Some things to still implement:
|
Hey, as you've probably noticed, currently I dont have time to improve this PR. I have to put it on hold until decembre (probably). I would prefer if you would take care of this, if you have time and motivation, since this code has to be checked on every other change made to the master. |
Yeah, no worries. There's not really a rush with feature, I've been improving it every now and then when I've had the time. |
8fb305e
to
bc9e541
Compare
The feature is still very much in progress. This commit isn't meant for merging as-is, see various TODO markers. Doesn't reorder or synchronise messages at all yet, just reposts the event message every time
All functions that handle multiple messages have been renamed to use plural in the function name. Additionally, there is a singular getEventMessage that's used by some functions, including the plural getEventMessages
Embeds are quick to update so they're updated first. Then the slow process of adding reactions begins.
Create removerole, removemainrole, deprecate removereaction
For example, when the 21st reaction gets removed.
bc9e541
to
5feee2f
Compare
2dedfd6
to
6a5fb32
Compare
2931dc1
to
2b67941
Compare
closes #65 one day
Done:
messageID is a list now
createEmbed returns a list of embeds
reactions are added to the embeds
adding roles
updating the event
signups / reactions work
command functions
dead code parts of messageID
Saving/Loading/snycing
on_raw_reaction_add
To Check:
make linter happy (: