Implement Trident

[IvanCraft623]

Introduction

Implements item and trident projectile.

Changes

API changes

Projectile::onHit() now returns Vector3 representing the new projectile motion.

Behavioural changes

Projectile::onHitEntity() no longer flags the projectile for despawn, child classes decide whether the entity should be despawned.

Follow-up

Implement enchantments
Implement custom death attack message for thrown tridents

Requires translations:

TODO

Name	Value in eng.ini
death.attack.thrownTrident	{%0} was impaled to death by {%1}

Tests

https://youtu.be/y__tfdHA3FE

[ghost]

Trident enchantments?

[ghost]

@dktapps When you merged it? It's really good implementation.

I thought that it must be in PocketMine-MP

[IvanCraft623]

oh no!

[dktapps]

That's a github bug, don't worry about it.

[IvanCraft623]

I will take up this PR when I have time.

[IvanCraft623]

Trident enchantments?

Enchantments will be implemented in a separate PR:

• Loyalty
• Impaling

[dktapps]

I don't love that we're breaking BC to implement this. Is there a way to do it while retaining BC?

[IvanCraft623]

I don't love that we're breaking BC to implement this. Is there a way to do it while retaining BC?

The BC Break is to be able to modify the motion that the projectile will have when impacting a target, previously it was simply set to zero, but the trident has the fature of "bouncing" when hitting an entity. I thought of an alternative of something like getMotionOnHit(ProjectileHitEvent), but I disliked it and we already had onHit which does not have a return value.

[IvanCraft623]

In its beginnings this PR did not contain BCBreak, this was done by creating a new trident inside onHitEntity() with the desired motion. But I discarded that because it felt hacky.

[dktapps]

Seems to me like we should enable the use of setMotion in onHit, as that's the conventional way to set motion.

There's also a behavioural BC break wrt. the despawn behaviour when hitting objects. I'm not sure how we could make that non-BC-breaking.

[dktapps]

I think we can avoid BC breaks:

• For the post-hit velocity, either add a getPostHitVelocity(ProjectileHitEvent) : ?Vector3, or permit the use of setMotion() inside the onHit* handlers
• For post-hit despawning, add despawnsOnHit() : bool or perhaps an onPostHit() : void that can be overridden by subclasses.

However, the branch will need to be rebased if we retarget this back to minor-next, since there's been stuff merged into it from major-next.

The base branch was changed.

[dktapps]

Is there a reason to require this to be a TridentItem? It doesn't look like it needs any specific methods.

[IvanCraft623]

Consistency, if I do TridentEntity->getItem() I would expect a TridentItem.

[dktapps]

I feel like it would make more sense to damage the item before passing it to TridentEntity constructor instead of having to get/set it here