[B+C] HangingType and VehicleType enums. Adds BUKKIT-5778.

[Pokechu22]

The Issue:

There are no HangingType nor VehicleType enums.

Justification for this PR:

There are several places where Hanging/Vehicle enums would be useful:

• Dealing with different Hanging/Vehicles differently in events, without having to get the EntityType.
• Easy tab-completion of commands requiring a vehicle (or hanging), such as a command that only allows specific players to use, say, horses.
  • Easy checking of valid values in such commands.

PR Breakdown:

In Bukkit:

1. Creates HangingType and VehicleType enums, containing all hangings and vehicles currently in existence.

• Constructor based off of an EntityType, meaning modifications to the base EntityType are also applied.
• Contains methods from EntityType, as well as a method to get the corresponding EntityType.
• Contains parse method from (BUKKIT-5777).

2. Added getVehicleType() method to Vehicle and VehicleEvent, and getHangingType() method to Hanging and HangingEvent.

3. Added tests for HangingType and VehicleType enums.

In CraftBukkit:

1. Added implementations of getVehicleType() and getHangingType() to the necessary classes.

Testing Results and Materials:

See org.bukkit.HangingTypeTest.java and org.bukkit.VehicleTypeTest.java. These tests cover the HangingType and VehicleType enums only, not the getters on the events or entities themselves.

Relevant PR(s):

B-1107 - #1107 - Adds a parsing method to EntityType, which I stole borrowed for use in VehicleType and HangingType. (BUKKIT-5777)

B-1109 - #1109 - This Pull Request
CB-1411 - Bukkit/CraftBukkit#1411 - CraftBukkit Counterpart of this Pull Request

JIRA Ticket:

BUKKIT-5778 - https://bukkit.atlassian.net/browse/BUKKIT-5778

EDITS:

1. Added partner pull-request for CraftBukkit.

[turt2live]

You can use EntityType or instanceof.

[caseif]

No worries, I stole borrowed the parsing method in EntityType from the Material enum.

[Pokechu22]

@turt2live

You can use EntityType or instanceof.

I understand what you mean by instanceof, but not using entitytype. One can certainly check if an entity is a hanging/vehicle with if (entity instanceof Hanging) or if (entity instanceof Vehicle), but one cannot do that with the EntityType enum. And while it is ensured that any entity that is involved in a HangingEvent or VehicleEvent is a hanging/vehicle, that doesn't mean that HangingType and VehicleType are not useful.

I don't understand how one could use EntityType in these situations. EntityType would include values that do not match valid vehicles and hangings, which means that there are values that do not match.

[turt2live]

Your situations are not very strong.

Dealing with different Hanging/Vehicles differently in events, without having to get the EntityType.

// Option 1
if(event.getEntity() instanceof Pig || event.getEntity().getType() == EntityType.PIG)

// Option 2 (for any vehicle)
if(event.getEntity() instanceof Vehicle)

Easy tab-completion of commands requiring a vehicle (or hanging), such as a command that only allows specific players to use, say, horses.

This can be done with a tab completer with some permissions and a configuration file. It's a trivial case.

I'm unsure of the full use case for this PR as this PR seems more of a unneeded convenience due to the problem scope mentioned. What is the full use case? Thus far there is no non-trivial solution that would push this PR forward.

Edit: Not to mention that an item in the world can be a vehicle.

[Pokechu22]

There is no location where this is absolutely required; it is mainly for convenience.

But one place where this is useful is checking if an EntityType is a hanging - One cannot do if (entityType instanceof Hanging). (Though one can do if (entityType.getEntityClass().newInstance() instanceof Hanging), in theory).

I don't believe there is a place where this is absolutely required - A switch statement could do most of these things, but if mojang creates a new vehicle, that's not going to be in plugins that aren't updated.

Edit: Not to mention that an item in the world can be a vehicle.

I don't get what you mean - While all entities (and I assume you mean entities and not items) do have a getPassenger function, they aren't acceptable in VehicleEvent's getVehicle method, as they don't extend vehicle.

[turt2live]

Why wouldn't entity instanceof Hanging work? Of course the entity type is not going to pass, it's an enum.

Your problem scope is still unclear and I don't think this PR solves the problem you appear to be trying to solve in a plugin that you are working on. If you are working on a plugin, I suggest you hop into #bukkitdev on IRC so many more people can try to assist. If you are not working on a plugin, then you really do need to spell out a solid use case for this PR.

[ST-DDT]

@Pokechu22
You dont have to use your way to to check whether a EntityType represents a Vehicle.
The easýies way would be:

Vehicle.class.isAssignableFrom(EntityType.MINECART.getEntityClass())

This should also be used in the tests to ensure that all EntityTypes that are Vehicles are listet in your Enums.
Although in my opinion @turt2live is right.

[bendem]

Wouldn't it just make sense to have all the entity types in the EntityType enum?

[ST-DDT]

@bendem I guess they are, aren't they? Which one is missing?
OR are you referring to their actual type? Such as vehicle

[bendem]

I was referring to that yeah, sounds kinda weird now that I think of it.
Another approach would be to add isVehicle / isHanging to EntityType.

[ST-DDT]

I guess creating this kind of methods is useless because there are so much types that have to be covered by this methods but all of them can be easyly covered by instanceof and isAssingableFrom
And thus will only spam that class.

[bendem]

True