Player NPC quits riding on skin update #992

Open
liec0dez opened this Issue Dec 1, 2016 · 10 comments

Projects

None yet

3 participants

@liec0dez
Contributor
liec0dez commented Dec 1, 2016 edited

Citizens: 2.0.21 Snapshot
Spigot: 1.10.2

When setting a player NPC as passenger of any vehicle before it's skin (based on nick) was retrieved, it will get thrown off as soon as the skin is displayed.

Reproducable w/ following snippet:
Minecart cart = (Minecart) Bukkit.getWorld("world").spawnEntity(player.getLocation(), EntityType.MINECART);
NPC npc = CitizensAPI.getNPCRegistry().createNPC(EntityType.PLAYER, "Notch");
npc.spawn(player.getLocation());
cart.setPassenger(npc.getEntity());

var player is CommandSender

@mcmonkey4eva
Member

Yup, Citizens does not pay attention to mounting atm, though it probably should!

@fullwall
Member
fullwall commented Dec 2, 2016

Latest build makes mount restoration better - only for Minecart NPCs though (not ordinary bukkit entities!)

@liec0dez
Contributor
liec0dez commented Dec 2, 2016

@fullwall

Latest build makes mount restoration better - only for Minecart NPCs though (not ordinary bukkit entities!)

Would be great to have the pathfinder deal with mounting properly, gotta get those NPCs riding horses and attack things :)

The method "updateMountedStatus()" in CitizensNavigator is kind of supposed to set the vehicles target the same as the rider's, correct? But how do I get the corresponding NPCHolder for an Entity or NPC? My NMS.getVehicle(npc.getEntity()) never return as instanceof NPCHolder, which means that this method is not doing anything.

@fullwall
Member
fullwall commented Dec 2, 2016 edited

@liec0dez from your sample above, you used:
(Minecart) Bukkit.getWorld("world").spawnEntity(player.getLocation(), EntityType.MINECART);
where Citizens is looking for
CitizensAPI.getNPCRegistry().createNPC(EntityType.MINECART, "").spawn(player.getLocation());

NPCHolder is just an interface that all NPCs implement in the Bukkit side.

@liec0dez
Contributor
liec0dez commented Dec 2, 2016 edited

@fullwall That's true, was not referring to my sample though. Tested that with a horse NPC which had a player NPC as passenger... Never returned as instance of NPCHolder :/

@fullwall
Member
fullwall commented Dec 3, 2016

Hmm.. so you are setting the horse's target or the player's target?

@liec0dez
Contributor
liec0dez commented Dec 3, 2016

Those were the tests I ran:

  • Only horse targeting

  • Only player targeting

  • Both targeting

  • None targeting

Not a single time NMS.getVehicle(horsenpc.getEntity() or NMS.getVehicle(player.getEntity() were an instanceof NPCHolder. How could it be anyways? Since

public org.bukkit.entity.Entity getVehicle(org.bukkit.entity.Entity entity) in NMSImpl.java returns a CraftEntity which does not inherit an NPCHolder. You would need to check if the CraftEntity is an NPC and then return that.

@fullwall
Member
fullwall commented Dec 3, 2016 edited

@liec0dez See here for example: https://github.com/CitizensDev/Citizens2/blob/master/v1_11_R1/src/main/java/net/citizensnpcs/nms/v1_11_R1/entity/BatController.java#L32 it should be an NPCHolder which is why I'm confused. Also, it returns an NPCHolder for me when I try it.

@liec0dez
Contributor
liec0dez commented Dec 3, 2016 edited

@fullwall So I just added this broadcast to updateMountedStatus() in CitizensNavigator.java
if (!(vehicle instanceof NPCHolder)) {
Bukkit.getServer().broadcastMessage(vehicle.getType()+" No NPCHolder");
return;
}
and ran this code:
NPC horsenpc = CitizensAPI.getNPCRegistry().createNPC(EntityType.HORSE, "Horse");
horsenpc.spawn(p.getLocation());
NPC playernpc = CitizensAPI.getNPCRegistry().createNPC(EntityType.PLAYER, "Rider");
playernpc.spawn(p.getLocation());
horsenpc.getEntity().setPassenger(playernpc.getEntity());
playernpc.getNavigator().setTarget(p, true);

That resulted in "HORSE No NPCHolder" appearing in chat on every tick. How and what did you test to have an instance of NPCHolder?

@liec0dez
Contributor
liec0dez commented Dec 3, 2016

@fullwall Additionally, NMS.getVehicle returns null if I replace EntityType.HORSE with EntityType.BAT

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