Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: respawn #702

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

feat: respawn #702

wants to merge 13 commits into from

Conversation

GroobleDierne
Copy link
Collaborator

@GroobleDierne GroobleDierne commented Dec 12, 2024

Current state:

  • Works with and without enable_respawn_screen
  • Restores player's health
  • Change the player's team

Todo

  • Respawn next to a teammate

When respawned, the client tries to teleport to its respawn location as defined by PlayerSpawnPositionS2c, but the server prevents this as it exceeds the maximum distance that the client is allowed to move in one tick. This doesn't matter for the Tag event as we'll have to teleport the player to another location anyway, but it could be an issue for other events, so we should have a way to bypass this distance check when it follows a respawn.

@andrewgazelka
Copy link
Collaborator

andrewgazelka commented Dec 12, 2024

Current state:

  • Works with and without enable_respawn_screen
  • Restores player's health
  • Change the player's team

Todo

  • Respawn next to a teammate

When respawned, the client tries to teleport to its respawn location as defined by PlayerSpawnPositionS2c, but the server prevents this as it exceeds the maximum distance that the client is allowed to move in one tick. This doesn't matter for the Tag event as we'll have to teleport the player to another location anyway, but it could be an issue for other events, so we should have a way to bypass this distance check when it follows a respawn.

Thanks for your work. :) I don't think we should bypass the distance check; instead we should set the player location properly before we send the respawn packet. bypassing the distance check is a big vulnerability. For instance:

  • player could decide they just wanna teleport to any place in the map
  • player could just not respawn at all and continue with respawn gear but in same place as they died

@andrewgazelka andrewgazelka changed the title feat(respawn) feat: respawn Dec 12, 2024
@github-actions github-actions bot added the feat label Dec 12, 2024
@andrewgazelka
Copy link
Collaborator

andrewgazelka commented Dec 12, 2024

@SanderMertens thoughts on

https://github.com/andrewgazelka/hyperion/blob/5cdd9e36b85369f1e77f67277f32bc65b28c2a69/crates/respawn/src/lib.rs#L34-L35

vs not having Pose in the query and doing .set(Pose::Standing)? also @Indra-db since I know you (or your friend) is working on this I think.

@GroobleDierne
Copy link
Collaborator Author

Just rebased everything.

@andrewgazelka what is the preferred way to get every player on a specific team?
Also client.modified::<Pose>(); doesn't work properly, the client switch to the dying pose correctly but won't go back to standing.

@andrewgazelka
Copy link
Collaborator

@andrewgazelka what is the preferred way to get every player on a specific team?

use .query (similar to system DSL)

could do

    world
        .query::<&Team>()
        .each_entity(|_, team|) { 
             if team != target continue 
             // ...
         }

inside of a system. see https://github.com/Indra-db/Flecs-Rust/tree/main/flecs_ecs/examples/flecs/queries

Also client.modified::<Pose>(); doesn't work properly, the client switch to the dying pose correctly but won't go back to standing.

just do what you were doing then

@andrewgazelka andrewgazelka marked this pull request as ready for review December 18, 2024 17:30
Comment on lines +28 to +29
for event in event_queue.drain() {
let client = event.client.entity_view(world);
Copy link

Choose a reason for hiding this comment

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

The code should match against event.status to ensure it equals ClientStatusCommand::PerformRespawn before proceeding with the respawn logic. Without this check, the code will attempt to respawn players who request stats, which is the other variant of ClientStatusCommand.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

@andrewgazelka
Copy link
Collaborator

Screen.Recording.2024-12-18.at.9.34.50.AM.mov

still appears a bit bugged :(

also—"Respawn next to a teammate" can probably be done in a future PR. let's try to get this in.

@GroobleDierne
Copy link
Collaborator Author

Screen.Recording.2024-12-18.at.9.34.50.AM.mov

still appears a bit bugged :(

also—"Respawn next to a teammate" can probably be done in a future PR. let's try to get this in.

This was introduced by a5c2f89 I'll look at what caused that.
Also there is a new very weird regression with this commit:

hotbar.bug.mp4

@andrewgazelka
Copy link
Collaborator

Screen.Recording.2024-12-18.at.9.34.50.AM.mov
still appears a bit bugged :(
also—"Respawn next to a teammate" can probably be done in a future PR. let's try to get this in.

This was introduced by a5c2f89 I'll look at what caused that. Also there is a new very weird regression with this commit:

hotbar.bug.mp4

hmmmmm yea in the future I want to invest more in testing infrastructure I think... like having a bot that can connect to the server and we can control it with functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants