NetCord icon indicating copy to clipboard operation
NetCord copied to clipboard

Feat: Adding poll support

Open budgetdevv opened this issue 10 months ago • 19 comments

Add poll support to NetCord.

budgetdevv avatar Apr 17 '24 10:04 budgetdevv

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

budgetdevv avatar Apr 17 '24 10:04 budgetdevv

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.

KubaZ2 avatar Apr 17 '24 13:04 KubaZ2

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

budgetdevv avatar Apr 17 '24 13:04 budgetdevv

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

budgetdevv avatar Apr 17 '24 13:04 budgetdevv

Finally checks are passing

budgetdevv avatar Apr 17 '24 17:04 budgetdevv

Alright, I will make further changes with that in mind

budgetdevv avatar Apr 18 '24 08:04 budgetdevv

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.

KubaZ2 avatar Apr 19 '24 11:04 KubaZ2

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

budgetdevv avatar Apr 19 '24 11:04 budgetdevv

Which count are you referring to btw?

budgetdevv avatar Apr 19 '24 11:04 budgetdevv

MessagePollAnswerCount.Count

KubaZ2 avatar Apr 19 '24 11:04 KubaZ2

Yeah because collections use int for count

budgetdevv avatar Apr 19 '24 11:04 budgetdevv

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

budgetdevv avatar Apr 19 '24 11:04 budgetdevv

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 avatar Apr 19 '24 11:04 KubaZ2

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.

KubaZ2 avatar Apr 19 '24 11:04 KubaZ2

So should I change them to unsigned int instead?

budgetdevv avatar Apr 19 '24 11:04 budgetdevv

I would use signed integers.

KubaZ2 avatar Apr 19 '24 11:04 KubaZ2

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

budgetdevv avatar Apr 19 '24 16:04 budgetdevv

Can confirm that sending a poll works now

budgetdevv avatar Apr 19 '24 18:04 budgetdevv

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

budgetdevv avatar Apr 19 '24 18:04 budgetdevv

Thanks!

KubaZ2 avatar Jul 09 '24 08:07 KubaZ2

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

budgetdevv avatar Jul 16 '24 18:07 budgetdevv