Fixes/requirements before charge #17
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rather than triggering our event from EntityDamageEvent then checking if it is cancelled in the same handler, properly listen to EggCaptureEvent and handle it there Added more explicit requirement checks/flow for triggering the EggCaptureEvent
Added playerHasRequirements helper to check if the egg catching should succeed Remediated a number of deprecated calls Removed suppression of remaining deprecation as these should be highlighted This fixes an issue where all requirements have to be met before applying others EG when using both vault cost and item cost, the vault cost would be taken whether or not the player had the required item cost to pay with
|
Good work. Thanks for the pull request :) I need some more time to review and test this, but from a quick look it looks good. |
|
Curious as to why this isn't pulled yet? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change fixes an issue where a player would be partially charged for a catch even if they didn't have the rest of the requirements to catch a mob.
For example, when using both vault cost and item cost, the vault cost would be taken away even if the player didn't have the required items. To fix this, I added a requirements check first, then charge any/all requirements once the catch is known to be successful.
Additionally, I encapsulated the egg capture inside an egg capture event listener, rather than triggering the event, checking for cancelled, then proceeding in the same listener.
It's worth noting that on chance fail, the player isn't changed, but that was the case previously as well. I'm not sure if this behavior was intended since that makes it so there really isn't any consequence to failing to capture a mob. I've addressed this separately and can open a different PR where I added a
ChargeOnChanceFailconfig option. I wanted to keep these atomic, rather than one large PR. See features/charge-on-chance-fail.