Simple client-side autojump. Remove hardcoded Android stepheight #7228

bendeutsch · 2018-04-09T04:59:12Z

During the movement phase, the client detects when an automatic jump is in order (based on collisions and such). Then, during the (next) control collection phase, a "jump" control is simulated.

The autojump is transparent for the server. It is indistinguishable from a regular jump, in fact it is a regular jump, just not triggered by the player but by the client software. The actual motion code is also unchanged.

Currently, jumps are only triggered when the player is moving forwards and is standing on solid ground and not swimming. The feature will be toggleable by a client setting (default enabled on mobile devices, disabled otherwise).
////////////
EDIT by paramat: For #6183

bendeutsch · 2018-04-09T05:04:00Z

At the time of writing, the "collision detection" part is basically broken. I was trying to use the collision results from the movement code, but those seem to be incomplete / unusable for this purpose? Sometimes I don't even get the collision from the ground I'm standing on 🙁

Still, the jumping part works, if you want to test it, it just may need running against a wall a bit. At the moment, the wall can be more than one block high; testing this is planned, but futile until I find a good detection method in the first place.

I may have to simulate a jump and see if the player "fits better". Other suggestions welcome!

paramat · 2018-04-09T16:49:19Z

Interesting and appreciated. I'm just wondering if there needs to be some server control over allowing this clientside option, probably yes as no having to use the jump button is inevitably a control advantage.

bendeutsch · 2018-04-09T18:30:53Z

You can't really control this server-side, but it's less of an advantage than you think. Currently it's triggering by actually colliding with something (and hence stopping). If you manually jump before you reach the wall, you get to keep your horizontal speed. There will also certainly be situations in which autojump triggers even if you would rather not jump; again not a problem with manual control. So I would not agree to it being "inevitably a control advantage".

And even if it is: if it's available to everyone, can you call it an advantage? Or are you thinking of singleplayer / PvE games that want to make / keep the game more challenging for players? In that case I could see a use for a server turning it off, but I would caution against coding up yet another option until servers ask for it…

paramat · 2018-04-09T19:51:08Z

Yes i'm unsure if server control is needed or worth doing, for now i'm not asking for it, server owners can advise on this.
What i mean by 'inevitable advantage' is not any possible movement advantages it creates but simply not having to use the jump button, that itself is a type of minor control advantage.

if it's available to everyone, can you call it an advantage?

Well what i mean is most players will not want to use it, and they shouldn't feel any pressure to use it due to any slight advantages it gives to other players. So that argument is invalid in my opinion.
However i expect autojump will always be less responsive than manual jump so it will probably come with a slight movement disadvantage, which is actually good to balance not needing to use the key.

bendeutsch · 2018-04-10T05:01:36Z

@paramat Ah, I see what you mean with "pressure to use it", thanks.

A new version is up. It appears that m_standing_node is not as reliable as I would have hoped. I've replaced the baseline for collision comparisons with getFootstepNode, which works better, but is a) semantically incorrect and b) does not "go deep enough" (it is made to return the carpet node you're standing on, which barely affects jumping).

The jump key is now "held down" for a while, and sneaking disables autojump.

Wayward1 · 2018-04-11T17:48:42Z

I really like this! It feels much more natural than using an increased stepheight (like on Android), and if made default on Android would remove the perceived speed advantage the increased stepheight gives Android users. There is one possible issue I spotted (though I am aware this is still WIP); when going up stairs there is an occasional higher "hop" in addition to the regular half-jump of normal stair ascent. I've made a quick video of this PR here: https://www.youtube.com/watch?v=DGVdEnrXk1E&feature=youtu.be

bendeutsch · 2018-04-11T18:43:47Z

@Wayward1 thanks for checking this out! Yes, it's still quite rough – it still needs some work on heights and collisions. I'll be sure to study your video!

In related news, autojump now has it's own setting (default off), including a checkbox in the key controls gui.

bendeutsch · 2018-04-12T20:45:45Z

