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

Attaching the player to an attached object (attachment chain) confuses the client #5013

Closed
orwell96 opened this issue Jan 8, 2017 · 25 comments
Labels
Bug Issues that were confirmed to be a bug

Comments

@orwell96
Copy link
Contributor

orwell96 commented Jan 8, 2017

Situation:
To get a workaround to the "feature" that the eye position is not moved when a player gets attached to an object with an offset given, I did the following.
Imagine 3 Entities:

  • a "root entity", in my case a wagon. (advtrains mod)
  • a "dummy entity". It is attached to the "root entity" with an offset position.
  • the Player. It is attached to the "dummy entity" with no offset.
    When all objects are near (0,0,0), everything's fine, but as soon as you get more distant from (0,0,0), problems arise.
    Problem:
  • If too far away from (0,0,0), the player is not positioned at the dummy object's location but anywhere else. Interestingly, it still moves in the same direction the wagon moves and controls are blocked.

Here's the output of minetest --verbose:

2017-01-08 21:52:22: INFO[Main]: Ground right-clicked
2017-01-08 21:52:22: VERBOSE[Server]: TOSERVER_INTERACT: action=3, item=7, pointed=[node under=298,3,128 above=298,4,128]
2017-01-08 21:52:22: VERBOSE[Server]: ServerEnvironment::addActiveObjectRaw(): Added id=7; there are now 2 active objects.
2017-01-08 21:52:22: VERBOSE[Server]: scriptapi_luaentity_add: id=7 name="advtrains:newlocomotive"
2017-01-08 21:52:22: VERBOSE[Server]: scriptapi_luaentity_activate: id=7
2017-01-08 21:52:22: ACTION[Server]: [advtrains][wagon no-id] activated 
2017-01-08 21:52:22: VERBOSE[Server]: virtual std::string LuaEntitySAO::getStaticData()
2017-01-08 21:52:22: ACTION[Server]: [advtrains]init_new_instance 148390874251.877819 (148390874251.875837) 
2017-01-08 21:52:22: VERBOSE[Server]: Server: Sent object remove/add: 0 removed, 0 added, packet size is 263
2017-01-08 21:52:22: INFO[Main]: GenericCAO: Got init data
2017-01-08 21:52:22: INFO[Main]: ClientEnvironment::addActiveObject(): added (id=7)
2017-01-08 21:52:22: INFO[Main]: GenericCAO::addToScene(): mesh
2017-01-08 21:52:22: INFO[Main]: Irrlicht: Loaded mesh: advtrains_engine_steam.b3d
2017-01-08 21:52:25: INFO[Main]: Client: time_of_day=5601 time_speed=0 dr=658
2017-01-08 21:52:25: INFO[Main]: Pointing at [node under=298,3,127 above=298,4,127]
2017-01-08 21:52:25: INFO[Main]: Pointing at [node under=297,2,127 above=297,3,127]
2017-01-08 21:52:26: INFO[Main]: Pointing at [object 7]
2017-01-08 21:52:26: WARNING[Server]: active block modifiers took 853ms (longer than 200ms)
2017-01-08 21:52:27: INFO[Server]: ServerMap: Written: 0 sector metadata files, 1 block files, 5283 blocks in memory.
2017-01-08 21:52:27: INFO[Server]: ServerMap: Blocks modified by: 
2017-01-08 21:52:27: INFO[Server]:   addActiveObjectRaw: - - - - - - - - - - - 1
2017-01-08 21:52:28: INFO[Main]: Right-clicked object
2017-01-08 21:52:28: VERBOSE[Server]: TOSERVER_INTERACT: action=3, item=0, pointed=[object 7]
2017-01-08 21:52:28: ACTION[Server]: ... right-clicks object 7: LuaEntitySAO at (298,3,126.15)
2017-01-08 21:52:29: INFO[Main]: Client: avg_rtt=0.001
2017-01-08 21:52:29: VERBOSE[Server]: ServerEnvironment::addActiveObjectRaw(): Added id=8; there are now 3 active objects.
2017-01-08 21:52:29: VERBOSE[Server]: scriptapi_luaentity_add: id=8 name="advtrains:attach_dummy"
2017-01-08 21:52:29: VERBOSE[Server]: scriptapi_luaentity_activate: id=8
2017-01-08 21:52:29: ACTION[Server]: [advtrains]dummy spawned 
2017-01-08 21:52:29: VERBOSE[Server]: virtual std::string LuaEntitySAO::getStaticData()
2017-01-08 21:52:29: VERBOSE[Server]: Server: Sent object remove/add: 0 removed, 0 added, packet size is 507
2017-01-08 21:52:29: INFO[Main]: GenericCAO: Got init data
2017-01-08 21:52:29: INFO[Main]: ClientEnvironment::addActiveObject(): added (id=8)
2017-01-08 21:52:29: INFO[Main]: GenericCAO::addToScene(): single_sprite
2017-01-08 21:52:29: INFO[Main]: GenericCAO::addToScene(): mesh
Loaded mesh: character.b3d
2017-01-08 21:52:29: INFO[Main]: Pointing at [nothing]
[snip]
2017-01-08 21:52:31: INFO[Main]: Pointing at [node under=98,5,125 above=97,5,125]
2017-01-08 21:52:33: WARNING[Server]: active block modifiers took 1100ms (longer than 200ms)
2017-01-08 21:52:33: VERBOSE[Server]: ServerEnvironment::deactivateFarObjects(): deactivating object id=7 on inactive block (18,0,7)
2017-01-08 21:52:33: VERBOSE[Server]: virtual std::string LuaEntitySAO::getStaticData()
2017-01-08 21:52:33: ACTION[Server]: [advtrains][wagon 148390874251.877819]: saving to wagon_save 
2017-01-08 21:52:33: VERBOSE[Server]: ServerEnvironment::deactivateFarObjects(): object id=7 is known by clients; not deleting yet
2017-01-08 21:52:33: VERBOSE[Server]: ServerEnvironment::deactivateFarObjects(): deactivating object id=8 on inactive block (18,0,7)
2017-01-08 21:52:33: VERBOSE[Server]: virtual std::string LuaEntitySAO::getStaticData()
2017-01-08 21:52:33: VERBOSE[Server]: ServerEnvironment::deactivateFarObjects(): object id=8 is known by clients; not deleting yet
2017-01-08 21:52:33: INFO[Server]: ServerMap: Unloaded 4 blocks from memory, of which 0 were written, 5417 blocks in memory.
2017-01-08 21:52:33: VERBOSE[Server]: Server: Sent object remove/add: 0 removed, 0 added, packet size is 8
2017-01-08 21:52:33: VERBOSE[Main]: ClientEnvironment::removeActiveObject(): id=7
2017-01-08 21:52:33: VERBOSE[Main]: ClientEnvironment::removeActiveObject(): id=8
2017-01-08 21:52:33: INFO[Server]: ServerMap: Written: 0 sector metadata files, 2 block files, 5417 blocks in memory.
2017-01-08 21:52:33: INFO[Server]: ServerMap: Blocks modified by: 
2017-01-08 21:52:33: INFO[Server]:   addActiveObjectRaw, deactivateFarObjects: Static data changed considerably: 1
2017-01-08 21:52:33: INFO[Server]:   deactivateFarObjects: Static data changed considerably: 1
2017-01-08 21:52:33: VERBOSE[Server]: scriptapi_luaentity_rm: id=7
2017-01-08 21:52:33: VERBOSE[Server]: scriptapi_luaentity_rm: id=8
2017-01-08 21:52:33: ACTION[Server]: Player ... moved too fast; resetting position
2017-01-08 21:52:33: VERBOSE[Server]: Server: Sending TOCLIENT_MOVE_PLAYER pos=(2980,30,1261.5) pitch=14.58 yaw=-87.99
2017-01-08 21:52:33: ACTION[Server]: Player ... moved too fast; resetting position
2017-01-08 21:52:33: VERBOSE[Server]: Server: Sending TOCLIENT_MOVE_PLAYER pos=(2980,30,1261.5) pitch=14.38 yaw=-89.19
2017-01-08 21:52:33: ACTION[Server]: Player ... moved too fast; resetting position
2017-01-08 21:52:33: VERBOSE[Server]: Server: Sending TOCLIENT_MOVE_PLAYER pos=(2980,30,1261.5) pitch=14.38 yaw=-89.39
2017-01-08 21:52:33: INFO[Main]: Client got TOCLIENT_MOVE_PLAYER pos=(2980,30,1261.5) pitch=14.58 yaw=-87.99
2017-01-08 21:52:33: INFO[Main]: Client got TOCLIENT_MOVE_PLAYER pos=(2980,30,1261.5) pitch=14.38 yaw=-89.19
2017-01-08 21:52:33: INFO[Main]: Client got TOCLIENT_MOVE_PLAYER pos=(2980,30,1261.5) pitch=14.38 yaw=-89.39
2017-01-08 21:52:33: INFO[Main]: Pointing at [nothing]
2017-01-08 21:52:33: VERBOSE[Server]: Server: Sending TOCLIENT_MOVE_PLAYER pos=(2980,30,1261.5) pitch=14.38 yaw=-89.39
2017-01-08 21:52:33: INFO[Main]: Client got TOCLIENT_MOVE_PLAYER pos=(2980,30,1261.5) pitch=14.38 yaw=-89.39

