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

Increase defaults for viewing_range, active_object_range and related settings #10597

Merged
merged 1 commit into from Nov 3, 2020

Conversation

lhofhansl
Copy link
Contributor

As discussed in #10571

Note that I did not touch the Android defaults. Should be increase those too? (They are woefully low.)

@red-001
Copy link
Contributor

red-001 commented Nov 2, 2020

What are the android defaults right now?

@lhofhansl
Copy link
Contributor Author

Here are the relevant ones:

	settings->setDefault("viewing_range", "50");
	settings->setDefault("max_block_generate_distance", "5");
	settings->setDefault("client_mapblock_limit", "1000");

And some more interesting ones:

	settings->setDefault("active_block_range", "2");
	settings->setDefault("max_objects_per_block", "20");
	settings->setDefault("server_map_save_interval", "15");
	settings->setDefault("max_simultaneous_block_sends_per_client", "10");
	settings->setDefault("emergequeue_limit_diskonly", "16");
	settings->setDefault("emergequeue_limit_generate", "16");

@paramat
Copy link
Contributor

paramat commented Nov 3, 2020

The new values seem reasonable.
Block Cache of 7500 is a moderate and reasonable increase, and is the volume of a sphere of radius 194 nodes, enough for the new default view range of 190.
I would like to see some consideration by other core devs before a merge, but will approve anyway

I think it is best to discuss and alter Android settings separately.

Note for future: minetest.conf.example is regularly auto-generated from settingtypes.txt, so is wasted work including it in PRs (it should perhaps be 'gitignored').

@lhofhansl
Copy link
Contributor Author

I'll remove minetest.conf.example before I merge. What's the tool called to generate it?

@paramat
Copy link
Contributor

paramat commented Dec 5, 2020

I removed my approval for this for 2 reasons:

  1. I made a mistake, i did not test this. If i had i would have noticed the bad stutter discussed in Frame stutter seems worse since 2-3 months ago #10683 . Although the changes of this PR may not be the cause or the primary cause of that stutter, i would not have agreed to setting changes that would make the stutter worse.

  2. See DISCUSS: Increase default viewing_range and server send distances #10571 (comment)

@lhofhansl
Copy link
Contributor Author

We can roll the server side changes. But I want to state my express disagreement with this.

HybridDog pushed a commit to HybridDog/minetest that referenced this pull request Dec 17, 2020
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

4 participants