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

Add flight speed #6076

Open
wants to merge 17 commits into
base: minor-next
Choose a base branch
from
Open

Conversation

Sergittos
Copy link

@Sergittos Sergittos commented Oct 7, 2023

Introduction

This pull request makes it possible for developers to modify a player's flight speed.

Relevant issues

Closes #5155

Changes

API changes

Added the following methods in Player:

  • Player->setFlightSpeed(): Sets the player's speed when flying.
  • Player->getFlightSpeed(): Returns the player's speed when flying.

Added the following constant in Player:

  • Player::DEFAULT_FLIGHT_SPEED

Tests

https://streamable.com/vctb3y

@dktapps
Copy link
Member

dktapps commented Oct 8, 2023

My issue with this is the same as the past PRs on the subject. There's no explanation or documentation of what the values mean, or why it's 0.05 by default. What's the unit? Blocks per seconds? Blocks per tick?

There's also going to be questions like why doesn't setMovementSpeed() affect flying speed, and why the server-side flight speed doesn't change when sprint-flying (which is inconsistent with movement speed).

The name setFlightSpeed() would be more in line with functions like setMovementSpeed().

@ShockedPlot7560
Copy link
Member

I also think the name isn't quite right. It will indeed be confusing. However, I don't think that the unity problem should hinder the merge of adding APIs. It's up to the developers to find out what they're modifying, and a warning could be added in the docs about this. That the unit is not determined and that the value should be handled with care?

@ShockedPlot7560 ShockedPlot7560 added Category: API Related to the plugin API Type: Enhancement Contributes features or other improvements to PocketMine-MP labels Dec 16, 2023
@Sergittos Sergittos changed the title Add fly speed Add flight speed Jan 27, 2024
Copy link
Member

@ShockedPlot7560 ShockedPlot7560 left a comment

Choose a reason for hiding this comment

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

Looks ok to me

src/player/Player.php Outdated Show resolved Hide resolved
Negative values make the player fly backwards, and this can be used in some game modes
@IvanCraft623
Copy link
Member

Apparently with tests I have done $value * 10 are the blocks per tick that the player will fly. So the default value 0.05 sets the player's flight speed to around 0.5 blocks per tick.

I've been testing if other attributes like movementSpeed or walkSpeed can modify the flight speed, but that doesn't happen. Alosi it seems that walkSpeed only modifies the player's fov.

When a player sprints while flying his speed is duplicated, with the default value the player's flight speed will be around 1 block per tick. The curious thing is that this only happens in client-side.

@TobiasGrether
Copy link
Member

@Sergittos @ShockedPlot7560 I think this PR is fine once documentation on the value of the FlySpeed is present. As in, an explanation as to how the values are calculated.

@UnknownOre
Copy link
Contributor

Apparently with tests I have done $value * 10 are the blocks per tick that the player will fly. So the default value 0.05 sets the player's flight speed to around 0.5 blocks per tick.

I've been testing if other attributes like movementSpeed or walkSpeed can modify the flight speed, but that doesn't happen. Alosi it seems that walkSpeed only modifies the player's fov.

When a player sprints while flying his speed is duplicated, with the default value the player's flight speed will be around 1 block per tick. The curious thing is that this only happens in client-side.

if this correct can't we use blocks it can travel per tick as a parameter in the function instead of magic numbers

@ShockedPlot7560
Copy link
Member

If @IvanCraft623 researchs is correct, we can document the method with this and it will be ready to merge for me.

@Sergittos
Copy link
Author

@Sergittos @ShockedPlot7560 I think this PR is fine once documentation on the value of the FlySpeed is present. As in, an explanation as to how the values are calculated.

Done.

ShockedPlot7560
ShockedPlot7560 previously approved these changes Aug 3, 2024
Copy link
Member

@ShockedPlot7560 ShockedPlot7560 left a comment

Choose a reason for hiding this comment

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

Documentation is enough for me.

src/player/Player.php Show resolved Hide resolved
Joshy3282 added a commit to Joshy3282/PocketMine-MP that referenced this pull request Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: API Related to the plugin API Type: Enhancement Contributes features or other improvements to PocketMine-MP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants