NetCordDev / NetCord

The modern and fully customizable C# Discord library.
https://netcord.dev
MIT License
65 stars 10 forks source link

Feat: Adding poll support #18

Closed budgetdevv closed 3 months ago

budgetdevv commented 6 months ago

Add poll support to NetCord.

budgetdevv commented 6 months ago

It is worth noting that this is still WIP. Made a PR just so progress can be tracked

KubaZ2 commented 6 months ago

NetCord doesn't use required keyword anywhere currently. It would be nice not to break the rule. I would make constructors with required/common values instead. Also I don't think making a poll base class is a good idea. Mixing send and receive properties is not what NetCord does currently.

budgetdevv commented 6 months ago

Okay, will try to refactor with what you said in mind

budgetdevv commented 6 months ago

I also realized that I have to make IJsonModel wrappers for non-properties ( We discussed this in Discord )

budgetdevv commented 6 months ago

Finally checks are passing

budgetdevv commented 6 months ago

Alright, I will make further changes with that in mind

KubaZ2 commented 6 months ago

Why is the AnwerId an uint now? Isn't it better to make it a signed integer? I personally would use unsigned types everywhere, but since .NET BCL doesn't use them (probably because they are not CLS compliant), I don't think properties like that should be unsigned unless there is some good reason for that. Also I don't think the AnswerId can be grater than or equal to the Count, but the Count is an int.

budgetdevv commented 6 months ago

Because count can never be negative, also we use ulong for IDs

budgetdevv commented 6 months ago

Which count are you referring to btw?

KubaZ2 commented 6 months ago

MessagePollAnswerCount.Count

budgetdevv commented 6 months ago

Yeah because collections use int for count

budgetdevv commented 6 months ago

I do not think this is a concern however, NetCord is unlikely to be ported to another language. But if int is used throughout the project for count, then I should probably make it an int for the sake of consistency

KubaZ2 commented 6 months ago

Because count can never be negative, also we use ulong for IDs

Snowflakes are documented as e.g. a uint64 and using the unsigned type makes the lib work longer. But tbh changing it to a long wouldn't be a bad idea.

KubaZ2 commented 6 months ago

I do not think this is a concern however, NetCord is unlikely to be ported to another language. But if int is used throughout the project for count, then I should probably make it an int for the sake of consistency

NetCord is not CLS compliant so you are probably correct. Also I didn't mean I do care about the CLS compliance.

budgetdevv commented 6 months ago

So should I change them to unsigned int instead?

KubaZ2 commented 6 months ago

I would use signed integers.

budgetdevv commented 6 months ago

I realized that I'm returning the wrong type for .GetPollAnswerVoters(), will fix it

budgetdevv commented 6 months ago

Can confirm that sending a poll works now

budgetdevv commented 6 months ago

However, it seems like after running ExpirePollAsync() ( Which works ), MessagePollResults.Answers is still an empty array, with IsFinalized set to false

KubaZ2 commented 3 months ago

Thanks!

budgetdevv commented 3 months ago

You're welcome for nothing, sorry I haven't had time to work on it ( School sucks )