I've got a new implementation: whenever the player collides horizontally, I test the same colliding movement one node higher. If it turns out that this results in a bit more horizontal movement, I trigger an autojump. This seems to work very well; single-node obstacles are reliably cleared, more than that is not even attempted. This works equally well with slabs and stairs; regular up-stepping movement no longer triggers a jump.

I had to add a field to the collision info struct for this, to help me find "horizontal collisions". I hope that's ok. Since I imagine collision detection is quite costly, I want to limit it to such cases.

All that's left to do is to refactor the autojump into a method and add this to old_move as well. Oh, and I just remembered I still need to test what happens if you bump your head while autojumping; it would be desirable to not autojump then.

paramat · 2018-04-12T21:29:53Z

src/localplayer.cpp

+		if (autojump_time <= 0.0f) {
+			autojump = false;
+		}
+	} else if (m_can_jump && !control.jump && !control.sneak && control.up && g_settings->getBool("autojump")) {


The autojump setting should be cached instead of reading it from settings frequently.

Good point; do you have an example of a cached setting that I could copy? Just off the top of your head?

I'll look. We can cache this because it is not a setting that will change in-game.

Actually it will; it's part of the key change GUI.

Should be solved with the PlayerSettings construct.

paramat · 2018-04-12T21:30:48Z

I had to add a field to the collision info struct for this, to help me find "horizontal collisions". I hope that's ok. Since I imagine collision detection is quite costly, I want to limit it to such cases.

Seems ok, collisions are costly so this seems good.

paramat · 2018-04-12T21:45:17Z

Settings cache example in another part of clientside code:

minetest/src/camera.cpp

Line 72 in 326eeca

m_cache_fall_bobbing_amount = g_settings->getFloat("fall_bobbing_amount");

^ Set the cache in the constructor.

minetest/src/camera.h

Line 225 in 326eeca

f32 m_cache_fall_bobbing_amount;

^ Variable is a 'private' class member.

bendeutsch · 2018-04-13T05:04:41Z

Autojumping is now in it's own method and works in the old movement code as well (and should work with joysticks, though I'll have to test this later). Since I've gotten all that I want from this PR, I'm removing the "WIP". There is still the issue with settings caching above, don't worry, I haven't forgotten! Just need to wrap my head around settings callback handlers 😛

The player will actually bang their head, repeatedly, when running against a node that is clear above, but that has a ceiling in front of it. There is a method for this in collision.cpp, but it uses helper structures internal to the collision testing, and as such is non-trivial to use. For the time being, I recommend just not banging your head so much.

bendeutsch · 2018-04-15T14:11:51Z

Please see #7243 for a proposed solution to settings caching. That PR does not contain autojump, of course, but it is relatively trivial to add it to this PR once that PR is accepted.

Would this help with the settings reading concerns?

bendeutsch · 2018-04-19T04:47:12Z

I have rebased this PR again, incorporating #7243 changes, and have added autojump to those flags cached by player settings. This should solve the caching concerns.

I haven't tried building and testing on Android, yet, has someone had a look? Specifically, I'm not sure how good the behaviour can be enabled and disabled in the client.

Wayward1 · 2018-04-21T11:24:03Z

I just tested this on Android, and it works great! And it's as easy as any other advanced setting to change as well

Fixer-007 · 2018-04-21T12:05:16Z

Provocative question, can PC users have this configurable feature too? Disabled by default

Wayward1 · 2018-04-21T14:11:40Z

@Fixer-007 this is available for all users.

bendeutsch · 2018-08-26T20:55:06Z

