-
Notifications
You must be signed in to change notification settings - Fork 27
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
New Features #110
base: main
Are you sure you want to change the base?
New Features #110
Conversation
shashankpandey04
commented
Nov 22, 2024
- Allow admin roles to accept LOAs
- Advance LOAs
- LOA View Command
- Custom Command Embed Edit
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
requesting changes
see comments for details
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
im fine with the /loa view, /ra view stuff
but, you cant have an ephemeral paginator; so you have to pick one or the other
i found this out the hard way with the paginator being broken on /loa active for a long time so we had to scrap the ephemeral part of it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you might wanna add an exception handler to starting
, just in case people put like 21/10/2024 or smth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need to change CustomCommandModification
to CustomCommandModificationEdit
why do we need to change MessageCustomisation
to MessageCustomisationEdit
whats the benefit here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CustomCommandModification
and MessageCustomisation
doesn't directly support the Embed Customization by default. So instead of modifying existing functions I modified the UI and added the functionality to edit Embeds in different class to prevent any errors in existing custom command creation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MessageCustomisation doesnt support EmbedCustomisation because it subclasses EmbedCustomisation in the "Add Embed" function
if you want to edit embeds, you should use EmbedCustomisation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok so first of all; i like what you did with the sub_vars on the fields
however, for run_pm_command
:
- there is no retry-after header | see the actual api docs
- even if there was one, you have to do
ResetTime - CurrentTime
to get the time to wait for, instead of just waiting on theResetTime
which would cause a wait period of 11291 years if i recall correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so um... blame prc? they added this header without ever putting it on the docs and i've been saying it doesnt exist for months 💀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for whitelisted car functionality, cant you just search through the people who hold the exotic car roles instead of guild.members
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for role in exotic_roles:
role_based_users[role] = [member for member in role.members if member.guild == guild]
for role, member in role_based_users.items():```
Updated to `role.members` as this would be a better approach in this case reducing the time complexity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of a 10 minute intermediate time for process_player_logs; if you want to check if the bot has restarted since the log, you should compare log.timestamp < bot.start_time
which is used by uptime as the bot startup time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure how i feel about adding the count of moderations to each item on the shift leaderboard
i mean, thats what the google spreadsheet is for
ask erm team ig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so i just asked in erm team and they suggested we move it to a /punishment leaderboard
where its sorted by moderations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah ok good
you actually followed through on what i said
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still dont know why we need to replace CustomCommandModification
with CustomCommandModificationEdit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i mean, simplifying the regex to a kv dict { name | display | global: member }
doesnt sound like that bad of an idea in retrospect
but, two people can have the same display, name, or global name, and then it'd replace the person in the dictionary with that
just something to consider
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed 👍
Suppose you have three members:
Quprgaming
quprgaming
Bajah with a global name of Rajah
Then,
'quprgaming': [member1, member2],
'bajah': [member3],
'Rajah': [member3]
}```
Then the condition
`if player_username_lower in guild_members_dict and guild_members_dict[player_username_lower]:`
Will check if the user is in guild members and present in that name key of guild member.
This will avoid the confusion between different members with same name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good!
39be997
to
729a183
Compare
|
19f9b00
to
a40a8c9
Compare
Tried to fix the Player Join Logs getting sent again every time bot restarts. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. add more configuration though like in discord_checks they should be able to do the following:
{
"discord_checks": {
"channel": 0, // Where to send discord check results to
"should_load": bool, // Should it load them or kick them?
"should_warn": bool, // Should it give them a PM Warning?
"kick_after_infractions": 0, // How many loads (if loading is on), or warnings if its not, should the user get before they're kicked?
"message": "str", // What message should it send the user?
"mentioned_roles": [ // What roles should be mentioned in the channel?
0
1
2
3
]
}
}