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 address map for better client index looking up #120

Merged
merged 4 commits into from
Jan 9, 2024
Merged

Add address map for better client index looking up #120

merged 4 commits into from
Jan 9, 2024

Conversation

kienhg96
Copy link
Contributor

@kienhg96 kienhg96 commented Jan 6, 2024

No description provided.

@gafferongames
Copy link
Contributor

Some warnings need to be fixed, see above.

@gafferongames
Copy link
Contributor

First issue. The map returns 0 if the client index isn't found, but client index of 0 is a valid index.

Please change that to return -1 if client index is found

@gafferongames
Copy link
Contributor

gafferongames commented Jan 6, 2024

Please change

#define NETCODE_ADDRESS_MAP_BUCKETS 256

To be some function of the max clients, because as the max clients increases for the server, you will need more buckets.

(Maybe it should just be #define NETCODE_ADDRESS_MAP_BUCKETS NETCODE_MAX_CLIENTS? Then the size is n^2 on max clients... which is fine, and it handles worst case)

@gafferongames
Copy link
Contributor

Please add a unit test for the map

@kienhg96
Copy link
Contributor Author

kienhg96 commented Jan 8, 2024

Ok, I'll check them

@kienhg96
Copy link
Contributor Author

kienhg96 commented Jan 8, 2024

I fixed the issues above, please have a check

Copy link
Contributor

@gafferongames gafferongames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add functions netcode_map_create and netcode_map_destroy that use the allocator, the same way these methods are used for other types. -- thanks!

@kienhg96
Copy link
Contributor Author

kienhg96 commented Jan 9, 2024

I have added the create and destroy functions using allocator.
I think you should wrap the allocator_context, allocate_function and free_function to a new struct called allocator.
It could be much more convenient.

@gafferongames gafferongames merged commit b855e52 into mas-bandwidth:main Jan 9, 2024
6 checks passed
@gafferongames
Copy link
Contributor

Merged. Thanks!

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