Open m3rcuriel opened 9 years ago
turt2live
commented
9 years ago The fix extends to a larger problem: The player profile is incomplete.
What would ideally occur is instead of working off of name, the UUID is still used but an OfflinePlayer is spun up using that player profile as it's backing object (not sure if this can be done right now though). This would make Glowkit unchanged and make this so it's not all names.
m3rcuriel
commented
9 years ago Mind if I change this to WIP and start looking at that? I was looking at that initially but it's a bit of a mess.
turt2live
commented
9 years ago WIP is generally only used if you are about to radically change the scope of your PR. "Needs changes" is arguably okay for now.
turt2live
commented
9 years ago @m3rcuriel The large amount of changes is not quite what I meant when talking to you on IRC about this. Instead of rewriting OfflinePlayer to comply with a PlayerProfile, write it to support the PlayerProfile. This means a new constructor and other applicable changes need to be made to avoid breaking everything on the planet. The new constructor would be used where applicable leaving the other two to be used where they are needed.
turt2live
commented
9 years ago @m3rcuriel this is no longer mergeable, please resolve the issue as soon as possible, thanks!
turt2live
commented
9 years ago @m3rcuriel You have several additional problems to the ones commented on above:
Please resolve the concerns as soon as possible.
turt2live
commented
9 years ago @m3rcuriel Your pull request is no longer mergeable due to recent commits on the master branch. Additionally, there are several concerns outlined in my previous comment. Please address all concerns as soon as possible, thanks.
m3rcuriel
commented
9 years ago Okay so it's now mergable and I think I fixed everything, however during testing I ended up surpassing the profile requests to the Mojang API so I'll finish testing in the morning and push.
Edit: API cooled down.
turt2live
commented
9 years ago @m3rcuriel Once again unmergeable :(
turt2live
commented
9 years ago @m3rcuriel This pull request is considered unmergeable as of yesterday. Please correct the issue for further review (also please ping me once it is mergeable so that I can continue to review it). Thanks.
m3rcuriel
commented
9 years ago Yeah. Sorry, I'll get it done Wednesday. Busy week.
turt2live
commented
9 years ago @m3rcuriel Wednesday has past :(
turt2live
commented
9 years ago @m3rcuriel This pull request is not mergeable. Please resolve the issue for further review.
turt2live
commented
9 years ago @m3rcuriel This is a final warning: Your pull request is not mergable. Please resolve the issue for further review.
m3rcuriel
commented
9 years ago @turt2live So sorry - jumped right back in. I lost my IntelliJ liscense and this PR slipped through the cracks. I found a slight problem with it, but I should have it done tomorrow
crosses fingers
m3rcuriel
commented
9 years ago @turt2live trying to keep you updated - there's a weird bug I haven't resolved but I'll work more tomorrow.
m3rcuriel
commented
9 years ago @turt2live I'm not going to develop this any more until even a single PR is pulled into master. I hit a snag and couldn't fix it and I'm not willing to invest into a project not being actively updated. Feel free to close/remove this if you need to. I'll keep an eye though and come back if this project starts getting commits again. A good possibility for something to pull is my other PR, #521, which has been ready to pull for a good 8 months. I know you're waiting on SpaceManiac, and I understand that. Wish you luck.
deathcap
commented
8 years ago @m3rcuriel I've merged your PR into Glowstone++, can you elaborate on the weird bug? Glowstone++ might have it
This PR fixes approximately half of issue #523. Players no longer show up as null in whitelist list, and non-existent players no longer print huge errors.