GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Health and Fall damage systems by MrGunflame · Pull Request #462 · feather-rs/feather

Conversation

@MrGunflame

@MrGunflame MrGunflame commented Sep 11, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

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)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
MrGunflame and others added 10 commits September 11, 2021 12:25
@MrGunflame
Fix typo
cdddb2b
@MrGunflame
First Health and Falldamage system
9d0a2ce
@MrGunflame
Add reset_player
b270390
@MrGunflame
Add regenerate to Health
b1f95b1
@MrGunflame
Remove abs causing weird behavoir when jumping before falling
d9422d2
@MrGunflame
Remove entities on 0 health
a4b9be7
@MrGunflame
Someone removed plugin_message
b037201
@MrGunflame
Default for Hunger
841c772
@MrGunflame
Add Health to player
88b6dac
@MrGunflame
Update reset_player
f975e31
ambeeeeee
ambeeeeee reviewed Sep 11, 2021
View reviewed changes
Comment thread feather/common/src/game.rs Outdated
self.set_block(pos, BlockId::air())
}

pub fn reset_player(&self, player: EntityRef) {

@ambeeeeee ambeeeeee Sep 11, 2021

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
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.

@ambeeeeee ambeeeeee Sep 11, 2021

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
MrGunflame added 5 commits September 12, 2021 08:18
@MrGunflame
Update falldamage calculation
0d37699
@MrGunflame
Make reset_player failiable, default to default_gamemode on respawn
3958d97
@MrGunflame
Remove unnecessary borrow
7d33f78
@MrGunflame
Update doc
b2fcfe5
@MrGunflame
Unload player entity when dead, recreate when respawning
4cefe92
@MrGunflame

MrGunflame commented Sep 12, 2021

Copy link
Copy Markdown
Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@qualterz

qualterz commented Sep 13, 2021

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

MrGunflame added 2 commits September 13, 2021 17:46
@MrGunflame
Reset fall distance when moving upwards
973606f
@MrGunflame
Only add FallDistance to entities with Health component
6dba89a
@MrGunflame

MrGunflame commented Sep 13, 2021

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@qualterz

qualterz commented Sep 13, 2021

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@MrGunflame

MrGunflame commented Sep 14, 2021

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).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Defman

Defman commented Sep 14, 2021

Copy link
Copy Markdown
Member

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.

MrGunflame, DamianLesiuk-360Training, and noahcoetsee reacted with thumbs up emoji
All reactions
  • 👍 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

MrGunflame and others added 5 commits September 17, 2021 22:59
@MrGunflame
Add Health components to entities
fa21991
@MrGunflame
Split falldamage calculation between players and other entities
2d626a6
@MrGunflame
Merge branch 'feather-rs:main' into main
ba49714
@MrGunflame
Merge branch 'feather-rs:main' into main
a99aae8
@MrGunflame
Merge branch 'main' into health
e386c6d
Nufflee
Nufflee reviewed Sep 28, 2021
View reviewed changes
Comment thread feather/server/src/client.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

ambeeeeee
ambeeeeee suggested changes Sep 28, 2021
View reviewed changes
Comment thread feather/server/src/packet_handlers/health.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();

@ambeeeeee ambeeeeee Sep 28, 2021

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@MrGunflame MrGunflame Sep 30, 2021

Copy link
Copy Markdown
Author

There was a problem hiding this comment.

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread feather/server/src/packet_handlers/health.rs Outdated
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();

@ambeeeeee ambeeeeee Sep 28, 2021

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
MrGunflame and others added 5 commits September 29, 2021 08:17
@MrGunflame @Nufflee
Update feather/server/src/client.rs
26be034
Co-authored-by: Nufflee <[email protected]>
@MrGunflame
Merge branch 'feather-rs:main' into main
46d8237
@MrGunflame
Update error handling
6fef440
@MrGunflame
Merge branch 'health' of github:MrGunflame/feather into health
720bccd
@MrGunflame
Merge branch 'main' into health
974c24a
@leElvyn

leElvyn commented Jan 31, 2022

Copy link
Copy Markdown
Contributor

Is this PR mergable ?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@MrGunflame

MrGunflame commented Feb 1, 2022

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.

leElvyn reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

Reviewers

2 more reviewers
@Nufflee Nufflee Nufflee left review comments
@ambeeeeee ambeeeeee ambeeeeee requested changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@MrGunflame @qualterz @Defman @leElvyn @ambeeeeee @Nufflee
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.