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.

Sign up for GitHub

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

Jump to bottom

Create ExprFallenDistance.java #3158

Merged
merged 12 commits into from
Jul 20, 2020
Merged

Create ExprFallenDistance.java #3158

merged 12 commits into from
Jul 20, 2020

Conversation

fednelpat
Copy link
Contributor

@fednelpat fednelpat commented Jul 17, 2020 •
edited by ShaneBeee
Loading

Description

You can get an entity's fallen distance using ENTITY's fall(en|ed|) distance (using the Bukkit Entity.getFallDistance() and Entity.setFallDistance(Float) methods)

Target Minecraft Versions: any
Requirements: none
Related Issues: none

ShaneBeee
ShaneBeee suggested changes Jul 17, 2020
View reviewed changes
Copy link
Contributor

@ShaneBeee ShaneBeee left a comment

Choose a reason for hiding this comment

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

A couple things to change up

src/main/java/ch/njol/skript/expressions/ExprFallenDistance.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/expressions/ExprFallenDistance.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/expressions/ExprFallenDistance.java Outdated Show resolved Hide resolved
fednelpat and others added 2 commits July 18, 2020 10:38
ShaneBeee
ShaneBeee suggested changes Jul 19, 2020
View reviewed changes
Copy link
Contributor

@ShaneBeee ShaneBeee left a comment

Choose a reason for hiding this comment

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

Just a few small requests.
This is mainly personal opinion on my behalf, so Im open for discussion.

src/main/java/ch/njol/skript/expressions/ExprFallenDistance.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/expressions/ExprFallenDistance.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/expressions/ExprFallenDistance.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/expressions/ExprFallenDistance.java Outdated Show resolved Hide resolved
@fednelpat fednelpat requested a review from ShaneBeee July 19, 2020 12:31
@fednelpat
Copy link
Contributor Author

fednelpat commented Jul 19, 2020 •
edited
Loading

Yeah you're right, I updated it to just 'fall[en] (distance|height)'

ShaneBeee
ShaneBeee suggested changes Jul 20, 2020
View reviewed changes
Copy link
Contributor

@ShaneBeee ShaneBeee left a comment

Choose a reason for hiding this comment

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

Looks good... just a couple little stylistic changes and I think this is good to go.

src/main/java/ch/njol/skript/expressions/ExprFallenDistance.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/expressions/ExprFallenDistance.java Outdated Show resolved Hide resolved
@fednelpat fednelpat requested a review from ShaneBeee July 20, 2020 10:04
ShaneBeee
ShaneBeee approved these changes Jul 20, 2020
View reviewed changes
Copy link
Contributor

@ShaneBeee ShaneBeee left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@Wealthyturtle Wealthyturtle dismissed stale reviews from Pikachu920 and FranKusmiruk July 20, 2020 10:28

Changes Applied

@Wealthyturtle Wealthyturtle merged commit 7b68e48 into SkriptLang:master Jul 20, 2020
@fednelpat fednelpat deleted the ExprFallenDistance branch July 20, 2020 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pikachu920 Pikachu920 Pikachu920 left review comments

@FranKusmiruk FranKusmiruk FranKusmiruk left review comments

@Wealthyturtle Wealthyturtle Wealthyturtle approved these changes

@ShaneBeee ShaneBeee ShaneBeee approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@fednelpat @Wealthyturtle @ShaneBeee @Pikachu920 @FranKusmiruk