Draft: Jump refactor #351

2026-02-16T23:17:14Z

Lubba-64 commented

2026-02-16 23:17:14 +00:00

(Migrated from github.com)

Closes #343

jwright159 (Migrated from github.com) requested changes

2026-02-17 00:01:30 +00:00

sbepis/src/player_controller/movement/jump.rs

					
				@ -70,27 +67,45 @@ impl Default for PlayerJumpSettings {

				        let high_jump_height = 1.5;

				        let charge_jump_height = 2.0;

				        let unreal_air_jump_height = 2.5;

				        let max_hold_time = 0.3;

The min and max stamina costs are for the charging time... they make no sense in the context of normal jumps

The high jump and the slide jump are the same thing (crouch jump)

I know macros like this have their place, but they're not very bevy-ish. That's why I use the event chaining, it's a pattern all over sbepis's code (especially bevy_pretty_nice_input)

Just make this "new" if it's the only constructor function

Also auto-dereference Res<> and Mut<> when passing them through functions, so it should take &PlayerJumpSettings instead of &Res (both are called like new(&settings))

Also auto-dereference Res<> and Mut<> when passing them through functions, so it should take &PlayerJumpSettings instead of &Res<PlayerJumpSettings> (both are called like `new(&settings)`)

Java-ass name tbh, can be way simpler and component-y. Like just Jumping

Java-ass name tbh, can be way simpler and component-y. Like just `Jumping`

sbepis/src/player_controller/movement/jump.rs

					
				@ -138,0 +175,4 @@

				) -> Result {

				    commands.entity(jump.entity).insert(JumpTimer {

				        timer_max: settings.high_jump.max_hold_time,

				        speed: settings.high_jump.speed,

I also think this macro is unnecessary. With the new structs you can just have one condition that holds a JumpType and gets the right settings from the PlayerJumpSettings in the observer

Lubba-64 (Migrated from github.com) reviewed

2026-02-17 00:06:45 +00:00

Lubba-64 (Migrated from github.com) reviewed

2026-02-17 00:08:56 +00:00

sbepis/src/player_controller/movement/jump.rs

					
				@ -70,27 +67,45 @@ impl Default for PlayerJumpSettings {

				        let high_jump_height = 1.5;

				        let charge_jump_height = 2.0;

				        let unreal_air_jump_height = 2.5;

				        let max_hold_time = 0.3;

I assume

enum NeedsAGoodName {
    Scalar(f32),
    Range(f32, f32),
}

is overengineered?

I assume ```rust enum NeedsAGoodName { Scalar(f32), Range(f32, f32), } ``` is overengineered?

Lubba-64 (Migrated from github.com) reviewed

2026-02-17 00:09:45 +00:00

sbepis/src/player_controller/movement/jump.rs

					
				@ -70,27 +67,45 @@ impl Default for PlayerJumpSettings {

				        let high_jump_height = 1.5;

				        let charge_jump_height = 2.0;

				        let unreal_air_jump_height = 2.5;

				        let max_hold_time = 0.3;

just trying to make one standard struct for each of them, but this is also something I'm thinking of refactoring away.

Lubba-64 (Migrated from github.com) reviewed

2026-02-17 00:10:36 +00:00

sbepis/src/player_controller/movement/jump.rs

					
				@ -70,27 +67,45 @@ impl Default for PlayerJumpSettings {

				        let high_jump_height = 1.5;

				        let charge_jump_height = 2.0;

				        let unreal_air_jump_height = 2.5;

				        let max_hold_time = 0.3;

I think I'm going to do some refactoring to make this less confusing. thanks for letting me know.

Lubba-64 (Migrated from github.com) reviewed

2026-02-17 00:11:00 +00:00

sbepis/src/player_controller/movement/jump.rs

					
				@ -70,27 +67,45 @@ impl Default for PlayerJumpSettings {

				        let high_jump_height = 1.5;

				        let charge_jump_height = 2.0;

				        let unreal_air_jump_height = 2.5;

				        let max_hold_time = 0.3;

not super sure what that looks like...

Lubba-64 (Migrated from github.com) reviewed

2026-02-17 00:11:16 +00:00

sbepis/src/player_controller/movement/jump.rs

					
				@ -70,27 +67,45 @@ impl Default for PlayerJumpSettings {

				        let high_jump_height = 1.5;

				        let charge_jump_height = 2.0;

				        let unreal_air_jump_height = 2.5;

				        let max_hold_time = 0.3;

thanks for the tip, will refactor that.

Lubba-64 (Migrated from github.com) reviewed

2026-02-17 00:12:05 +00:00

sbepis/src/player_controller/movement/jump.rs

					
				@ -70,27 +67,45 @@ impl Default for PlayerJumpSettings {

				        let high_jump_height = 1.5;

				        let charge_jump_height = 2.0;

				        let unreal_air_jump_height = 2.5;

				        let max_hold_time = 0.3;

will change it to JumpTimer

Lubba-64 (Migrated from github.com) reviewed

2026-02-17 00:12:16 +00:00

sbepis/src/player_controller/movement/jump.rs

					
				@ -138,0 +175,4 @@

				) -> Result {

				    commands.entity(jump.entity).insert(JumpTimer {

				        timer_max: settings.high_jump.max_hold_time,

				        speed: settings.high_jump.speed,

also not sure about how that would work.

jwright159 (Migrated from github.com) reviewed

2026-02-17 00:15:13 +00:00

sbepis/src/player_controller/movement/jump.rs

					
				@ -70,27 +67,45 @@ impl Default for PlayerJumpSettings {

				        let high_jump_height = 1.5;

				        let charge_jump_height = 2.0;

				        let unreal_air_jump_height = 2.5;

				        let max_hold_time = 0.3;

Honestly all of JumpSettings might need to be an enum since the same comment goes for min_speed and max_speed... that feels wrong but it's the best thing i can think of

jwright159 (Migrated from github.com) reviewed

2026-02-17 00:18:15 +00:00

sbepis/src/player_controller/movement/jump.rs

					
				@ -70,27 +67,45 @@ impl Default for PlayerJumpSettings {

				        let high_jump_height = 1.5;

				        let charge_jump_height = 2.0;

				        let unreal_air_jump_height = 2.5;

				        let max_hold_time = 0.3;

It's the thing where the observers for all the different kinds of jumps all trigger DoJumpForRealsiesThisTime or whatever i called it, providing the necessary info to actually trigger the beginning of the jump

It's the thing where the observers for all the different kinds of jumps all trigger `DoJumpForRealsiesThisTime` or whatever i called it, providing the necessary info to actually trigger the beginning of the jump

jwright159 (Migrated from github.com) requested changes

2026-02-19 13:26:52 +00:00

sbepis/src/player_controller/movement/jump.rs

					
				@ -33,2 +31,2 @@

				#[action(invalidate = false)]

				pub struct ChargeCrouchJump;

				#[auto_component(plugin = PlayerControllerPlugin, derive(Debug, Default), reflect, register)]

				pub struct SlideJumping;

Yeah the jump heights are WAY off. This calculation is based on it being a single impulse. Instead of using this, delete it and have a constant speed for all jumps (not using this calculation, just make a magic number in the settings), varying in time instead

sbepis/src/player_controller/movement/jump.rs

Charging should be removed in here

jwright159 (Migrated from github.com) reviewed

2026-02-25 00:42:48 +00:00

sbepis/src/player_controller/movement/jump.rs

					
				@ -33,2 +31,2 @@

				#[action(invalidate = false)]

				pub struct ChargeCrouchJump;

				#[auto_component(plugin = PlayerControllerPlugin, derive(Debug, Default), reflect, register)]

				pub struct SlideJumping;