Skip to content

Fix MC-255756#9649

Closed
roro1506HD wants to merge 4 commits into
PaperMC:masterfrom
roro1506HD:fix/mc-255756
Closed

Fix MC-255756#9649
roro1506HD wants to merge 4 commits into
PaperMC:masterfrom
roro1506HD:fix/mc-255756

Conversation

@roro1506HD
Copy link
Copy Markdown
Contributor

This PR fixes shields not playing sounds when being broken by an axe or when blocking a damage. This bug is client-sided but can easily be fixed by playing the sounds instead of the entity events, this is exactly what this PR does.

@roro1506HD roro1506HD requested a review from a team as a code owner August 25, 2023 12:56
@imDaniX
Copy link
Copy Markdown

imDaniX commented Aug 25, 2023

Maybe it would be better to both broadcast the entity event and play the sound?

Comment thread patches/server/1027-Fix-MC-255756-shield-break-block-sounds-not-playing.patch Outdated
@roro1506HD
Copy link
Copy Markdown
Contributor Author

Maybe it would be better to both broadcast the entity event and play the sound?

I chose not to send both because when mojang is gonna fix the issue on their end, we're gonna have a time where the sound is gonna be played twice

Copy link
Copy Markdown
Member

@Machine-Maker Machine-Maker left a comment

Choose a reason for hiding this comment

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

This whole patch can be merged into the patch that fixes a ton of other random vanilla bugs.

Comment thread patches/server/1027-Fix-MC-255756-shield-break-block-sounds-not-playing.patch Outdated
Comment thread patches/server/1027-Fix-MC-255756-shield-break-block-sounds-not-playing.patch Outdated
Copy link
Copy Markdown
Member

@Machine-Maker Machine-Maker left a comment

Choose a reason for hiding this comment

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

I think this bypasses the isSilent check that exists in the regular logic.

The more I look into this specific issue, the less satisfied I am with fixing it on the server. The correct fix is a client-sided one (as illustrated in the mojira issue). There are other inconsistencies we are creating. The sound now has a set seed and has the exact same pitch for all listeners which isn't the case in vanilla.

I think we should just not fix this as it would be too complicated to fix from the server.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lets put a // Paper - diff on change on the specific line in LivingEntity#handleEntityEvent that you took the playing sound call from

@Warriorrrr Warriorrrr moved this from Changes required to Waiting For Author in Paper PR Queue Mar 5, 2025
@kennytv kennytv added the pre-softspoon: never rebased Pre-hardfork pull requests that were not re-opened with the new main branch label Mar 23, 2025
@kennytv kennytv deleted the branch PaperMC:master March 23, 2025 19:15
@kennytv kennytv closed this Mar 23, 2025
@roro1506HD roro1506HD deleted the fix/mc-255756 branch October 16, 2025 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pre-softspoon: never rebased Pre-hardfork pull requests that were not re-opened with the new main branch pre-softspoon

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

7 participants