Skip to content

Conversation

EmmmaTech
Copy link
Contributor

@EmmmaTech EmmmaTech commented Dec 25, 2021

Summary

This pull request implements #495 by having new functions that loop through each command and set the permissions

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why
  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, typehinting, examples, ...)

@Dorukyum
Copy link
Member

Not a bad idea but it would be inconsistent since ext.commands.Cog uses the cog_check async method.

@EmmmaTech
Copy link
Contributor Author

Not a bad idea but it would be inconsistent since ext.commands.Cog uses the cog_check async method.

So should I rework the PR around that?

@EmmmaTech
Copy link
Contributor Author

Thinking about it, the way I worded the title gives the wrong impression because none of the functions I added are actually decorators. So I guess I should just change the title of the PR

@EmmmaTech EmmmaTech changed the title Make cog_check work like permissions decorator for slash commands Add functions that set permissions for all slash commands in a cog Dec 29, 2021
Dorukyum
Dorukyum previously approved these changes Dec 30, 2021
Lulalaby
Lulalaby previously approved these changes Dec 30, 2021
@EmmmaTech EmmmaTech dismissed stale reviews from Lulalaby and Dorukyum via 6978c2d December 31, 2021 20:49
@EmmmaTech
Copy link
Contributor Author

Wait how did that dismiss the reviews

@Dorukyum
Copy link
Member

By the way, how are these meant to be used?

@EmmmaTech
Copy link
Contributor Author

By the way, how are these meant to be used?

I intended for the functions to be used in cog_check, however I added 0 checks for that, so it could be used in the init function or any other function in the cog.

@Dorukyum
Copy link
Member

Dorukyum commented Jan 1, 2022

By the way, how are these meant to be used?

I intended for the functions to be used in cog_check, however I added 0 checks for that, so it could be used in the init function or any other function in the cog.

How can these be used in the cog_check? That is a method ran as a check before every command of the cog while these methods set permissions for the commands.
Cog decorators could be used for this functionality instead.

@EmmmaTech
Copy link
Contributor Author

By the way, how are these meant to be used?

I intended for the functions to be used in cog_check, however I added 0 checks for that, so it could be used in the init function or any other function in the cog.

How can these be used in the cog_check? That is a method ran as a check before every command of the cog while these methods set permissions for the commands.
Cog decorators could be used for this functionality instead.

Okay, but do you know which function these cog decorators would go in?

Plus, I have a better idea: each of the functions could still be ran in cog_check, however, it would just store an array of the permissions in the cog. Then later on, in another function, each of these permissions are loaded into every function.

@Lulalaby Lulalaby added this to the v2.0 milestone Jan 7, 2022
@Lulalaby Lulalaby marked this pull request as draft January 18, 2022 06:00
@Lulalaby
Copy link
Member

⚠️ Warning: This is a candidate for getting closed
ℹ️ Reason: Collides with perms 2.1

@EmmmaTech
Copy link
Contributor Author

⚠️ Warning: This is a candidate for getting closed

ℹ️ Reason: Collides with perms 2.1

Could I redesign it around the perms in 2.1?

@Lulalaby
Copy link
Member

No

@EmmmaTech
Copy link
Contributor Author

No

Okay. You can close it then if you'd like

@Lulalaby
Copy link
Member

We discuss it internally

@VincentRPS VincentRPS modified the milestones: v2.0, v2.1 Jan 21, 2022
@VincentRPS VincentRPS added priority: medium Medium Priority status: in progress Work in Progess labels Jan 26, 2022
@EmmmaTech
Copy link
Contributor Author

Closing because of reworks with Command Permissions.

@EmmmaTech EmmmaTech closed this Jan 29, 2022
@VincentRPS VincentRPS removed this from the v2.1 milestone Jan 30, 2022
@EmmmaTech EmmmaTech deleted the master branch August 31, 2022 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: medium Medium Priority status: in progress Work in Progess
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants