improve: refactor walking for better responsivenes

[nekiro]

Description

More smooth, better, refactored walking

Behavior

Actual

Walking is laggy

Expected

Should be smooth

Fixes

Fix #1027

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I checked the PR checks reports
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works

[Zbizu]

it's working, I had to set this one to minimum as well:

recording on otcr:

2025-01-09.17-00-00.mp4

[kokekanon]

me test :
server : main repo otland
otcr:

pressing both keys (not holding down one key permanently)

2025-01-09.12-57-04_1.mp4

[mehah]

@nekiro
the feature that puts the delay on the first step has stopped working.

because that was what reset the step when you didn't walk for 150ms.

[nekiro]

I have reworked the whole walking, it seems better to me. I also deleted some useless options.
It performs poorly on high ping though, so converting this into draft.

[Zbizu]

I've added a cout at Game::playerMove in the server code to test this change
it appears that the client doesn't send the packet at all now
(tested with 12.x walk system)
no ping issues there, as the test was carried out on localhost server

2025-01-12.06-49-44.mp4

[nekiro]

I've added a cout at Game::playerMove in the server code to test this change it appears that the client doesn't send the packet at all now (tested with 12.x walk system) no ping issues there, as the test was carried out on localhost server

2025-01-12.06-49-44.mp4

I'm testing on tfs master and no such problems were spotted

[BastardofWinterfell]

I gave this a try, but unfortunately, as is, at least for me, it does not yet solve the issues with walking. I've tried playing around with the delay settings in Options, but I am still having the same issues as before.

Here is a comparison between OTCv8 and this OTC of walking in a straight line and then suddenly stopping. I was pressing the W key to walk north and stopped pressing it as the char arrives at the gold coin. In OTC v8, the char stops exactly on the tile and immediately when I stopped pressing the W key, as it should. In this OTC, the char stops for a second on the correct tile, but then takes an extra step forward after I'm no longer pressing any key.

lv_0_20250109184103.mp4

Here is a comparison between OTCv8 and this OTC of walking back and forth between two specific tiles with gold coins to show how the clients handle sudden direction changes. In OTC v8, the character stops and turns around on the correct tile with the gold coin when I stop pressing one key and start pressing the other, as it should. In this OTC, the character seems to "stutter" during this direction change as if the client is predicting that I'm going to keep going instead of turning around.

lv_0_20250109184720.mp4

Here is a comparison between OTCv8 and this OTC of walking over a path, marked by gold coins on the ground, and making two turns in different directions. In OTC v8, the character follows my exact key presses with no delay, no extra steps, no attempt to correct itself, and perfectly follows my intended path over the gold coins. In this OTC, the character "stutters" during the direction change and walks on the wrong tiles.

lv_0_20250109185348.mp4

Only difference in these videos is the client. I'm using the same character walking with the same speed, playing on the same server (protocol 10.41) with the same average ping (around 150 ms) and average fps (around 120) playing on the same PC.

I'm hoping this can eventually be sorted out and the walking system in this OTC made to behave and feel as fluid and responsive as OTCv8.

[BastardofWinterfell]

After further investigation and testing, I was able to almost completely "fix" the walking issues I described in my previous comment with the following two changes:

1. https://github.com/mehah/otclient/blob/main/src/client/localplayer.cpp#L44

Following @kokekanon's suggestion, changed this line from:
return m_walkTimer.ticksElapsed() >= getStepDuration() && (dir == m_direction || !m_walking);
to:
return m_walkTimer.ticksElapsed() >= getStepDuration() && (!m_walking);
This fixed the extra step the character was taking when suddenly stopping and the "stutter" when changing directions quickly.

2. https://github.com/mehah/otclient/blob/main/modules/game_walk/walk.lua#L41

Changed this line from:
modules.game_interface.getRootPanel():setAutoRepeatDelay(200)
to:
modules.game_interface.getRootPanel():setAutoRepeatDelay(10)
This fixed missed key presses which greatly increased the perceived responsiveness and fluidity of the character movement for me.

Not sure if this is how it is supposed to be, but it seems that the AutoRepeatDelay is being "set" in at least two different places in the code:

• https://github.com/mehah/otclient/blob/main/modules/game_walk/walk.lua#L41 (which was set to 200)
• https://github.com/mehah/otclient/blob/main/modules/game_interface/gameinterface.lua#L136 (which was set to 50)

