Send key updates to the server immediately (to fix presses shorter than 100ms being missed)

[JensDoesLua]

• Goal of the PR
  Fast/short key presses should always result in a change to the result of player.get_player_control() - for at least one game step.

• How does the PR work?
  In client.cpp, sendPlayerPos() is only called if counter >= m_recommended_send_interval, which is fine for sending repeated packets to the server. Checking if any pressed keys changed is already done in sendPlayerPos().
  This PR moves the counter >= m_recommended_send_interval check to sendPlayerPos(), into the if (identical) branch. This way, a package with new key state is directly send when a key is pressed.
  Without this change, a short key press is lost if the m_recommended_send_interval (by default 100ms) just started, a key is pressed and released before the timer ends. After 100ms, the position and pressed keys are sent again, but the already released key press is lost.

• Does it resolve any reported issue?
  This one.

• Does this relate to a goal in the roadmap?
  No

To do

This PR is Ready for Review.

How to test

Example how to reproduce in singleplayer mode, short press aux1 or jump (or other keys):

local previous_control = {}
core.register_globalstep(function()
	for _, player in pairs(core.get_connected_players()) do
		local name = player:get_player_name()
		local control = player:get_player_control()
		for key, value in pairs(control) do
			if previous_control[key] ~= value then
				print(name .. ":" .. key .. ":" .. tostring(value))
			end
		end
		previous_control = control
		return
	end
end)

[appgurueu]

This looks like a step in the right direction; it should alleviate the problem, especially in singleplayer.

I'm not sure whether the problem can be properly fixed this way, because we only send "is pressed" state as opposed to "was pressed / released" events to the server. So if you press a key and subsequently release it, what's likely to happen is that the control down state gets overwritten by the second packet, if both happen within the same server step. So to mods, the event is still lost. Now you could have some server logic that diffs packets arriving in the same step and synthesizes events from that, but really we should think about different packets for sending an ordered stream of control events: The set pos packet is more of a hack and not nice to extend for custom controls.

[sfan5]

we should think about different packets for sending an ordered stream of control events: The set pos packet is more of a hack and not nice to extend for custom controls.

that's more bandaid on something that can't ever work properly. the solution is SSCSM.

[JensDoesLua]

It might not be perfect, but it is at least better that the current state. And it actually works, no key presses are lost for me anymore.

[sfan5]

Did you try it outside of singleplayer? on a multiplayer server?
The default step is 90ms, which is plenty of time to press and release a key.

It's also worth noting that the server will by definition never see key pressed for shorter than the step size.

[JensDoesLua]

on a multiplayer server?
Yes, it is not always working, but at least better. I can try to fix the multiple packages per gamestep problem by adding the "was pressed" logic to the server receive side?

by definition
where can i find this definition?

[sfan5]

where can i find this definition?

I think you misunderstand. This isn't in code, it's just logical: If you check something every N milliseconds, you can miss changes that last for less than N milliseconds.