Skip to content

Health and Fall damage systems#462

Open
MrGunflame wants to merge 27 commits intofeather-rs:mainfrom
MrGunflame:health
Open

Health and Fall damage systems#462
MrGunflame wants to merge 27 commits intofeather-rs:mainfrom
MrGunflame:health

Conversation

@MrGunflame
Copy link
Copy Markdown

@MrGunflame MrGunflame commented Sep 11, 2021

Health and Fall damage systems

Status

  • Ready
  • Development
  • Hold

Description

Adds Health and Hunger components. Adds a health and fall damage system.

Related issues

#358

Progress

Currently waiting for physics/velocity for correct player fall damage calcuation

  • Health and Hunger components
  • Health system
  • Fall damage system
  • Hiding dead clients for others

Checklist

  • Ran cargo fmt, cargo clippy --all-targets, cargo build --release and cargo test and fixed any generated errors!
  • Removed unnecessary commented out code
  • Used specific traces (if you trace actions please specify the cause i.e. the player)

Comment thread feather/common/src/game.rs Outdated
self.set_block(pos, BlockId::air())
}

pub fn reset_player(&self, player: EntityRef) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just completely ignoring a failure here is not ideal, maybe return an error of some kind upon failure.

Comment thread feather/server/src/entities.rs Outdated
Comment on lines +62 to +63
/// Increasing distance of an entity falling. Used to calculate
/// fall damage for entities with health.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming I understand what this means, I think it would be best to say something similar to
/// Distance fallen since an entity was on solid ground.
Without the explanation of what the type is for.

@MrGunflame
Copy link
Copy Markdown
Author

Should be mostly complete now. Someone glance over it again.

@qualterz
Copy link
Copy Markdown
Contributor

It works quite weird when you bunnyhoping down from low hills, and you also take damage during bunnyhop with some delay.

@MrGunflame
Copy link
Copy Markdown
Author

Really hard to reproduce, I believe what happened was that when you hit the ground and jump again before the server ticks, the fall damage is calculated for both jumps.
Resetting the fall distance when moving up should fix this, but this behavior should only be applied to players.

@qualterz
Copy link
Copy Markdown
Contributor

Yep, looks like it is fixed. But there is another problem, sometimes when you fell down, the server does not register your fall at all, no one debug message about the missed fall. Demo

@MrGunflame
Copy link
Copy Markdown
Author

That's a really weird one, seems to only happen when, immediately after connecting, you jump from a cliff into another chunk causing the client to reload and for some reason teleporting a bit up, resetting the falling distance. I'm not sure how to solve this in the current implementation while not re-enabling the bunnyhopping issue. I guess there's a reason minecraft uses player velocity to calculate fall damage, but that's not implemented in feather yet (afaik).

@Defman
Copy link
Copy Markdown
Member

Defman commented Sep 14, 2021

I think this is mostly do to physics not being implemented on server side, which means we 100% rely on the data from the client.

Comment thread feather/server/src/client.rs Outdated
Comment on lines +15 to +16
let client_id = game.ecs.get::<ClientId>(player_id).unwrap();
let client = server.clients.get(*client_id).unwrap();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you ? these instead of unwrapping?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, not sure why I used unwrap there, but returning an error is the better way. Though not sure what error would be appropriate for server.clients.get instead of unwrapping if any?

Comment on lines +23 to +25
let network_id = game.ecs.get::<NetworkId>(player_id).unwrap();
let position = game.ecs.get::<Position>(player_id).unwrap();
let uuid = game.ecs.get::<Uuid>(player_id).unwrap();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you ? these instead of unwrapping?

@leElvyn
Copy link
Copy Markdown
Contributor

leElvyn commented Jan 31, 2022

Is this PR mergable ?

@MrGunflame
Copy link
Copy Markdown
Author

Currently this implementation works for mobs, but for players it's a little buggy due to entity physics not being implemented yet. I'd therefore wait until entity physics are implemented before merging this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants