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

Trackables - Locations everywhere! #5798

Closed
wants to merge 1 commit into from

Conversation

TheFruxz
Copy link
Contributor

@TheFruxz TheFruxz commented Jun 11, 2021

Trackables, Locations everywhere

The Introduction

Hey!
During my development of Paper, Spigot and Bukkit plugins I came across worlds, entities, blocks and their locations very often. Whenever you want to use their locations for something like a function or a setter, you first have to take the object (e.g. the entity) and then use a separate sub-function to retrieve the location.

Example:

public void setParticleLocation(ArmorStand armorStand) {
   setLocationOfParticle(armorStand.getLocation())
}

My idea now would have been to make the complete matter unnecessary for the developer by giving the developer directly the possibility to pass the position, if this would be logical in a case. But how did I think of this? This is what comes now!

The IDEA

How to make it universal at every corner where you can throw in a location, so that you don't have to access the entity or block first and then manually grab the location first?

My solution is to make every object that contains a location and is therefore "traceable", as such an object and in (pretty much) every function where you need / use a location as a parameter, where the location is then extracted in the background code, without the developer has to do it again and again.

Example:

setLocationOfParticle(armorStand)

The Code

But how does it work?

The trackable itself

Every object that has a .getLocation() method, e.g. an entity, block, etc. gets a new interface, named Trackable, which holds the .getLocation() method, which is then overridden by e.g. the entity and the block and thus is universal for every object that has a location.

The functions

Every function which supports a location as input, e.g. the World#playSound(...) function gets a duplicate of that function which has the same input parameters as the original function, only the (Location location, ...) parameter is replaced with the trackable, i.e. (Trackable trackable, ...). And this is best done on all functions with location parameters, so that a universal & seamless development-experience is created!

My Reason

My reason for doing / proposing all this is to create a transitionless development experience and thus allow the developer to focus on the essential things without limiting or even removing the previous possibilities.

It should help a developer and take away work, saving him little time and effort on the individual, but relatively much on the whole.

The Work

Of course I would try very hard to make the changes myself, however I would extremely appreciate feedback of the current and upcoming situation and development support.
Especially since it would be my first major paper feature, I would appreciate any support and suggestions.

INFO

The branch pointing to this PR is initially only equipped with a PlaceHolder file, that would then of course change significantly in the course of development (...):

The Roadmap

This can be modified during feedback and corrections, also please be carefull reading that :D

  • Creating an Trackable interface, with the needed functions
  • Adding the Trackable interface to objects with locations, like blocks and entitys
  • Creating duplicated location-parameter functions with support for Trackable objects
  • Eat an cookie and look, if something went wrong

Thanks!

Hey, thank you for reading, please excuse the a bit broken English, I had this partially translated.
As said, if you have any suggestions or opinions, feel free to write it in, this is an idea of mine that I would very much like to take forward with the paper community and the paper developers.

Stay safe!

@theomega24
Copy link
Contributor

christ almighty

@Owen1212055
Copy link
Member

Although the aesthetic of this pull request looks very nice, I feel that the benefit that is provided here is very little. Every single method that uses a location would need to be converted to this new "Trackable" interface whilst only saving a singular method.

If you really need an "easy" way to get a location you just can do something like this.

        public static void thing(Entity entity) {
		thing(entity.getLocation());
	}
	
	public static void thing(Block block) {
		thing(block.getLocation());
	}
	
	public static void thing(Location location) {
	}

Additionally, not all things that have a "location" can be represented as a org.bukkit.Location. For example, chunks have a "location" but this isn't really representable by that object. Just my input on this, and I'd be interested how others think.

@imDaniX
Copy link

imDaniX commented Jun 12, 2021

I kinda like the idea. As mentioned earlier, there's not much benefit, but it's rather a pleasant feature to have. But I feel like it's gonna be an annoying task to update it with upstream API changes.

@qixils
Copy link
Contributor

qixils commented Jun 12, 2021

i have a fork of paper originally created to add interfaces like this- HasUniqueId, HasPlayer, etc etc. it reduced how often we needed to add a ton of overloads in our plugin like in Owen's example, as we have a lot of custom player/entity classes (database objects, minigame players, etc.) which would each need overloads.

using interfaces like this allows reducing the overloads to just 2 (example: one for UUID, one for HasUniqueId) and is used everywhere in Adventure: Component+ComponentLike and Identity+Identified, along with similar interfaces Translatable, HoverEventSource, PermissionChecker, etc.

i definitely support more interfaces, though i do also wonder how well Adventure's new Pointer class could help with this. there's a draft PR for it at #5708 though it's currently a bit underutilized imo. it could potentially be expanded to include pointers for PLAYER, LOCATION, etc. then plugin developers would be able to do something like:

public static void thing(Location location) {
    location.setY(-100);
}
public static void thing(Pointered pointered) {
    thing(pointered.get(Location.POINTER).orElseThrow(() -> new IllegalArgumentException()))
}

my main issue with this is it is very non-obvious what this overload does from a glance, you'd have to rely on javadocs to explain that it's looking for a location

@BillyGalbreath
Copy link
Contributor

Wowzers. Love the presentation here. I could definitely use a PR guy over on my team (public relations. not code since, well.. you seem to be lacking that). ;)

@Proximyst
Copy link
Contributor

I agree with Lexi, #5708 would work better for this (among a lot of other sharable properties).

@TheFruxz
Copy link
Contributor Author

I agree

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

Successfully merging this pull request may close these issues.

None yet

7 participants