[nekiro]

After further investigation and testing, I was able to almost completely "fix" the walking issues I described in my previous comment with the following two changes:

1. https://github.com/mehah/otclient/blob/main/src/client/localplayer.cpp#L44

Following @kokekanon's suggestion, changed this line from: return m_walkTimer.ticksElapsed() >= getStepDuration() && (dir == m_direction || !m_walking); to: return m_walkTimer.ticksElapsed() >= getStepDuration() && (!m_walking); This fixed the extra step the character was taking when suddenly stopping and the "stutter" when changing directions quickly.

2. https://github.com/mehah/otclient/blob/main/modules/game_walk/walk.lua#L41

Changed this line from: modules.game_interface.getRootPanel():setAutoRepeatDelay(200) to: modules.game_interface.getRootPanel():setAutoRepeatDelay(10) This fixed missed key presses which greatly increased the perceived responsiveness and fluidity of the character movement for me.

Not sure if this is how it is supposed to be, but it seems that the AutoRepeatDelay is being "set" in at least two different places in the code:

• https://github.com/mehah/otclient/blob/main/modules/game_walk/walk.lua#L41 (which was set to 200)
• https://github.com/mehah/otclient/blob/main/modules/game_interface/gameinterface.lua#L136 (which was set to 50)

It was a draft (not yet done), not sure what about your fixes, but they seem to be workarounds. I did rework whole walking system just now, so let me know if this is now better than it was before.

One important thing to note for all other testers, you have to disable BOT_PROTECTION macro, because of the g_game.walk call from lua side. Explained it in #1037

The only problem I noticed with current code is that diagonals arent properly timed, so character gets kinda stuck on trying to prewalk too fast. I'll take a look at this soon.

[kokekanon]

simple test

Server	Ping
tfs 8.6 (0.4)	100-150	✅✅ (improved significantly)
tfs 1.5 (8.6)	1	✅
tfs 1.4.2(10.98)	1	✅
tfs 1.6 (13.10)	1	✅
Canary (11.00)	1	✅
Canary (13.40)	1	✅

[nekiro]

I will also take a look at how sudden speed changes behave with these changes, because before it was very buggy.

[kokekanon]

Insignificant detail

lvl 73
sudden speed changes
speed : 182 -> 40 (paralice)

PR otcr

moonwalking

Video.mov

vs

v8

bandicam.2025-01-13.19-48-19-654.mp4

vs

cipsoft

bandicam.2025-01-13.21-00-11-646.mp4

[Zbizu]

Environment

client: 13.20
server: tfs-based
platform: Windows for both
connection type: localhost
login type: protocol login (legacy charlist request over 7171 port)
tests are marked as OK when their results are same as rl client

Test results

