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

Attachments: Fix attachments to temporary removed objects #8989

Merged
merged 2 commits into from Oct 2, 2019

Conversation

SmallJoker
Copy link
Member

@SmallJoker SmallJoker commented Sep 25, 2019

Fixes #8929 by not clearing the attachment upon object removal.

To do

This PR is Ready for Review.

How to test

Imported from #8929:

  1. Player 1 sits in a car(t), player 2 watches
  2. Player 1 drives away, until either the parent object or the player is unloaded client-side
  3. Drive back, and all attachments must be the same

Now do this for various combinations of player_transfer_distance and active_block_range:

  • Where former is notable bigger than latter
  • Where latter is notable bigger than former

Additional tests: #8701 (to confirm that it still works as intended)

@SmallJoker SmallJoker added the Bugfix 🐛 PRs that fix a bug label Sep 25, 2019
src/server.cpp Outdated Show resolved Hide resolved
@IhrFussel
Copy link

IhrFussel commented Sep 25, 2019

I tested this PR and it fixes my reported bug it seems.

If the cart with the player is too far away the cart unloads and the player model floats above ground so the position even updates when entity is out of sight which is actually something good I think.

@IhrFussel
Copy link

IhrFussel commented Sep 26, 2019

This PR makes behavior inconsistent between 5.0.1 and 5.1.0 clients though:

5.1.0 client

  • Sees realtime position of attached player model and name tag even after entity unloaded

5.0.1 client

  • Only sees last known position of name tag before entity unloaded
  • Additionally the name tag moves to ~ 200 nodes away from 0,0,0 after a bit

@sfan5
Copy link
Member

sfan5 commented Sep 26, 2019

We can't fix broken behaviour of older releases of Minetest clients.

@SmallJoker
Copy link
Member Author

SmallJoker commented Sep 26, 2019

@IhrFussel I am well aware of this difference. In fact, I added it on purpose. Do you really want to go back to the buggy behaviour? Doubt it.

EDIT: There are a few more issues regarding unloaded entities but this PR should be enough for the moment.

@SmallJoker SmallJoker added this to the 5.1.0 milestone Sep 28, 2019
Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

Looks good.

@SmallJoker SmallJoker merged commit 81c2370 into minetest:master Oct 2, 2019
@SmallJoker SmallJoker deleted the reattach branch July 5, 2020 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Attached player disappears after entity reloads
3 participants