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

Add MessageBuilder.Limits object #959

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Galarzaa90
Copy link
Contributor

Similar to EmbedBuilder.Limits, added one for MessageBuilder, altough this one is an interface, so I'm not sure if there are any side effects to this?

Also, I'm missing one for files/attachments, but then when I saw that they are two separate fields, I wasn't sure how to do it.

@lukellmann
Copy link
Member

Similar to EmbedBuilder.Limits, added one for MessageBuilder, altough this one is an interface, so I'm not sure if there are any side effects to this?

No, that shouldn't be an issue.

@lukellmann
Copy link
Member

I'm curious, what do you use these values for? Hardcoding them into Kord might be an issue if Discord increases them at some point.

@Galarzaa90
Copy link
Contributor Author

I'm curious, what do you use these values for? Hardcoding them into Kord might be an issue if Discord increases them at some point.

These ones in particular, I only have one instance where I use them, because for the most part I use embeds. But same as EmbedBuilder.Limits, I use them to limit information to not exceed the limits.

I know that hardcoding might be an issue, but either you hardcode them here or you hardcode them youself in your implementation, and since we already have EmbedBuilder.Limits, thought it would make sense to have these as well for completeness/consistency. Might come handy for average users.

If the value is updated, I would have to update my hardcoded values anyway. If I'm using the constants from here, I would temporarily replace them with my own constants, while the value here is updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants