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

Manually reset attribute map on restoring player instance #11580

Closed
wants to merge 3 commits into from

Conversation

electronicboy
Copy link
Member

@electronicboy electronicboy commented Nov 4, 2024

Spigots reuse of the entity instances causes the server to try to copy a players existing attributes back onto itself, which causes an error.

We'll manually clear the applicable attribute modifiers, but, in the long run, we should just revert out of the broken behavior of trying to reuse ServerPlayer instances.


Download the paperclip jar for this pull request: paper-11580.zip

@electronicboy electronicboy requested a review from a team as a code owner November 4, 2024 22:03
+ this.attributes.values().forEach(AttributeInstance::removeModifiers);
+ }
+
+ public void removeAllTransientModifiers() {
Copy link
Member

Choose a reason for hiding this comment

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

This method scared me for a second, should add a note that we expect the getters to return copies

Spigots reuse of the entity instances causes the server to try to copy
a players existing attributes back onto itself, which causes an error.

We'll manually clear the applicable attribute modifiers, but, in the long run,
we should just revert out of the broken behavior of trying to reuse ServerPlayer
instances.
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.

Removing transient modifiers means armor needs to reapply.
That does not happen because we are not actually a new entity.

Do not break API modifier expectations of lasting through death.
@electronicboy electronicboy added the build-pr-jar Enables a workflow to build Paperclip jars on the pull request. label Nov 12, 2024
@lynxplay
Copy link
Contributor

Closing this as the immediate issue has been solved by upstream.
It was solved however by breaking vanilla logic and no longer removing attribute modifiers on death.

The API needs to be fleshed out to support plugins to set attribute modifiers that do/do not persist through death.

@lynxplay lynxplay closed this Nov 24, 2024
@lynxplay lynxplay deleted the fix/attributes-restorefrom branch November 24, 2024 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-pr-jar Enables a workflow to build Paperclip jars on the pull request.
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

3 participants