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

[Suggestion] Store the owner of the builders and use it when firing events #1020

Closed
Eufranio opened this issue Feb 1, 2018 · 6 comments
Closed

Comments

@Eufranio
Copy link

Eufranio commented Feb 1, 2018

Currently, Actually Additions gets the Minecraft Fake Player to check the drops of the block placer. By relying on the Minecraft Fake Player, other mods listening to this event won't be able to proccess the event as it would do for real players, since it doesn't store any reference to the player.

My suggestion is that the builder blocks (placer, breaker) store who placed them and use this info in the events. So, instead using the Minecraft Fake Player, it would use a custom FakePlayer with the GameProfile (or only UUID) of the player who placed the block in the event.

@canitzp
Copy link
Collaborator

canitzp commented Feb 1, 2018

But what happens if the player us offline? The we couldn't get an EntityPlayer instance an have a null player

@Eufranio
Copy link
Author

Eufranio commented Feb 1, 2018

We are talking about persistant data, you wouldn't be storing an EntityPlayer object, but a UUID (and maybe the username as well) that gets written as string or two longs two NBT. When the block asks for an FakePlayer to fire the event, build and cache a new FakePlayer instance constructed with the stored UUID and username.

@canitzp
Copy link
Collaborator

canitzp commented Feb 1, 2018

This could work (I personally nerver worked with FakePlayer before) but onky the UUID is enough, but which advantages do we get?
/Maybe this could solve claimed chunks problems/

@Eufranio
Copy link
Author

Eufranio commented Feb 1, 2018

You get a better compatibility with ANY other mods that listen to those world interaction events, including protection mods, since you will be passing the correct info instead a wildcard (like a generic FakePlayer, or the Minecraft FP).

@Ellpeck
Copy link
Owner

Ellpeck commented Feb 4, 2018

Why? All other mods that I know of using a Fake Player do it the same way Actually Additions does it: Either create a custom fake player, or use the one that Forge provides.
Either way, the class structure for Fake Players that Forge gives you makes it seem like the intended way is to make a fake player that's not bound to the player that "created" or "owns" it, so I don't really see the need for this being implemented.

@Eufranio
Copy link
Author

Eufranio commented Feb 4, 2018

Mods like BuildCraft, EnderIO, Minecolonies, Forestry, are already using the correct information with FakePlayers. Mekanism, OpenBlocks, and others will soon use as well. As I said, you dont HAVE to provide information to the fake player, but its a good pratice to, so other mods listening to those events receive the right info (and not a wildcard).

Also, if you check the FakePlayer class, you'll see the FakePlayer(WorldServer, GameProfile) constructor (which is the only constructor as well). There's also FakePlayerFactory.get(WorldServer, GameProfile), Forge just needs to reference this on its docs, since its not documented yet. Tbh, fake players at all arent, so I dont blame you...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants