Skip to content

Update PlayerAdvancementDoneEvent.java - #12272

Closed
BrihtaKai wants to merge 1 commit into
PaperMC:mainfrom
BrihtaKai:featurer
Closed

Update PlayerAdvancementDoneEvent.java#12272
BrihtaKai wants to merge 1 commit into
PaperMC:mainfrom
BrihtaKai:featurer

Conversation

@BrihtaKai

Copy link
Copy Markdown

PlayerAdvancementDoneEvent can be cancelled.

@BrihtaKai
BrihtaKai requested a review from a team as a code owner March 11, 2025 15:56
@github-project-automation github-project-automation Bot moved this to Awaiting review in Paper PR Queue Mar 11, 2025
@Leguan16

Copy link
Copy Markdown
Contributor

Hey,

For this to actually have any effect you would also need to change the server code.

@notTamion

Copy link
Copy Markdown
Contributor

Is there any specific reason you would like for this event to be cancellable? PlayerAdvancementCriterionGrantEvent, which is fired right before the PlayerAdvancementDoneEvent already allows cancellation.

@BrihtaKai

Copy link
Copy Markdown
Author

Is there any specific reason you would like for this event to be cancellable? PlayerAdvancementCriterionGrantEvent, which is fired right before the PlayerAdvancementDoneEvent already allows cancellation.

For more flexible advancement management.

@notTamion

Copy link
Copy Markdown
Contributor

I mean i am not 100 % against making it cancellable. Just saying there is already a way to do it (which is more flexible than the PlayerAdvancementDoneEvent since its called not only for done advancements)

@BrihtaKai

Copy link
Copy Markdown
Author

Hey,

For this to actually have any effect you would also need to change the server code.

Where?

@notTamion

Copy link
Copy Markdown
Contributor

Where?

please use our discord for help regarding contributing. You have to find where PlayerAdvancementDoneEvent is currently being called and find a way to nicely implement it

@electronicboy

Copy link
Copy Markdown
Member

I don't think that such an event being cancellable makes sense; You've already got all the criteria and are considered by everything to have "won" the advancement, this just seems like it would moreso be just disabling the side-effect triggers of that?

@notTamion

Copy link
Copy Markdown
Contributor

Given whats been said above, is there anything you would like to add to that or can we close this PR?

@BrihtaKai

Copy link
Copy Markdown
Author

Given whats been said above, is there anything you would like to add to that or can we close this PR?

No. You can close.

@BrihtaKai BrihtaKai closed this Mar 21, 2025
@github-project-automation github-project-automation Bot moved this from Awaiting review to Closed in Paper PR Queue Mar 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

4 participants