first step:
bot protection on: still broken ❌ (see #1037)
bot protection off: OK ✅

(rest of tests were taken with bot protection off)
two coins test - OK ✅
path of coins test - OK ✅
diagonal steps (numpad) - OK ✅
buffering next step - OK ✅ (pressing arrows to queue next step when walk locked on low speed)
stairhop/teleporthop test - OK ✅ - this test was about how fast I can move away from the stairs after I switched the floor - default setting was too high, but I was able to get 1:1 results when I set floor/teleport delay to 50ms
walk after turn - OK ✅ - Even better than in rl client (rl client has horrible delay for this, somewhere between 500 and 1000ms)
packets sent - OK ✅ - I no longer get disconnected for sending too many packets

Tests were repeated on Ubuntu dedicated server connecting with Windows client (ping: 30ms), same server engine. Results: OK (except for bot protection "on")
All tests were performed using OpenGL build

[BastardofWinterfell]

With these latest changes, I'm happy to inform that almost all issues I previously had with the walking being laggy and unresponsive have been solved. Therefore, first things first: awesome work @nekiro and thank you!

However, I am still getting an extra step after a key is no longer being pressed and stutters when changing directions quickly. Here's a video of what I mean:

2025-01-14_16-39-20.mp4

If I implement this change to localplayer.cpp#L44, the extra step and stutter when changing directions no longer happens.

1. https://github.com/mehah/otclient/blob/main/src/client/localplayer.cpp#L44

Following @kokekanon's suggestion, changed this line from:
return m_walkTimer.ticksElapsed() >= getStepDuration() && (dir == m_direction || !m_walking); to:
return m_walkTimer.ticksElapsed() >= getStepDuration() && (!m_walking);
This fixed the extra step the character was taking when stopping and the "stutter" when changing directions quickly.

[nekiro]

Does not reproduce for me on latest tfs master with 100-150 ping.

[BastardofWinterfell]

I'm playing on a 10.41 protocol/client version TFS1.0-based server with 150~180 ping.
Not sure what causes the extra step and stutter, but with the change to localplayer.cpp#L44, it does not happen.
I figured I'd let you know in case there is a better solution somewhere else in the code.

[nekiro]

I'm playing on a 10.41 protocol/client version TFS1.0-based server with 150~180 ping. Not sure what causes the extra step and stutter, but with the change to localplayer.cpp#L44, it does not happen. I figured I'd let you know in case there is a better solution somewhere else in the code.

I'm no longer using the code you mention to change. This code was rewritten and looks different now. I tried with delay of 150-180 and still no walk back is happening for me. I'll try with program to delay my localhost network, so maybe I can reproduce it.

[BastardofWinterfell]

My apologies, @nekiro.
It was a noob mistake on my part.

I'm glad to report that after correctly pulling the latest changes from this PR, walking is working completely fine. No more issues, and controlling the character feels a million times better than before. No lag, no key press delay, no extra steps, no stutters when changing directions. All issues I was having with walking have been solved. Thank you very much.

[mehah]

OTClient.-.Redemption.2025-01-14.18-40-53.mp4

@nekiro
I tested with Canary 13.40 and the camera is stuttering, I don't know if you can see it in the video, wait for someone else to confirm. (@kokekanon)

but if it's ok for everyone, we'll merge it without any problems.

[nekiro]

OTClient.-.Redemption.2025-01-14.18-40-53.mp4
@nekiro I tested with Canary 13.40 and the camera is stuttering, I don't know if you can see it in the video, wait for someone else to confirm. (@kokekanon)

but if it's ok for everyone, we'll merge it without any problems.

Can't see in video, but this pull request still need changes for sudden speed changes, so not ready to merge yet.

[mehah]

2025-01-14.18-57-56.mp4

I recorded it now with OBS, I think it's better, pay attention to the houses, the camera keeps shaking.

but in the video it is almost imperceptible.

[kokekanon]

I tested with Canary 13.40 and the camera is stuttering, I don't know if you can see it in the video, wait for someone

in my particular case

Client: this PR GL.
Server : canary 13.40
PC: i7-10700k . 3080 nvidia. 32 gb ram

I don't notice the “camera is stuttering”.
I remember that a user once had it happen, he deactivated the v-sync and the camara stuttering was solved

[mehah]

@nekiro @kokekanon
resolved by setting g keyboard.setKeyDelay(key, 10), but I don't know what it might affect on other protocols.

[nekiro]

@nekiro @kokekanon resolved by setting g keyboard.setKeyDelay(key, 10), but I don't know what it might affect on other protocols.

It should be good, the only thing we can be afraid of is cpu usage with that fast repeat
I'll try to reproduce the stutter with current code and adjust accordingly.

[mehah]

@nekiro @kokekanon resolved by setting g keyboard.setKeyDelay(key, 10), but I don't know what it might affect on other protocols.

It should be good, the only thing we can be afraid of is cpu usage with that fast repeat I'll try to reproduce the stutter with current code and adjust accordingly.

setRepeat would only be for that key that the player would be pressing at that moment, when he released it, it would return to the normal state, so I believe it won't have much cost.

@nekiro
in fact, we would have to put
g keyboard.setKeyDelay(key, 10) inside g_keyboard.bindKeyDown
and g_keyboard.setKeyDelay(key, 30) inside g_keyboard.bindKeyUp

[nekiro]

I believe this is ready to be merged. I did not work out the speed change bugs (they are the same as they were before these changes so it's okay), because I can't currently invest more time into it, so we can create a new issue for it (I think there is one already for it though #963).

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Nottinghster]

@kokekanon @luanluciano93 @Zbizu @BastardofWinterfell

Guys, can you test it?

[mehah]

canary 13.40 locally, working as expected, now it will be necessary to test on other protocols.