Input: Refactor +use handling to fix continuous use#1821
Conversation
|
skimmed through the changes, are you sure these won't reintroduce any of the bugs fixed by #1792? |
|
Pretty sure, as all the changes that i removed were mainly introduced as workarounds for the broken use system. |
|
@TimGoll would you mind taking a look at my code here whenever you got time? It still contains debug stuff, so don't to a picky review but rather checkout the code locally and try some interactions with entities. I intent to get this ready on the weekend, so we can fully review and merge this and test it with more people on the main branch. |
I will try to take a look at it tomorrow, but I'm unsure if I can meaningful debug it without testing it. And not sure if I got time for this week |
|
Jeah no worries, would be nice to maybe catch stuff i am missing with this, otherwise we can test this extensively on main / the beta workshop item when this is fine code-wise. |
|
Sorry, I did not have time yet. I hope I have some spare time friday evening |
1ec7c16 to
fbd9fba
Compare
We refactored the "use key handling" to correctly work with entities that use the continuous use functionality, like the health station. Why? Previous efforts to consolidate "use key handling" led to a regression where continuous use did not work as intended anymore when directly looking at such an entity. Though, it still worked when using the use key in the proximity of the entity without directly looking at it or running into it with the use key held down, because the server then handled the continuous use by itself. This is behavior that the client cannot "block", so we needed to refactor this code to use the server-side continuous use handling as its only source of truth again. The root cause for this issue was that the client suppressed further use handling after sending the net message to the server, which caused the server-side `KeyPressed` check to return false in the timer. Simply changing this would not be sufficient, as we need still to suppress this handling for the other use-cases, like weapon pickup or remote use. Changes: We mainly refactored the server-side "use key handling" and client-side key suppression to fix this issue and simplify the code. Also, the continous use execution frequency is now every tick again, instead of every 0.1 seconds. The actual continuous execution is now handled by solely by the server again. The `TTT2PlayerUseEntity` net message code was refactored to only handle its special cases like spectator use, remote use and weapon pickup. We send this message from within the bind handler on the client if applicable. This code is still necessary, as these are cases where the normal use handling is not triggered by the server due to the items being out of reach or may not be correctly prioritized e.g. the wrong weapon being picked up instead of the intended looked at entity. The corpse search and old `UseOverride` handling was moved back to the original TTT way of handling it, by utilizing the `KeyRelease` hook. We did not modify the code here and merely copied over the original code again. Removed `ENT.CanUseKey` from placeable entities as it is not needed anymore with the new handling. Removed now obsolete `IsSpecialUsableEntity` and `IsUsableEntity` functions. Refactored `ClientUse` function to be client only and always block further use handling. This function is generally useful for client-side only use key handling, e.g., for UI interactions. E.g. the C4 uses this to open the C4 menu when the use key is pressed while looking at it.
fbd9fba to
f25aca2
Compare
|
Okay @TimGoll , i finished this one up and fixed any remaining bugs i could find when testing this locally. Also added a description and changelog! The change is actually rather simple now, but understanding the code parts can be hard, if you have any questions ask me and i'll happily help out! |
TimGoll
left a comment
There was a problem hiding this comment.
Looks good to me - although I'd personally keep that one (now unused) function in the code
|
Sorry for the late review. I really struggle with my time currently. I need 40hour days to manage all my todos. The review might look like I just skimmed through it, but I did take my time. It does look really good, especially with all these helpful comments! Additionally, this PR's description is really helpful. I think it should work. Maybe @wgetJane can confirm this, as they spent a lot of time on this as well. Thank you! |
We refactored the "use key handling" to correctly work with entities that use the continuous use functionality, like the health station.
Why?
Previous efforts to consolidate "use key handling" led to a regression where continuous use did not work as intended anymore when directly looking at such an entity.
Though, it still worked when using the use key in the proximity of the entity without directly looking at it or running into it with the use key held down, because the server then handled the continuous use by itself.
This is behavior that the client cannot "block", so we needed to refactor this code to use the server-side continuous use handling as its only source of truth again.
The root cause for this issue was that the client suppressed further use handling after sending the net message to the server, which caused the server-side
KeyPressedcheck to return false in the timer.Simply changing this would not be sufficient, as we need still to suppress this handling for the other use-cases, like weapon pickup or remote use.
Changes
We mainly refactored the server-side "use key handling" to fix this issue and simplify the code. Also, the continous use execution frequency is now every tick again, instead of every 0.1 seconds.
The actual continuous execution is now handled by solely by the server again.
The
TTT2PlayerUseEntitynet message code was refactored to only handle its special cases like spectator use, remote use and weapon pickup. We send this message from within the bind handler on the client if applicable.This code is still necessary, as these are cases where the normal use handling is not triggered by the server due to the items being out of reach or may not be correctly prioritized e.g. the wrong weapon being picked up instead of the intended looked at entity.
The corpse search and old
UseOverridehandling was moved back to the original TTT way of handling it, by utilizing theKeyReleasehook.We did not modify the code here and merely copied over the original code again.
Removed
ENT.CanUseKeyfrom placeable entities as it is not needed anymore with the new handling.Removed now obsolete
IsSpecialUsableEntityandIsUsableEntityfunctions.Refactored
ClientUsefunction to be client only and always block further use handling.This function is generally useful for client-side only use key handling, e.g., for UI interactions.
E.g. the C4 uses this to open the C4 menu when the use key is pressed while looking at it.
Fixes #1816