Skip to content
This repository was archived by the owner on Jun 3, 2024. It is now read-only.

LivingFallEvent and PlayerFlyableFallEvent#33

Merged
coderbot16 merged 15 commits intoPatchworkMC:masterfrom
IceCryptonym:master
Feb 17, 2020
Merged

LivingFallEvent and PlayerFlyableFallEvent#33
coderbot16 merged 15 commits intoPatchworkMC:masterfrom
IceCryptonym:master

Conversation

@IceCryptonym
Copy link
Copy Markdown
Contributor

Implements fall events from #29. One thing I am uncertain about is how I should have gone about onLivingFall(), when injecting into the methods. I have to modify two local variables at once because onLivingFall() returns a float[]. If you know of a better way, I'd like to know. : )

I believe this will work, I tested it and it seems to do as expected.

Copy link
Copy Markdown
Member

@coderbot16 coderbot16 left a comment

Choose a reason for hiding this comment

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

Please fix checkstyle and use tabs instead of spaces. There's also a few things that I would like to be fixed. Thanks for contributing!

@coderbot16 coderbot16 self-requested a review January 31, 2020 04:09
Copy link
Copy Markdown
Member

@coderbot16 coderbot16 left a comment

Choose a reason for hiding this comment

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

Your implementation is spot-on, just some nitpicks to naming and javadoc.

@IceCryptonym
Copy link
Copy Markdown
Contributor Author

I apologize for the mess in commits. I'm still learning how to use git.

@shedaniel
Copy link
Copy Markdown
Contributor

Don't worry it is better than me naming commits abc

Copy link
Copy Markdown
Member

@coderbot16 coderbot16 left a comment

Choose a reason for hiding this comment

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

Thanks so much for this!

@coderbot16
Copy link
Copy Markdown
Member

Please fix licensing errors (2020) and this is good to merge

@coderbot16
Copy link
Copy Markdown
Member

Looks like merging kitlith's PR created merge conflicts, sorry. Please fix them and then I'll merge this.

Copy link
Copy Markdown
Member

@coderbot16 coderbot16 left a comment

Choose a reason for hiding this comment

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

Uh... didn't notice before, but could you restore .gitignore and remove the .vscode files? Then this will finally be ready to merge.

Copy link
Copy Markdown
Member

@coderbot16 coderbot16 left a comment

Choose a reason for hiding this comment

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

Thanks for this! 👍

@coderbot16 coderbot16 merged commit a7d1dd3 into PatchworkMC:master Feb 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants