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

feat: min and max constraints for slash command options #1026

Merged
merged 12 commits into from Feb 26, 2022

Conversation

FedorLap2006
Copy link
Collaborator

@jolheiser
Copy link
Contributor

I don't know that I particularly like interface{} for this. We can use int or double according to Discord docs, but I still think float64 might make more sense. Maybe with some truncating internally if it's an integer field?

@FedorLap2006
Copy link
Collaborator Author

I don't know that I particularly like interface{} for this. We can use int or double according to Discord docs, but I still think float64 might make more sense. Maybe with some truncating internally if it's an integer field?

The only downside of this approach - when value is 0 - encoding/json omits it, but shouldn't, and interface{} eliminates this problem.

@iopred
Copy link
Collaborator

iopred commented Nov 18, 2021

Using *float64 makes sense then, we've done that in a few places.

@FedorLap2006
Copy link
Collaborator Author

It's quite inconvenient for the end user tho

@FedorLap2006
Copy link
Collaborator Author

@CarsonHoffman What is your opinion on this?

@FedorLap2006
Copy link
Collaborator Author

Alright, @iopred @CarsonHoffman should be ready for review

@sentinelb51
Copy link
Contributor

sentinelb51 commented Dec 28, 2021

We've got this far:
MinValue and MaxValue as interface{} for ApplicationCommandOption

What was NOT included are the following:

Add a new enum (value 10) for ApplicationCommandOptionType
ApplicationCommandOptionNumber ApplicationCommandOptionType = 10

For func (t ApplicationCommandOptionType) String() string, add:
case ApplicationCommandOptionNumber: return "Number"

For func (o ApplicationCommandInteractionDataOption) FloatValue() float64, add:
if o.Type != ApplicationCommandOptionNumber { panic("FloatValue called on data option of type " + o.Type.String()) }

Summary:
ApplicationCommandOptionNumber should be used when you want to work with floats, otherwise ApplicationCommandOptionInteger

@FedorLap2006
Copy link
Collaborator Author

FedorLap2006 commented Feb 25, 2022

Not entirely sure about merging this, because it's definitely going to break a lot of stuff. So will hold it for a couple of days.

@Nv7-GitHub
Copy link
Contributor

NEED THIS

@FedorLap2006 FedorLap2006 added the high priority Issue or PR with high priority of merge label Feb 26, 2022
@FedorLap2006 FedorLap2006 merged commit a4d6947 into bwmarrin:master Feb 26, 2022
@FedorLap2006 FedorLap2006 added the breaking changes Contains breaking changes. Should be reflected in the changelog label Feb 26, 2022
@FedorLap2006 FedorLap2006 added this to the v0.24.0 milestone Feb 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking changes Contains breaking changes. Should be reflected in the changelog high priority Issue or PR with high priority of merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants