Skip to content

Cleanup damage source a bit#12106

Merged
lynxplay merged 1 commit into
PaperMC:mainfrom
Lulu13022002:up/damage-source
Feb 16, 2025
Merged

Cleanup damage source a bit#12106
lynxplay merged 1 commit into
PaperMC:mainfrom
Lulu13022002:up/damage-source

Conversation

@Lulu13022002
Copy link
Copy Markdown
Contributor

Closes #11025

@Lulu13022002 Lulu13022002 requested a review from a team as a code owner February 12, 2025 19:50
Copy link
Copy Markdown
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.

Generally I like the cleanup, passing the cause like this is a lot nicer.
The thing I am a bit confused on is the withers/setters in DamageSource now.
Some are withers, e.g. perform a copy, others actually mutate the instance.
It seems semi arbitrary, presumably this is fine like this as the setters are only called on instances copied prior, however that feels pretty much like we are asking for an explosion down the line given the inconsistent behaviour here.

The two setters, critial and customBlockState, are I think not in a hot enough code path where the one extra copy of a damage source (which involves 0 deep copying anyway) would be a problem.
Are there other reasons I missed as to why you decided to make them setters?

@lynxplay lynxplay added the type: bug Something doesn't work as it was intended to. label Feb 12, 2025
@lynxplay lynxplay merged commit 7bee997 into PaperMC:main Feb 16, 2025
Y2Kwastaken pushed a commit to Y2Kwastaken/Paper that referenced this pull request Feb 16, 2025
@Lulu13022002 Lulu13022002 deleted the up/damage-source branch February 16, 2025 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug Something doesn't work as it was intended to.

Projects

Status: Merged

Development

Successfully merging this pull request may close these issues.

Sweeping Edge knockback vanilla inconsistency

2 participants