Skip to content
This repository was archived by the owner on Nov 9, 2017. It is now read-only.

Conversation

@Aaron1011
Copy link
Contributor

This PR is the same as #340, except it's against master.

This merges cleanly, if @SpaceManiac wants it merged into master instead of scoreboards


Edited by turt2live

Related Links:

@turt2bot
Copy link

@Aaron1011 Thank you for your contribution! I didn't find a Glowkit dependency in your description. If your pull request does require a Glowkit pull request, please make a new comment with the following text (backticks, , included): GLOWKIT: 1where1` is the Glowkit pull request number.

If your pull request does not require a Glowkit pull request then there is nothing you have to do.

@jengel3
Copy link

jengel3 commented Oct 19, 2014

Shouldn't the 'tag' in nametagVisibility be capitalized?

@Aaron1011
Copy link
Contributor Author

@Jake0oo0: I think it should be left the way it is. Since nametag is one word, it should be left lowercase, as only the first letter of each word is capitalized.

@jengel3
Copy link

jengel3 commented Oct 19, 2014

Minecraft Wiki says it's two words here...I assume that's right...

@turt2live
Copy link
Contributor

It's fine as-is.

On Sun, Oct 19, 2014 at 1:45 PM, Jake notifications@github.com wrote:

Minecraft Wiki says it's two words here
http://minecraft.gamepedia.com/Name_Tag...I assume that's right...


Reply to this email directly or view it on GitHub
GlowstoneMC/Glowstone#459 (comment).

Sincerely,
Travis R

(Turt2Live http://turt2live.com/)

@turt2live turt2live mentioned this pull request Oct 19, 2014
@turt2live turt2live changed the title Scoreboards (against master) Scoreboards Oct 19, 2014
@turt2live
Copy link
Contributor

GLOWKIT: 35

@turt2bot
Copy link

@turt2live This PR has been associated with GlowstoneMC/Glowkit#35 - Scoreboards :D

@turt2live
Copy link
Contributor

@Aaron1011 Because this PR causes a huge amount of spam in the console, I do ask that your PR solve it.

It should be very clear from the following resource: click me (image)

@dequis
Copy link
Contributor

dequis commented Oct 19, 2014

Welcome!

What test plugin is that?

@turt2live
Copy link
Contributor

@dequis The first working scoreboard plugin I found on Google.

@Aaron1011
Copy link
Contributor Author

@turt2bot: That's actually totally unrelated to my PR. That scoreboards plugin is trying to lookup a player. UuidUtils attempts to fetch it from Mojang, but doesn't handle JSON parsing errors, causing the exception to propagate up.

The Skulls PR (GlowstoneMC/Glowstone#267) properly handles parse and URL related errors; we might want to switch UuidUtils over to it once it gets merged.

@turt2live
Copy link
Contributor

@Aaron1011 It's related as the scoreboard API takes in OfflinePlayers. But because Skulls is more mature and on it's way to being pulled and it contains the changes you have described, I am putting this PR on hold until that one is pulled.

PRHOLD: Waiting for #267

@dequis
Copy link
Contributor

dequis commented Nov 5, 2014

Referencing #81 because all the PRs linked from that ticket are closed. Don't mind me.

@dequis dequis mentioned this pull request Nov 5, 2014
@turt2live
Copy link
Contributor

Hold cleared: Skulls have been pulled. Pending further review.

@turt2live
Copy link
Contributor

@Aaron1011 As a warning, your pull request has checkstyle failures. Although these do not affect the pull request process, it is strongly recommended to fix them. The failures are outlined on the Travis-CI build as well as below.

GlowWorld.java:17:8: Unused import - net.glowstone.io.ScoreboardIoService.
GlowWorld.java:18:1: Duplicate import to line 10 - net.glowstone.io.WorldMetadataService.WorldFinalValues.
GlowWorld.java:19:1: Duplicate import to line 11 - net.glowstone.io.WorldStorageProvider.
GlowWorld.java:20:1: Duplicate import to line 12 - net.glowstone.io.anvil.AnvilWorldStorageProvider.

deathcap referenced this pull request in GlowstoneMC/Glowstone May 9, 2015
@turt2live turt2live removed their assignment Feb 7, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants