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

Packet tracking stats for 3.2 #38306

Closed
wants to merge 4 commits into from

Conversation

chiguireitor
Copy link

This PR adds additional functionality to the ENet module to enabled packet statistics tracking (packets sent/received, bytes sent/received) useful for debugging multiplayer games.

@chiguireitor chiguireitor requested a review from a team as a code owner April 28, 2020 17:19
@Calinou Calinou added this to the 3.2 milestone Apr 28, 2020
@Faless
Copy link
Collaborator

Faless commented May 1, 2020

I don't like the idea of making this ENetPeer-specific. Since we need to track this manually, it would be best to have it in MultiplayerAPI.

@chiguireitor
Copy link
Author

Could be moved upper towards the NetworkPeer? I feel like it's very implementation specific (i.e. there are some client/server configuration messages that aren't taken into account on upper layers) and having it on the lowest level allows for real deep optimizations, but that's just my point of view on my specific need. The code isn't really that complex so moving it to somewhere where it makes more sense/is more broad couldn't harm.

@Faless
Copy link
Collaborator

Faless commented May 1, 2020

Well, it could be peer dependent, but not in the way it's currently implemented. As an example, bytes_sent does not reflect the actual number of payload sent in the network when e.g. compression is enabled, or when the server broadcast a message to all peers,

@chiguireitor
Copy link
Author

Gotcha, indeed, i wanted to be able to check how much data is being sent and received on the client side mainly to optimize that side, but indeed this could be an issue if the server isn't calculating correctly the amounts. Will do a fix when i finish my other current module.

@yuraj11
Copy link

yuraj11 commented Dec 6, 2020

@chiguireitor Hi, do you still work on this? It would be useful addition.

@Calinou
Copy link
Member

Calinou commented Dec 6, 2020

@yuraj11 Doesn't #31870 supersede this? The main difference is that #31870 is editor-only. I'm not sure how much of an issue this is in practice.

@chiguireitor
Copy link
Author

@Calinou although #31870 looks like this, this is for tracking on-the-go, could be implemented for some sort of packed quota for limited networks and so on.

@chiguireitor
Copy link
Author

@yuraj11 i'm using this on a personal fork of mine for development purposes.... doing an mmorpg needs closely monitoring of the usage on mobile..... it works ok for me, lemme know if you use it

@yuraj11
Copy link

yuraj11 commented Dec 9, 2020

@Calinou Well it would be also useful to be available as API for in game debug purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants