Skip to content

send-info-after-death #3032

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
Closed
wants to merge 1 commit into from
Closed

send-info-after-death #3032

wants to merge 1 commit into from

Conversation

kaanakduman
Copy link
Contributor

@kaanakduman kaanakduman commented Feb 24, 2020 •
edited by pop4959
Loading

A possibly fixed version of #2912. I'm not very skilled in coding, but here's my best shot on making the suggested changes. I couldn't have done this without seeing the PR from @darbyjack . I'm not sure if this is the standard way of modifying a PR, sorry if it's not.

Closes #2896, closes #3207

@kxa317
send-info-after-death
b87981a 
A possibly fixed version of #2912. I'm not very skilled in coding but here's my best shot on making the suggested changes. I couldn't have done this without seeing darbyjack's PR. I'm not sure if this is the standard way of modifying a PR, sorry if it's not.
@pop4959 pop4959 requested a review from darbyjack February 24, 2020 01:31
@pop4959
Copy link
Member

pop4959 commented Feb 24, 2020

I don't know if there is a convenient way to modify other people's pull requests without being a project maintainer. This is probably fine, but I think @darbyjack should at least have a chance to take a look at this since he has the original pull request still open.

As far as I can tell, it seems all of the suggested fixes were applied appropriately though. Looks good.

@pop4959 pop4959 added the type: enhancement Features and feature requests. label Feb 24, 2020
@kaanakduman
Copy link
Contributor Author

Thanks pop! Could you help me understand something? Why did the check fail for me when it passed for the other PR? Also, how come you approved the changes even though the check failed?

@pop4959
Copy link
Member

pop4959 commented Feb 24, 2020

@Kakd The build failed because Spigot added new methods to interfaces which Essentials is using in some of its implementation / testing framework. If you look at the details of the build you will see this:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.0:compile (default-compile) on project EssentialsX: Compilation failure
[ERROR] /home/runner/work/Essentials/Essentials/Essentials/src/com/earth2me/essentials/OfflinePlayer.java:[41,8] com.earth2me.essentials.OfflinePlayer is not abstract and does not override abstract method swingOffHand() in org.bukkit.entity.LivingEntity

Someone will just have to run a re-build of this PR once those methods are added. Nothing needed on your part.

@pop4959
Copy link
Member

pop4959 commented Mar 5, 2020

@Kakd Build fails for a new reason now. Apparently it isn't able to check out from your repo anymore. I'm not sure why that would be the case but you should probably check on that.

reference is not a tree: b87981a785670f5daf6f5b8308dbf998206d0996
Git checkout failed with exit code: 128

Ichbinjoe
Ichbinjoe requested changes Mar 31, 2020
View reviewed changes
Essentials/src/com/earth2me/essentials/EssentialsEntityListener.java
event.setDeathMessage("");
}
if (ess.getSettings().infoAfterDeath()) {
user.sendMessage(tl("infoAfterDeath", loc.getWorld(), loc.getBlockX(), loc.getBlockY(), loc.getBlockZ()));
Copy link
Member

Choose a reason for hiding this comment

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

This should be log.getWorld().getName or something like that

@Ichbinjoe Ichbinjoe added the status: waiting on author Pull requests that require changes from the author in order to merge. label Mar 31, 2020
@SrBedrock
Copy link
Contributor

@Kakd

kaanakduman pushed a commit to kaanakduman/Essentials that referenced this pull request Apr 29, 2020
@kxa317
I think I know what caused the problem with EssentialsX#3032, but I c…
42c7a2a 
…ouldn't figure out how to fix it without making a new pull request, so here's that.

Closes EssentialsX#2896
@kaanakduman kaanakduman mentioned this pull request Apr 29, 2020
Merged
@darbyjack
Copy link
Member

Closing this one as you've recreated a new one.

@darbyjack darbyjack closed this Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ichbinjoe Ichbinjoe Ichbinjoe requested changes

@pop4959 pop4959 pop4959 approved these changes

@darbyjack darbyjack Awaiting requested review from darbyjack

Assignees

No one assigned

Labels

status: waiting on author Pull requests that require changes from the author in order to merge. type: enhancement Features and feature requests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Coords in Death Message

6 participants

@kaanakduman @pop4959 @SrBedrock @darbyjack @Ichbinjoe @kxa317