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

Added PlayerUseBowWithoutProjectileEvent #11668 #11669

Conversation

Chaosdave34
Copy link
Contributor

Added PlayerUseBowWithoutProjectileEvent when a player tries to load a crossbow or draw a bow without having a suitable projectile in their inventory. Makes it possible to set a projectile and enable the player to use the weapon nevertheless.

Chaosdave34 and others added 2 commits November 26, 2024 17:29

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…a crossbow or draw a bow without having a suitable projectile in their inventory. Makes it possible to set a projectile and enable the player to use the weapon nevertheless.
@Chaosdave34 Chaosdave34 requested a review from a team as a code owner November 26, 2024 16:38
Copy link
Contributor

@lynxplay lynxplay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome to paper and thank you for your first PR 🥳

Left some basic comments regarding annotations.
The PR is also missing javadocs on the API types.

Beyond that, the event name feels a bit off, the player isn't using the bow there, it is searching for a projectile. We already have some other events there so maybe we can look into merging those concepts? Not sure tho.

@Chaosdave34
Copy link
Contributor Author

Maybe PlayerTryUseBowWithoutProjectileEvent would be better?

I already thought about using the PlayerReadyArrowEvent for this.
The simplest way to implement the functionality to let a player fire a bow without having an arrow in their inventory seems to be this for me, because when creating a new ItemStack (even with CraftItemStack#asNmsCopy(), to set the arrow from the CraftItemStack used in the event), it would be diffcult to consume the arrow (removing the arrow from the player inventory) when the arrow ist not manipulated by the event.
(get the arrow -> create a craftbukkit copy -> call the event with the arrow -> the arrow is not manipulated -> get the arrow from the event -> create a nms copy -> created a new ItemStack although the original one was not changed -> Player#useAmmo() breaks; or am I missing something?)

@lynxplay
Copy link
Contributor

lynxplay commented Dec 1, 2024

No you are not missing anything I don't think.
I don't see a nice way to have this interact with player ready arrow event either no.

Naming wise, well, realistically this should probably not be limited to bows only.
It is a general "PlayerFailedProjectileSearch" or something (shitty name but you get the gist 😅) as this is going to be called for crossbows as well.

I'll try to get this PR into a pr party next weekend so I can poke some other maintainers on a better name for this, but I think the functionality this is adding is worth having 👍

@Chaosdave34 Chaosdave34 deleted the PlayerUseBowWithoutProjectileEvent branch December 22, 2024 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

None yet

3 participants