Add repeat_dig_time setting

[ryvnf]

Goal of the PR

Provide better accessibility for people who accidentally dig instant breakable nodes but want dig repetition and cannot use the safe_dig_and_place setting.

This PR solves this problem by introducing a repeat_dig_interval which is like repeat_place_interval but for digging. The setting simply ensures that it will take at minimum the settings value from finishing digging one node to starting to dig the next node.

Does it resolve any reported issue?

Yes, #13726.

To do

This PR is ready to review.

How to test

Hop into a world and hold the dig button on nodes which are instant breakable. Could be done by hopping into a creative world in Mineclonia and start digging any nodes.

Try it with the default value and verify that the digging speed remains the same as previously. Then go into the settings menu and update the Dig repetition interval setting to a higher value. Hop into the world again and dig nodes, verify that there is a larger delay between digging each node.

Things to consider

The range of values allowed is probably something which should be discussed. Previously, the hardcoded value was 0.15. Currently I made the minimum setting value 0.15 and the maximum 2.0 with the default being 0.15.

The repeat_place_interval is minimum 0.16 and maximum 2.0. It could be argued that maybe its minimum value should be lowered to 0.15 for consistency with the repeat_dig_interval setting.

But perhaps the minimum value of repeat_dig_inverval could be lower?

[grorp]

Try it with the default value and verify that the digging speed remains the same as previously.

The existing minimum value of 0.15 for runData.nodig_delay_timer only applies for nodes that take zero time to dig. There can be lower values in other cases:

• In Minetest Game creative mode, the most common value seems to be 0.0328125.
• When digging dirt in Minetest Game without creative mode, the value is 0.14.

This means that this PR actually (by default) results in a lower digging speed in some cases. To preserve the existing behavior, the default value for repeat_dig_time should be 0.0.

[ryvnf]

I have applied your suggested change but I realize that this might make the setting confusing as one might assume that a repeat_dig_interval of 0.0 would mean no delay between digging nodes.

I did however get an idea for an alternative proposal for how this could work. I will try to implement that but do so as a separate commit so it can simply be dropped if you do not like it.

[ryvnf]

I have updated the branch according to my new proposal. I decided to rebase and drop the previous commit because I think this new version is simply better and should be merged instead.

Changes

What I changed was that I made the repeat_dig_interval be

The minimum time in seconds it takes between repeated node removals when holding the dig button.

So it is the minimum time between breaking nodes and not the delay between digging nodes.

Using a value of 1.0 makes it take one second between breaking nodes in creative mode for Mineclonia, MineClone 2 and Minetest Game even though they all have different digging times. I think it is good for the setting to have that kind of consistency.

Reason

Like I stated above, I think a minimum and default value of 0.0 would be confusing because one might assume it means that there is no delay between digging nodes which is not true. Instead it would be 0.15 for instant-breakable nodes and less than that for some other nodes.

This also results in nodes with dig times less than 0.15 getting dig times of 0.15. The effect of this on gameplay will be minimal since such nodes are rare. I also think its good that it fixes the weirdness with how a node with a dig time of 0.1 is dug faster than a node with a dig time of 0.0.

It also removes the special condition for instant breakable nodes which makes the code cleaner.

Lowered the minimum value of repeat_place_time to 0.15 from 0.16

It could be argued that this is an unrelated change. I changed it so the valid intervals for repeat_place_time and repeat_dig_time are the same. Because the difference is only 10 ms it should not affect gameplay. I made it a separate commit. I can remove it if this is completely unacceptable but I think it is a good change to get symmetry between the settings.

Testing

I made this Lua script which can be used to verify that the formula it uses to compute the delay timer works as expected.

function get_nodig_delay_timer(dig_time_complete, crack_animation_length, repeat_dig_time)
	local nodig_delay_timer = dig_time_complete / crack_animation_length

	-- We don't want a corresponding delay to very time consuming nodes
	-- and nodes without digging time (e.g. torches) get a fixed delay.
	nodig_delay_timer = math.min(nodig_delay_timer, 0.3)

	-- Ensure that the delay between breaking nodes
	-- (dig_time_complete + nodig_delay_timer) is at least the
	-- value of the repeat_dig_time setting.
	nodig_delay_timer = math.max(nodig_delay_timer, repeat_dig_time - dig_time_complete)

	return nodig_delay_timer
end

repeat_dig_time = 0.15
crack_animation_length = 8
print("repeat_dig_time = " ..repeat_dig_time)
print("crack_animation_length = " .. crack_animation_length)
print("node_dig_time" .. "\t" .. "delay_time" .. "\t" .. "total_dig_time")
for dig_time_complete = 0, 0.5, 0.05 do
	local nodig_delay_timer = get_nodig_delay_timer(dig_time_complete, crack_animation_length, repeat_dig_time)
	print(dig_time_complete .. "\t\t" .. nodig_delay_timer .. "\t\t" .. (nodig_delay_timer + dig_time_complete))
end

[ryvnf]

I just made a change to the setting description. Changed it from

The minimum time in seconds it takes between repeated node removals when holding the dig button.

to

The minimum time in seconds it takes between digging nodes when holding the dig button.

which I think reads better.

I will not touch the branch anymore unless there are more things to be done after another review.

[grorp]

Tested a bit, seems good.

I'm not sure what to think of the change to the minimum of repeat_dig_time, but it probably won't make a big difference.

[grorp]

Calls to g_settings->(de)registerChangedCallback are missing for the new setting.