Add pick API

[TonytheMacaroni]

Adds API to get if an entity is pickable, as well as its pick radius.

[imDaniX]

Having this on a Entity interface feels wrong. player.getPickRadius() will confuse many people into thinking it's a radius of player ability to pickup items. Also, Item already has canMobPickup() and canPlayerPickup() methods, so a having third one with such unclear naming will mess stuff up.

I'd say this better to be abstracted as an interface with getPickupRadius() method (also maybe getItemStack()?), and make related Entity interfaces extend it (I guess it's just Item and AbstractArrow right now).

[TonytheMacaroni]

I'd say this better to be abstracted as an interface with getPickupRadius() method (also maybe getItemStack()?), and make related Entity interfaces extend it (I guess it's just Item and AbstractArrow right now).

I'm a bit confused? This doesn't related to picking up items/arrows - it's related to 'picking' entities, such as when you middle click to obtain a spawn egg or attack an entity.

[DerEchtePilz]

I'd say this better to be abstracted as an interface with getPickupRadius() method (also maybe getItemStack()?), and make related Entity interfaces extend it (I guess it's just Item and AbstractArrow right now).

I'm a bit confused? This doesn't related to picking up items/arrows - it's related to 'picking' entities, such as when you middle click to obtain a spawn egg or attack an entity.

Should this be mentioned in the JavaDocs then? I actually don't know if it should, all I can say that I was confused about the exact functionality too so it might be something to think about.

[imDaniX]

I'm a bit confused? This doesn't related to picking up items/arrows - it's related to 'picking' entities, such as when you middle click to obtain a spawn egg or attack an entity.

Oh, sorry, I don't really know the internals. But honestly, then it sounds even more confusing. Even though NMS uses "pick", I think there should be a better term/wording for this, and javadocs are clearly.. unclear of what exactly the parameter is.

[TonytheMacaroni]

Attempted to clarify the javadocs a bit more.

[lynxplay]

What is the API usecase for this here ?
Can you describe this further ?

Just exposing internals for the sake if exposing seems meh, especially with such a weird concept like the "pickable" mojang fields.

[TonytheMacaroni]

Personally, was trying to to get the entities within a player's line of sight that they could potentially hit. Referenced vanilla code and it used these values, so wrote this PR to expose them.

[electronicboy]

Discussing this in voice,
regarding isPickable,

1. We have no idea what the proper intended contract here is, it just seems to be, mojang had a problem, and shoved in a hack for said problem. There is no strong server sided basis for it, "pickable" is such a tragic name in terms of the server API in terms of what it's trying to represent, isClientTargetable is maybe akin to a better name, but, it's horrible naming all around
2. This probably belongs in unsafe, as said, there is no strong contract for it

regarding the range, this is generally not a range, this is basically just a "hit box inflation scale" value

[MiniDigger]

I think in general, this needs another name, its super confusing right now

[TonytheMacaroni]

Renamed isPickable and getPickRadius to isInteractable and getInteractionRadius. Additionally, added a check for Entity#isSpectator in isInteractable since all areas that use the NMS method also check for spectator status. Expanded upon the Javadoc for isInteractable to include more examples of what an "interaction" is. Didn't go for naming similar to isClientTargetable, since it's not only used by the client. Feel free to suggest better names.

[lynxplay]

Given we have 0 idea what the mojang defined concept behind this is, exposing it as API is a meh thing.
The increadibly niche usecase makes it even less compelling.

I can see the reason you want it, but I think this would best live in UnsafeValues.
The concept can change any version, we don't have a clear definition for these values.
Exposing them to non unsafe API without a proper definition is asking for this to break down the line.

Please move this to UnsafeValues and have them take the Entity instance as param instead.