ID 7 is the wagon, ID 8 is the dummy entity.
As you can see, the blocks where the wagon and dummy entities are get unloaded...
Disabling anticheat does NOT fix the problem, so this is not the cause.

You can use this to test.

@orwell96 orwell96 changed the title Attaching the player to an attached object (attachment chain) confuse anticheat Attaching the player to an attached object (attachment chain) confuse the client Jan 8, 2017
@orwell96 orwell96 changed the title Attaching the player to an attached object (attachment chain) confuse the client Attaching the player to an attached object (attachment chain) confuses the client Jan 8, 2017
@orwell96
Copy link
Contributor Author

While trying (again) to fix the problem that attach offset is not applied to the camera, I recognized 'interesting' behavior. seems like the Irrlicht coordinate system is reset sometimes. Does someone know and can explain to me what minetest actually does there?

@juhdanad
Copy link
Contributor

I think you mean the camera offset.
If the Minetest world were in a large coordinate system, graphics bugs would appear far from the center of the world due to the limited precision of floating point numbers.
To avoid this, the whole world is shifted away in every 200 nodes (2000 Irrlicht units). This ensures that all coordinates are not too great in magnitude.
To get the display coordinates, you have to subtract the offset from the real coorcinates.
disp_pos = pos - camera.getOffset() * BS

