added a 100ms sleep to the keep alive system to prevent it from having excessive cpu usage #117
Conversation
|
The keepalive system should be 15sec, 15000ms |
|
^ There is no point checking keepalive every 100ms. Since keepalive typically runs every 30 seconds, checking every 15 seconds ensures we catch all players' status even if they join right after a check, while still being far more efficient than checking every 100ms. |
| @@ -84,10 +85,11 @@ impl System for KeepAliveSystem { | |||
| if let Err(e) = state.broadcast(&packet, broadcast_opts).await { | |||
| error!("Error sending keep alive packet: {}", e); | |||
| }; | |||
|
|
|||
| // TODO, this should be configurable as some people may have bad network so the clients may end up disconnecting from the server moments before the keep alive is sent | |||
There was a problem hiding this comment.
15 seconds delay means the client gets 2 chances. The default timeout for keepalive is 30 seconds.
|
@AnonymousBit0111 another issue has occured, only a single player can join |
| // TODO Kick player | ||
| } | ||
|
|
||
| let result = state.universe.get_mut::<IncomingKeepAlivePacket>(conn_id); |
There was a problem hiding this comment.
This is where the problem happens I'm guessing.
|
|
||
| if result.is_err() { | ||
| let err = result.as_ref().err().unwrap(); | ||
| if matches!(err, ECSError::ComponentTypeNotFound) { | ||
| state | ||
| .universe | ||
| .add_component(conn_id, IncomingKeepAlivePacket { id: self.id })?; | ||
| let mut last_received_keep_alive = state.universe.get_mut(conn_id)?; | ||
| *last_received_keep_alive = self; | ||
| } else { | ||
| warn!( | ||
| "Failed to get or create <IncomingKeepAlive> component: {:?}", | ||
| err | ||
| ); | ||
| return Err(crate::errors::NetError::ECSError(result.err().unwrap())); | ||
| } | ||
| } else { | ||
| *last_keep_alive = KeepAlive::from(self.id); | ||
| let mut last_received_keep_alive: ComponentRefMut<'_, IncomingKeepAlivePacket> = | ||
| result.unwrap(); | ||
|
|
||
| *last_received_keep_alive = self; |
There was a problem hiding this comment.
Why not just do a match statement?
match result {
Ok(component) => {
// ...
},
Err(e) => {
// ...
}
}There was a problem hiding this comment.
state.universe.get_mut::<IncomingKeepAlivePacket>(conn_id)is the culprit
There was a problem hiding this comment.
If something stupid is being done, probably. I haven't read the code in too much depth, but this shouldn't be the case.
There was a problem hiding this comment.
it wasnt, I think the error system is a bit inconsistent, I expected only ComponentTypeNotFound, but also got ComponentRetrievalError, when they're both reporting what should be the same error since the IncomingKeepAlive hasn't been created yet, I've fixed this in #122
No description provided.