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.

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

UUID lookups do not thread #111

Closed
DefinitlyEvil opened this issue Dec 2, 2015 · 62 comments
Closed

UUID lookups do not thread #111

DefinitlyEvil opened this issue Dec 2, 2015 · 62 comments

Comments

@DefinitlyEvil
Copy link

@DefinitlyEvil DefinitlyEvil commented Dec 2, 2015

Is it possible to fetch player's UUID in another thread? It hangs the server when typing in a random username.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@mastercoms
Copy link
Member

@mastercoms mastercoms commented Dec 2, 2015

Yep, totally possible, but I'm rewriting Glowstone++, so someone else will have to do it.

@blakegall
Copy link
Contributor

@blakegall blakegall commented Dec 2, 2015

Wait, @mastercoms you're completely rewriting glowstone++?

@ethsmith
Copy link

@ethsmith ethsmith commented Dec 2, 2015

Woot!

@mastercoms
Copy link
Member

@mastercoms mastercoms commented Dec 2, 2015

Yep, JSON based item definitions, tickless server, and the server is based on events, services, and modular components.

@blakegall
Copy link
Contributor

@blakegall blakegall commented Dec 2, 2015

@mastercoms That sounds awesome! Are you coding that in your own fork?

@mastercoms
Copy link
Member

@mastercoms mastercoms commented Dec 2, 2015

I am coding it in a new repository, on my computer. Once it is fairly done, I will upload it.

@kamcio96
Copy link
Member

@kamcio96 kamcio96 commented Dec 3, 2015

Will it implement SpongeAPI?

@ethsmith
Copy link

@ethsmith ethsmith commented Dec 3, 2015

I don't see why not, Sponge API support was one of the features of G++ and a recode shouldn't get rid of that idea.

@DefinitlyEvil
Copy link
Author

@DefinitlyEvil DefinitlyEvil commented Dec 3, 2015

@mastercoms Tickless will be crazy, how do you synchronize objects?

@ethsmith
Copy link

@ethsmith ethsmith commented Dec 3, 2015

I wonder the same thing but I'm sure whatever idea he has it's a good one

@DefinitlyEvil
Copy link
Author

@DefinitlyEvil DefinitlyEvil commented Dec 3, 2015

@TekkitCommando Many years ago people thought about multi-threaded server it means no ticks but they are unable to do that.

@kamcio96
Copy link
Member

@kamcio96 kamcio96 commented Dec 3, 2015

Ofc it is possible 😄

@DefinitlyEvil
Copy link
Author

@DefinitlyEvil DefinitlyEvil commented Dec 3, 2015

@kamcio96 Okay. >~<

@mastercoms
Copy link
Member

@mastercoms mastercoms commented Dec 3, 2015

It will be like the LInux tickless kernel. For things that can't use the new system to do repeatable actions using services and events, Glowstone++ will only tick those actions when necessary.

And yes, I am basing the recode off of Sponge, though right now it is a lot of non API code, and some Bukkit code I am planning to replace with Sponge. I am not sure if I should use SpongeVanilla/Common, or make a new base from scratch. What do you guys think? I am also experimenting with using both Bukkit and Sponge. A lot of things are in flux right now.

@DefinitlyEvil
Copy link
Author

@DefinitlyEvil DefinitlyEvil commented Dec 3, 2015

Awesome!

@blakegall
Copy link
Contributor

@blakegall blakegall commented Dec 3, 2015

I might make a Linux build designed to run a minecraft server

@DefinitlyEvil
Copy link
Author

@DefinitlyEvil DefinitlyEvil commented Dec 3, 2015

@codekilla747 Omg!

@kamcio96
Copy link
Member

@kamcio96 kamcio96 commented Dec 3, 2015

@mastercoms
"base from scratch" was Glowstone point ;)
Do you have idea how to use both Bukkit and SpongeAPI?
I suggest to write it only in SpongeAPI and then write a bridge (SpongeAPI has more events);

@DefinitlyEvil
Copy link
Author

@DefinitlyEvil DefinitlyEvil commented Dec 3, 2015

@kamcio96 You mean only support Sponge plugin and make a Bukkit bridge to Sponge?

@mastercoms
Copy link
Member

@mastercoms mastercoms commented Dec 3, 2015

@kamcio96
Copy link
Member

@kamcio96 kamcio96 commented Dec 3, 2015

@DefinitlyEvil yes
@mastercoms so we can compile that one class 😈

@gabizou
Copy link
Contributor

@gabizou gabizou commented Dec 3, 2015

@kamcio96 There are a few things missing from SpongeAPI, like https://github.com/SpongePowered/SpongeCommon/blob/master/src/main/java/org/spongepowered/common/ProtocolMinecraftVersion.java which is in SpongeCommon

��Sorry, but why is it necessary to expose the protocol version in an API?

@mastercoms
Copy link
Member

@mastercoms mastercoms commented Dec 3, 2015

@gabizou Not saying it should be in the API, just saying we need it for our project, and it is missing if we only use Sponge API.

@kamcio96 It just seems like it is a better idea to fork SpongeVanilla, and put our stuff in it, and help them finish it, instead of rewriting everything. If we write it from scratch. we should at least push some changes to them (of course adapted for SpongeVanilla).

@kamcio96
Copy link
Member

@kamcio96 kamcio96 commented Dec 3, 2015