@paramat
Copy link
Contributor

paramat commented Dec 31, 2018

Related #8037

@jmdevy
Copy link
Contributor

jmdevy commented Apr 28, 2019

Changing this line,

v3f lastpos = pos_translator.val_current;

to v3f lastpos = pos_translator.val_old; fixed the issue, I don't know if there are any consequences to doing this.
@paramat @SmallJoker

@SmallJoker
Copy link
Member

@CoderForTheBetter Which issue does that fix? I can hardly believe that changing this variable does have any visual effect since it's only used for the footstep sounds.

@jmdevy
Copy link
Contributor

jmdevy commented Apr 28, 2019

My mistake, I was using a mod I wrote that corrected for the 200 node offset, back to the drawing board.

@jmdevy
Copy link
Contributor

jmdevy commented Apr 28, 2019

I've done more research, I'll leave what I found in case it helps someone else figure all of this out. Use a mod that will make a chain of attachments like the OP has, go to somewhere that is > 200 nodes (x, y, or z) away from the origin. I chose to do go 200 nodes in the negative x-direction, and then used this code,
m_attachment_position.X = m_attachment_position.X - 200;
here

if (m_matrixnode && parent_node) {
m_matrixnode->setParent(parent_node);
getPosRotMatrix().setTranslation(m_attachment_position);
//setPitchYawRoll(getPosRotMatrix(), m_attachment_rotation);
// use Irrlicht eulers instead
getPosRotMatrix().setRotationDegrees(m_attachment_rotation);
m_matrixnode->updateAbsolutePosition();
}

just after line 1320. Now the attachment will be in the correct place; however, the '200' used above only works if the visual_scale in the axis you choose is 10 for the first parent entity, otherwise, you'll have to use a different number. The properties of the entities in my hierarchy were, root visual_size=(10,10,10), child1 (attached to root) visual_size=(0, 0, 0), and then player (attached to child1).

The above is mostly a patch I think, as SmallJoker says, it might be a sync problem.

@SmallJoker
Copy link
Member

SmallJoker commented May 7, 2019

