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

Allow place_param2 values of 0 to be predicted client-side #13784

Closed
rollerozxa opened this issue Sep 6, 2023 · 4 comments · Fixed by #13787
Closed

Allow place_param2 values of 0 to be predicted client-side #13784

rollerozxa opened this issue Sep 6, 2023 · 4 comments · Fixed by #13787
Labels
Bug Issues that were confirmed to be a bug

Comments

@rollerozxa
Copy link
Member

Minetest version

Minetest 5.8.0-rollerozxa-904d01562 (Linux)
Using Irrlicht 1.9.0mt12
Using LuaJIT 2.1.0-beta3
BUILD_TYPE=Release
RUN_IN_PLACE=0
USE_CURL=1
USE_GETTEXT=1
USE_SOUND=1
STATIC_SHAREDIR="/usr/share/minetest"
STATIC_LOCALEDIR="/usr/share/locale"

Active renderer

No response

Irrlicht device

No response

Operating system and version

Linux

CPU model

No response

GPU model

No response

OpenGL version

No response

Summary

When adding place_param2 = 0 to a node to enable the client to predict the direction of a node (e.g. planks in MTG are rotatable but should only ever be rotated at one angle when placed), it ignores the value as 0 is also used for nodes without any param2 prediction, leading to flickering textures on the client.

Steps to reproduce

  1. Go into MTG, grab planks
  2. Try to place them not in the direction they usually go (east or west)
  3. Watch the top texture briefly flicker
@rollerozxa rollerozxa added the Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible label Sep 6, 2023
@sfan5 sfan5 added Feature request Issues that request the addition or enhancement of a feature and removed Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible labels Sep 6, 2023
@sfan5 sfan5 changed the title place_param2 values of 0 are ignored Allow place_param2 values of 0 to work Sep 6, 2023
@sfan5
Copy link
Member

sfan5 commented Sep 6, 2023

To no surprise this is because "0" is the neutral value so it is ignored. I wasn't totally sure if this should be a bug or feature request, now it's the latter.

@rollerozxa
Copy link
Member Author

I realised I probably was a bit vague in the title so a bit of a clarification. place_param2 = 0 works on the server-side, see e.g. planks on MTG which can only placed in one direction unless rotated with a screwdriver. The issue is that the client-side can't differentiate between a value of "0" meaning "no param2 prediction, do whatever you want client" and a value of "0" meaning "set the param2 value to 0 so e.g. the node will always be at a particular angle".

@rollerozxa rollerozxa changed the title Allow place_param2 values of 0 to work Allow place_param2 values of 0 to be predicted client-side Sep 6, 2023
@celeron55
Copy link
Member

celeron55 commented Sep 6, 2023

This goes quite deep. The network protocol requires an additional boolean value in order to enable telling the client "yes, my 0 is 0, not a command to ask you to figure out param2 on your own".

Currently 0 has a double meaning, it's only 0 if the client doesn't determine magic needs to happen based on various separate conditionals:

} else if (predicted_f.param_type_2 == CPT2_WALLMOUNTED ||

@sfan5
Copy link
Member

sfan5 commented Sep 6, 2023

place_param2 = 0 works on the server-side, see e.g. planks on MTG which can only placed in one direction

I see.
The cause for this is a bit complex: since the server placement runs in Lua which can differ between nil and 0 it works there. The C++ definition does not have that distinction. Incidentally that only makes a difference on the client, hence the mismatch you see.

@sfan5 sfan5 added Bug Issues that were confirmed to be a bug and removed Feature request Issues that request the addition or enhancement of a feature labels Sep 6, 2023
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

Successfully merging a pull request may close this issue.

3 participants