Skip to content

Fix CraftPersistentDataTypeRegistry CME#8425

Closed
Gerrygames wants to merge 1 commit into
PaperMC:masterfrom
Gerrygames:fix/CME2
Closed

Fix CraftPersistentDataTypeRegistry CME#8425
Gerrygames wants to merge 1 commit into
PaperMC:masterfrom
Gerrygames:fix/CME2

Conversation

@Gerrygames
Copy link
Copy Markdown
Contributor

New approach to #6701. This will have no performance impact after adapters are created.

@Gerrygames Gerrygames requested a review from a team as a code owner October 1, 2022 18:01
@Owen1212055
Copy link
Copy Markdown
Member

Is accessing this api async something we want to encourage? Especially if we move away from having our own itemstack implementation I am not sure how safe doing any of this would be even.

@Machine-Maker
Copy link
Copy Markdown
Member

Machine-Maker commented Dec 28, 2022

Well we definitely want to support creating and manipulating itemstacks on other threads. Specifically itemstacks that don't have an in-world representation (aren't in an inventory or container, or entity). You have to be more specific when you say "access this api async". Do you mean have the same instance of ItemStack on two threads, both making modifications? Or do you mean creating and manipulating a single ItemStack instance all in one thread that is NOT the main thread.

@lynxplay
Copy link
Copy Markdown
Contributor

Would also agree. While the item stack itself does not have to be thread safe as an instance itself (e.g. I don't think we need to promote editing item stacks on multiple threads) we should still allow item stack creation and their customization off the main thread if that thread owns the item stack instance and no other (including main thread) mutates the item.

lynxplay added a commit to KnockturnMC/ktp that referenced this pull request May 13, 2023
Fixes a CME during pdc type lookup by pulling the patch proposed in the
upstream PR PaperMC/Paper#8425.
@codebycam
Copy link
Copy Markdown

Closing this because a fix was already merged (in the PR you mentioned).

@codebycam codebycam closed this Sep 4, 2023
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.

6 participants