@CoderForTheBetter Thanks a lot for your research. Indeed, a rotation & re-positioning calculation error is probably the cause for this issue. The node needs to be rotated at the position (0,0,0), whereas the camera seems to shift that.
A testing video of mine from 7 months ago to investigate what's going on with the attachment
-REMOVED LINK-

@jmdevy
Copy link
Contributor

jmdevy commented May 11, 2019

Figured out where that camera shift is happening:

// Update offset if too far away from the center of the map
m_camera_offset.X += CAMERA_OFFSET_STEP*
(((s16)(my_cp.X/BS) - m_camera_offset.X)/CAMERA_OFFSET_STEP);
m_camera_offset.Y += CAMERA_OFFSET_STEP*
(((s16)(my_cp.Y/BS) - m_camera_offset.Y)/CAMERA_OFFSET_STEP);
m_camera_offset.Z += CAMERA_OFFSET_STEP*
(((s16)(my_cp.Z/BS) - m_camera_offset.Z)/CAMERA_OFFSET_STEP);

Commenting out those lines fixes these issues (at least the camera offset but not really the model's, it seems) but then teleporting very far from 0,0,0 results in visual glitches (even when not attached to anything) probably because of floating point rounding at large numbers (which is the whole point of these lines, to avoid those visual glitches).

Just thought I would mention it.

@SmallJoker
Copy link
Member

What's the status here after #8701 ?

@TheTermos
Copy link
Contributor

TheTermos commented Jun 1, 2020

Tested with f51cf7c

Attaching player to an attachment still unusable, view constantly wobbling around and visual glitches affecting the whole scene.
But i couldn't reproduce player being detached after some distance traveled, so I guess there's been some improvement since I last tested (around September, 5.1.0 or 5.0.1 stable)

Edit: I could reproduce it eventually, nothing has changed.

@v-rob
Copy link
Member

v-rob commented Mar 9, 2021

As far as I can figure out, the camera offset exists because of float precision errors. Is there any problem with removing BS or changing it to something like 0.5? Or would it still be too imprecise in some way or another and cause compatibility problems? Excuse my ignorance, but I can't find much information on the full reason why the camera offset exists or proposals to make it better.

@TheTermos
Copy link
Contributor

@v-rob a huge change to player attachments was merged just before 5.4 release - #10952
Good idea to check if the problem still persists.

@sfan5
Copy link
Member

sfan5 commented Mar 9, 2021

As far as I can figure out, the camera offset exists because of float precision errors. Is there any problem with removing BS or changing it to something like 0.5?

You're talking about the camera offset and then asking if BS can be removed. Those two are not the same.
To my knowledge:

  • The camera offset exists to counter float imprecision, you get consistent calculations whether you're at x,y=(0,0) or (25000,28000)
  • BS exists so that programmers do not accidentally introduce rounding errors when casting float coordinates to int.

@TheTermos
Copy link
Contributor

TheTermos commented Mar 9, 2021

Is there any problem with removing BS or changing it to something like 0.5? Or would it still be too imprecise in some way

This wouldn't help because all positions and distances are affected by BS, so e.g. it doesn't matter if you do
300 000 + 0.01 or 30 000 + 0.001, the difference in magnitude between these numbers remains the same and so does the error.
Precision must be increased to f64.

@v-rob
Copy link
Member

v-rob commented Mar 10, 2021

I wasn't asking about this issue in particular, just about the camera offset; there doesn't seem to be any dedicated issue to the camera offset. Admittedly, this issue wasn't the most relevant. I was asking about BS because I was thinking (at the time of writing) that making every float smaller would increase precision (since BS makes everything 10 times larger), but I forgot that

300 000 + 0.01 or 30 000 + 0.001

happens. (I've probably been working with fixed point numbers in my own projects too much; I like them sooooo much better than floating point.)

However, increasing precision to f64 would only work for entity position, not the camera offset. The camera offset affects SceneNodes, which use floats internally, so we'd have to change a lot of Irrlicht code to use f64s instead of f32s.

@TheTermos
Copy link
Contributor

The camera offset affects SceneNodes, which use floats internally

Indeed, that's a bit of a bummer, but...
if it could be arranged that all calculations were done using f64 and only the final result was converted into f32, it might just work.

Once I had a working version of 64bit(ish) MT, there are conversion operators between different versions of vector and abbox, that could be incorporated into MT Irrlight fork.
https://github.com/TheTermos/minetest/blob/object-position-precision/src/irrlicht_changes/aabbox3d.h
https://github.com/TheTermos/minetest/blob/object-position-precision/src/irrlicht_changes/vector3d.h

@v-rob
Copy link
Member

v-rob commented Mar 10, 2021

A problem is attachment chains, where object 1 is attached to object 2, which is attached to object 3. Each object has its own position relative to the parent object, and they're all floats. It sounds very hard to work doubles into the mix.

Is there an issue somewhere that I overlooked that mainly concerns the camera offset? It routinely causes problems, so I wonder if there's an issue for replacing it with a less problematic solution.

@sfan5
Copy link
Member

sfan5 commented Mar 10, 2021

Do you have a less problematic solution in mind?

@v-rob
Copy link
Member

v-rob commented Mar 10, 2021

Not at the moment, but having a place to discuss better solutions would be nice. A recurring problem like the camera offset surely deserves its own issue instead of scattered conversation in IRC and different issues.

@TheTermos
Copy link
Contributor

A problem is attachment chains, where object 1 is attached to object 2, which is attached to object 3. Each object has its own position relative to the parent object

I still wonder why there should be a problem though. relative positions and camera offsets are still fairly large numbers, and if I'm not mistaken, f32 error margin around 300k is a magnitude of a MT world centimeter, that should be plenty enough precision for camera movement.

@TheTermos
Copy link
Contributor

Ah of course, the problem isn't about the position of camera itself, but rendering in the camera space, and I can see that 1cm precision imay be insufficient for that.

A temporary improvement could be to increase the step, why 200 if 900 should give roughly similar precision.

Maybe consider changing MT Irrlicht fork so all positions are f64, that might not be too difficult, and if conversion operators were to be added MT should work with such version without changes, and still would benefit somewhat from increased precision of internal Irrlicht calculations.

@doxygen-spammer
Copy link
Contributor

I have implemented the dummy object workaround in the Konstal 105 modpack here: https://invent.kde.org/davidhurka/doxy_mini_tram/-/tree/work/davidhurka/attachment-object. You can try it out with this modpack yourself.

On Minetest 5.4.1 from snap, the problem still persists to the full extent.

Attaching player to an attachment still unusable, view constantly wobbling around and visual glitches affecting the whole scene.

On Minetest 5.6.0-dev self-compiled, I could not observe any problem. (But: The box on which I run self-compiled has far more than 100x more performance than my slowbook, so it may well be that I just didn’t see the issue.)

If anyone has time to check out my modpack 👓 , please do so to confirm my observations. You need to build a long advtrains track, place a “Minitram Konstal 105” on it (while looking forward along the track), enter it (rightclick), and accelerate (forward button).

You can disable advtrains_attachment_offset_patch, then you see the behavior without workaround. You can easily tell that it is disabled, because you just see the floor of the train and can not look through the windows. Well, that is the reason why #10101 is so annoying. ;)

I will try it myself with Minetest 5.6.0-dev on the slowbook, as soon as I have lots of time to compile. ;)


But i couldn't reproduce player being detached after some distance traveled, so I guess there's been some improvement since I last tested (around September, 5.1.0 or 5.0.1 stable)

Edit: I could reproduce it eventually, nothing has changed.

I have not tested enough to be sure this does not happen anymore. I haven’t observed it.

@doxygen-spammer
Copy link
Contributor

On Minetest 5.4.1 from snap, the problem still persists to the full extent.

I have now compiled Minetest 5.4.0, 5.5.0, and today’s 5.6.0-dev on both computers. I wasn’t able to reproduce the problem anywhere. On 5.4.1 from snap it is gone too.

I didn’t install any of the compiled versions, and I also didn’t install updates and didn’t even reboot the slowbook since tuesday.

Can anyone explain what is going on?

@SmallJoker
Copy link
Member

SmallJoker commented May 1, 2022

Your client is haunted. The only explanation would be a race condition or mismatch in your testing results/notes.

Please ask to re-open this issue if you find testing code that's reproducible (more or less) reliably on 5.5.0 or 5.6.0-dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues that were confirmed to be a bug
Projects
None yet
Development

No branches or pull requests

9 participants