Skip to content

Use player scoreboard for determining collision rule when pushing entities#9127

Closed
Redned235 wants to merge 1 commit into
PaperMC:masterfrom
Redned235:fix/scoreboard-collision
Closed

Use player scoreboard for determining collision rule when pushing entities#9127
Redned235 wants to merge 1 commit into
PaperMC:masterfrom
Redned235:fix/scoreboard-collision

Conversation

@Redned235
Copy link
Copy Markdown
Contributor

The vanilla code in EntitySelector#pushable uses the Entity#getTeam method which returns the Level scoreboard. This means the wrong collision option is returned here when an API scoreboard is being used. Although no actual pushing occurs here as the client takes authority for that, the server still emits footstep sounds and treats it as if collision should be happening when it should not be.

Demonstration of the issue (turn sound on):

2023-04-11.21-44-24.mp4

@Redned235 Redned235 requested a review from a team as a code owner April 12, 2023 03:53
@electronicboy
Copy link
Copy Markdown
Member

collisions get weird, as especially player on player is pretty much 100% server sided, but, there is the wider caveat that this is a potential behavioral change for non player entities? I do wonder if something like this should be configurable

@Redned235
Copy link
Copy Markdown
Contributor Author

collisions get weird, as especially player on player is pretty much 100% server sided, but, there is the wider caveat that this is a potential behavioral change for non player entities? I do wonder if something like this should be configurable

I feel this would be more of a bug since in the event a vanilla scoreboard was used, the pushing behavior on the server would be properly in sync. Not entirely sure the ramifications of this, but if this could cause problems in certain situations I can add a config option.

…ities

The vanilla code in EntitySelector#pushable uses the Entity#getTeam method which returns the Level scoreboard. This means the wrong collision option is returned here  when an API scoreboard is being used. Although no actual pushing occurs here as the client takes authority for that, the server still emits footstep sounds and treats it as if collision should be happening when it should not be.
@Redned235 Redned235 force-pushed the fix/scoreboard-collision branch from 6bfa071 to ad9fe25 Compare April 12, 2023 14:40
@Owen1212055
Copy link
Copy Markdown
Member

If we are overriding the getScoreBoard method, shouldn't we just modify the getTeam behavior to instead use .getScoreBoard() instead of this.level.getScoreboard(). Since, is it possible this issue could occur in other places due to this team desync?

@Redned235
Copy link
Copy Markdown
Contributor Author

If we are overriding the getScoreBoard method, shouldn't we just modify the getTeam behavior to instead use .getScoreBoard() instead of this.level.getScoreboard(). Since, is it possible this issue could occur in other places due to this team desync?

I was thinking about doing this but noticed that other places like the /team command also would be affected by that. As of now none of the vanilla commands can interact with API scoreboards or teams and I thought going this route would be too big of a behavioral change.

+ if (!this.isPushable()) {
+ return;
+ }
+ net.minecraft.world.scores.Team team = this.getTeam();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd probably argue that, if we are adding a new patch for this, this change should be in the added patch as well.

Fixing it in two places makes dropping a patch harder / knowing what the patch/fix entails.
Not too sure on it tho, I hope we can get some PRarathon started this weekend and I'll get back on this 👍

@Warriorrrr Warriorrrr moved this from Awaiting final testing 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
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