There was still a linting problem outstanding; I must have missed this. Fixed (and rebased while we're at it).

paramat · 2018-09-02T04:17:19Z

This is high in my priority queue, i'll review and hopefully add the 2nd approval soonish.

SmallJoker · 2018-11-19T19:22:50Z

@paramat Just wondering - how long is your priority queue?

EDIT: Can we please finally get this PR into the game?

paramat · 2018-11-19T23:05:37Z

LOL sorry. This should certainly be in for 5.0.0.

paramat · 2018-11-20T00:58:10Z

Apart from my line comment code looks good.
Settingtypes needs rebasing.
As a general tip: don't bother editing conf.example in PRs, we auto-generate that occasionally to update it.
So i'm very close to +1 on this @bendeutsch

paramat · 2018-11-20T02:35:47Z

src/localplayer.cpp

+	// jumped
+	v3f run_delta = position_before_move - m_position;
+	run_delta.Y = 0.0f;
+	v3f jump_delta = position_before_move - jump_pos;


Here and above. Mathematically works but the order of terms bothers me because a movement vector is 'final pos' - 'start pos'.

Works by detecting a collision while moving forward and then simulating a jump. If the simulated jump is more successful, an artificial jump key press is injected in the client. Includes setting and key change GUI element for enabling and disabling this feature.

paramat · 2018-11-21T20:48:21Z

src/localplayer.cpp

 	run_delta.Y = 0.0f;
-	v3f jump_delta = position_before_move - jump_pos;
+	v3f jump_delta = initial_position - jump_pos;


Might as well swap the terms too, i'm +1 and you can merge this if you do.

For a 1D example:
Moving from 2 to 5 is a movement of +3, which is '5 - 2', 'final - initial'.
These deltas are wrong (inverted), they only work because getting the length of a vector is unaffected by the sign.

Whether it's correct or wrong depends on the viewpoint. Deltas can be measured in all directions.

Somewhat true, but a delta is usually measured going forwards in time from initial to final position.

paramat · 2018-11-21T20:48:54Z

src/localplayer.cpp

@@ -1147,9 +1147,9 @@ void LocalPlayer::handleAutojump(f32 dtime, Environment *env,

 	// see if we can get a little bit farther horizontally if we had
 	// jumped
-	v3f run_delta = position_before_move - m_position;
+	v3f run_delta = initial_position - m_position;


Swap terms.

stujones11 · 2018-11-22T17:53:07Z

Tested and working nicely on android 👍

bendeutsch · 2018-11-22T18:18:24Z

Hi there, sorry I took so long before reacting. Thanks @SmallJoker for stepping in – I take it it's been rebased now, too?

@paramat I got an email that you have a question about "always jumping", but I can't find it above any more. I'd have to review the code again, but I'm pretty sure the "simulate the jump key" feature only works if you're moving into something while on the ground, so it's "released" as soon as you're airborne at least. But I'll have another look.

Edit: I've checked, the key is "held" for 100ms, then released again and could be re-pressed.

SmallJoker · 2018-11-22T18:20:14Z

@bendeutsch Yes, I rebased the PR on the fly because I wanted to make sure that it works on the current HEAD as well (it does).

paramat · 2018-11-22T20:51:55Z

@bendeutsch

I got an email that you have a question about "always jumping", but I can't find it above any more.

I deleted the comment soon after because i realised i was wrong, there's no problem.
👍

…st#7228) Works by detecting a collision while moving forward and then simulating a jump. If the simulated jump is more successful, an artificial jump key press is injected in the client. Includes setting and key change GUI element for enabling and disabling this feature.

paramat added the Feature ✨ PRs that add or enhance a feature label Apr 9, 2018

rubenwardy added the WIP The PR is still being worked on by its author and not ready yet. label Apr 9, 2018

bendeutsch mentioned this pull request Apr 9, 2018

Move player movement code into Player class #7220

Closed

bendeutsch force-pushed the client-autojump branch from a3bbc22 to 7b7db8b Compare April 10, 2018 04:54

bendeutsch force-pushed the client-autojump branch from 7b7db8b to 55980a7 Compare April 11, 2018 18:36

paramat mentioned this pull request Apr 11, 2018

Going up stairs is faster on Android (faster swim and stepheight advantages) (autojump discussion) #6183

Closed

bendeutsch force-pushed the client-autojump branch from 55980a7 to 642eb9c Compare April 12, 2018 20:36

paramat reviewed Apr 12, 2018

View reviewed changes

bendeutsch force-pushed the client-autojump branch from 642eb9c to dd9a828 Compare April 13, 2018 04:58

bendeutsch changed the title ~~[WIP] Simple client-side autojump~~ Simple client-side autojump Apr 13, 2018

bendeutsch mentioned this pull request Apr 15, 2018

PlayerSettings struct for player movement code #7243

Merged

bendeutsch mentioned this pull request Apr 15, 2018

Server side movement #6219

Closed

bendeutsch force-pushed the client-autojump branch from dd9a828 to 850d9ee Compare April 19, 2018 04:41

bendeutsch force-pushed the client-autojump branch from 850d9ee to 22d934d Compare April 22, 2018 12:17

SmallJoker approved these changes Aug 23, 2018

View reviewed changes

SmallJoker added the One approval ✅ ◻️ label Aug 23, 2018

bendeutsch force-pushed the client-autojump branch 2 times, most recently from 79ed26d to 5c7d871 Compare August 26, 2018 20:23

SmallJoker mentioned this pull request Sep 23, 2018

Stepheight issues with touching ground in liquid #7742

Open

SmallJoker added the Rebase needed The PR needs to be rebased by its author. label Nov 19, 2018

paramat reviewed Nov 20, 2018

View reviewed changes

SmallJoker force-pushed the client-autojump branch from 5c7d871 to a6ecc80 Compare November 21, 2018 19:26

SmallJoker removed the Rebase needed The PR needs to be rebased by its author. label Nov 21, 2018

Rename _before move to initial_

effd328

paramat reviewed Nov 21, 2018

View reviewed changes

swap terms

c51546e

paramat added >= Two approvals ✅ ✅ and removed One approval ✅ ◻️ labels Nov 22, 2018

paramat merged commit 93bccb3 into minetest:master Nov 22, 2018

gaelysam mentioned this pull request Nov 23, 2018

Add pitch fly mode #7817

Merged

Simple client-side autojump. Remove hardcoded Android stepheight #7228

Simple client-side autojump. Remove hardcoded Android stepheight #7228

Conversation

bendeutsch commented Apr 9, 2018 • edited by paramat

bendeutsch commented Apr 9, 2018

paramat commented Apr 9, 2018

bendeutsch commented Apr 9, 2018

paramat commented Apr 9, 2018 • edited

bendeutsch commented Apr 10, 2018

Wayward1 commented Apr 11, 2018

bendeutsch commented Apr 11, 2018

bendeutsch commented Apr 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paramat commented Apr 12, 2018 • edited

paramat commented Apr 12, 2018 • edited

bendeutsch commented Apr 13, 2018

bendeutsch commented Apr 15, 2018

bendeutsch commented Apr 19, 2018

Wayward1 commented Apr 21, 2018

Fixer-007 commented Apr 21, 2018

Wayward1 commented Apr 21, 2018 • edited

bendeutsch commented Aug 26, 2018

paramat commented Sep 2, 2018 • edited

SmallJoker commented Nov 19, 2018 • edited

paramat commented Nov 19, 2018

paramat commented Nov 20, 2018 • edited

Choose a reason for hiding this comment

paramat Nov 21, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stujones11 commented Nov 22, 2018

bendeutsch commented Nov 22, 2018 • edited

SmallJoker commented Nov 22, 2018

paramat commented Nov 22, 2018 • edited

bendeutsch commented Apr 9, 2018 •

edited by paramat

paramat commented Apr 9, 2018 •

edited

paramat commented Apr 12, 2018 •

edited

paramat commented Apr 12, 2018 •

edited

Wayward1 commented Apr 21, 2018 •

edited

paramat commented Sep 2, 2018 •

edited

SmallJoker commented Nov 19, 2018 •

edited

paramat commented Nov 20, 2018 •

edited

paramat Nov 21, 2018 •

edited

bendeutsch commented Nov 22, 2018 •

edited

paramat commented Nov 22, 2018 •

edited