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

Disable desynchronize_mapblock_texture_animation by default #13514

Merged
merged 1 commit into from May 26, 2023

Conversation

rollerozxa
Copy link
Member

This PR disables the desynchronize_mapblock_texture_animation setting by default. When enabled, it makes it impossible to keep animated node textures in sync between mapblocks, they will always intentionally be out of sync.

Below is an example, with the Rainbow Wool node on Voxelmanip Classic (animated to cycle between wool colours), when the setting is enabled:

image

What I actually want it to look is like this, when the setting is disabled:

image

I don't see any reason as to why a game, mod or server would want to show off glaring mapblock seams like this, so I believe it should be disabled by default.

Another example with a less obnoxious animated texture, MTG's lava shows an ugly checkerboard pattern with the setting enabled.

image

To do

This PR is a Ready for Review.

How to test

👀

@@ -2681,7 +2681,7 @@

# Whether node texture animations should be desynchronized per mapblock.
# type: bool
# desynchronize_mapblock_texture_animation = true
# desynchronize_mapblock_texture_animation = false
Copy link
Member

Choose a reason for hiding this comment

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

(You don't have to edit this file.)

@Desour
Copy link
Member

Desour commented May 24, 2023

Here are some issues that have complained about this setting, and/or wanted to change it:
#175
#4862
#6845
It mostly boils down to preference.


My opinion on this:
TL;DR: I prefer default false.

An ideal solution would be that each node can specify in its nodedef that it can and should be animated in desync (e.g. false for lava, and true for torch). And the animation desync should not be per block, but per node (this would be easy to do if we did the animation in shaders).

The more reasonable default for such a nodedef field would be false, as (old) nodes that are mistakenly desynced will look worse than synced torches or similar.
Old clients that don't understand the field should also default to false, this is controlled by desynchronize_mapblock_texture_animation. => Set it to false.

Another reason why false is preferable:
desynchronize_mapblock_texture_animation is a feature that does not work perfectly. It looks like a bug, and in some situations it doesn't do what it should (i.e. when torches are placed next to each other). Such things that have their drawbacks are usually off by default, I think.

@nerzhul nerzhul merged commit f4cb16c into minetest:master May 26, 2023
13 checks passed
Andrey2470T pushed a commit to Andrey2470T/minetest that referenced this pull request May 31, 2023
Wuzzy2 pushed a commit to Wuzzy2/minetest that referenced this pull request Jul 31, 2023
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.

None yet

5 participants