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

groupId is int whereas friendId is uint32_t, reason? #606

Closed
sudden6 opened this issue Oct 26, 2017 · 6 comments · Fixed by #800
Closed

groupId is int whereas friendId is uint32_t, reason? #606

sudden6 opened this issue Oct 26, 2017 · 6 comments · Fixed by #800
Assignees
Labels
P3 Low priority refactor Refactoring production code, eg. renaming a variable, not affecting semantics toxav Audio/video
Milestone

Comments

@sudden6
Copy link

sudden6 commented Oct 26, 2017

In https://github.com/TokTok/c-toxcore/blob/master/toxav/toxav.api.h#L595 the groupId is an int whereas throughout the rest of the API friendId's are uint32_t like in https://github.com/TokTok/c-toxcore/blob/master/toxcore/tox.h#L1293

I don't think it's of much relevance right now, but is this a mistake? Or am I overlooking special meanings for groupId < 0?

@sudden6
Copy link
Author

sudden6 commented Jan 5, 2018

@robinlinden would it make sense to add this to the 0.2.0 milestone or will it conflict with #295?

@robinlinden
Copy link
Member

Don't worry about #295. If it conflicts I'll change it in that PR. We'll put this in 0.2.0.

@robinlinden robinlinden added this to the v0.2.0 milestone Jan 5, 2018
@iphydf iphydf modified the milestones: v0.2.0-RC1, v0.2.0 Jan 14, 2018
@iphydf
Copy link
Member

iphydf commented Feb 18, 2018

This is legacy reasons. All numbers used to be signed ints, but we changed it a few years ago in the api redesign. These old groupav functions weren't part of the redesign because they are really strange (they take callbacks but aren't event-like functions).

@iphydf iphydf modified the milestones: v0.2.0, v0.3.0 Feb 19, 2018
@iphydf
Copy link
Member

iphydf commented Feb 19, 2018

@sudden6 can you make a PR for this?

@sudden6
Copy link
Author

sudden6 commented Feb 19, 2018

to change the type to uint32_t?

@iphydf
Copy link
Member

iphydf commented Feb 19, 2018

Yes.

sudden6 added a commit to sudden6/c-toxcore that referenced this issue Feb 19, 2018
sudden6 added a commit to sudden6/c-toxcore that referenced this issue Feb 19, 2018
sudden6 added a commit to sudden6/c-toxcore that referenced this issue Feb 19, 2018
sudden6 added a commit to sudden6/c-toxcore that referenced this issue Feb 20, 2018
@iphydf iphydf added refactor Refactoring production code, eg. renaming a variable, not affecting semantics and removed code cleanup labels May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 Low priority refactor Refactoring production code, eg. renaming a variable, not affecting semantics toxav Audio/video
Development

Successfully merging a pull request may close this issue.

4 participants