@mastercoms So now I don't understand you. You want fork SpongeVanilla and do what? Glowstone is alone project. Adding minecraft code is pointless.

@ethsmith
Copy link

@ethsmith ethsmith commented Dec 3, 2015

@mastercoms yes sponge vanilla is the best idea by far! Not worth making from scratch

@gabizou
Copy link
Contributor

@gabizou gabizou commented Dec 3, 2015

Not saying it should be in the API, just saying we need it for our project, and it is missing if we only use Sponge API.

That summarizes what is commonly known as "implementation detail." If you need it for implementation, then it's included in the implementation. I don't see much of a problem since the API doesn't have a care in the world about it.

t just seems like it is a better idea to fork SpongeVanilla

Also, this transitively means forking SpongeCommon as a majority of the implementation resides there (SpongeCommon exists as a buffer of the "common" implementation that can be shared between both Vanilla and Forge). In this context however, it explicitly is using Minecraft's objects as much as possible to piggy back not having to write new objects/implementation.

@kamcio96
Copy link
Member

@kamcio96 kamcio96 commented Dec 3, 2015

@gabizou you are right
@TekkitCommando why making from scratch is bad idea? one mojang change (eg protect theirs files) and we have to write it again. And we can add more and more functions to server and plugins.

@Tzky
Copy link

@Tzky Tzky commented Dec 3, 2015

I suggest to write it only in SpongeAPI and then write a bridge (SpongeAPI has more events);

There already is an incomplete bridge between Bukkit and SpongeAPI. It's a Sponge plugin named Pore which implements BukkitAPI on top of SpongeAPI. However it's a) outdated and b) incomplete.

@ethsmith
Copy link

@ethsmith ethsmith commented Dec 3, 2015

@kamcio96 Not that it's a bad idea, I mean G++ and Glowstone were founded on from scratch code. I am saying that if some of the features mastercoms wants is already present in SpongeVanilla why not use it. I didn't know that Sponge uses Minecraft code. I thought they were doing things from scratch as well

@kamcio96
Copy link
Member

@kamcio96 kamcio96 commented Dec 3, 2015

@TekkitCommando unfortunately not :/

@mastercoms
Copy link
Member

@mastercoms mastercoms commented Feb 17, 2016

Will be fixed with #205

@mastercoms mastercoms reopened this Feb 17, 2016
@kamcio96
Copy link
Member

@kamcio96 kamcio96 commented Feb 17, 2016

GlowServer has OfflinePlayer getOfflinePlayer(String name). It isn't possible to do it in that method.
We should add something like void getOfflinePlayer(String name, PlayerLookupCallback cb).

@mastercoms
Copy link
Member

@mastercoms mastercoms commented Feb 17, 2016

Yes.

@mastercoms mastercoms closed this Feb 24, 2016
@mastercoms mastercoms reopened this Feb 24, 2016
@mastercoms mastercoms added this to the 2016Q2 milestone Jun 7, 2016
@gdude2002 gdude2002 closed this Jun 22, 2016
@mastercoms mastercoms reopened this Jun 23, 2016
@gdude2002
Copy link
Member

@gdude2002 gdude2002 commented Jun 23, 2016

#205 was merged, does that not fix this ticket?

@mastercoms
Copy link
Member

@mastercoms mastercoms commented Jun 23, 2016

No.

mastercoms added a commit that referenced this issue Sep 18, 2016
@mastercoms
Copy link
Member

@mastercoms mastercoms commented Sep 19, 2016

@gdude2002 Player lookup is still blocking.

@aramperes aramperes removed this from the 2016Q3 milestone Jan 7, 2017
@gdude2002 gdude2002 added this to Backlog in March 2017 Feb 27, 2017
@gdude2002 gdude2002 moved this from Backlog to Stalled in March 2017 Feb 27, 2017
@gdude2002 gdude2002 moved this from Stalled (Little activity since '16) to Backlog in March 2017 Mar 10, 2017
@gdude2002 gdude2002 added this to Backlog in April 2017 Apr 1, 2017
@gdude2002 gdude2002 removed this from Backlog in March 2017 Apr 1, 2017
@gdude2002 gdude2002 removed this from Backlog in April 2017 May 4, 2017
@gdude2002 gdude2002 added this to Backlog in May 2017 May 4, 2017
@kaenganxt
Copy link
Member

@kaenganxt kaenganxt commented Dec 28, 2017

#604 partially fixed this issue. API methods for non-blocking lookups are now provided (e.g. GlowServer#getOfflinePlayerAsync(String name), which returns a CompletableFuture<OfflinePlayer>).

Internal lookups (for example the whitelist or op command) still use blocking calls but can be replaced in the future.

@ethsmith
Copy link

@ethsmith ethsmith commented Dec 28, 2017

Holy crap this is old. Back when this was made, Deathcap was still working on Glowstone++ and I was still a member of the G++ org.

@aramperes
Copy link
Member

@aramperes aramperes commented Jan 18, 2018

The main issue I see with migrating commands to use the non-blocking methods is the command return values. Command handlers are called synchronously and expect a boolean return value (for success/failure).

This isn't a big concern because the return value isn't used as of now, but we may need to use them for command blocks. Should we just ignore this problem for now, and extend the command API later for asynchronous support?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
May 2017
Backlog
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.