Advanced settings: Noise window reads “eased” value incorrectly

[Wuzzy2]

Issue type

• Bug report

Minetest version

95d4ff6

Summary

Sometimes, when you edit a noise setting in advanced settings, the checkbox “eased” gets checked even if its not in the setting value.

Steps to reproduce

1. Open Minetest
2. In advanced settings, reset mgv5_np_factor to default. The word “eased” should not appear in the value
3. Click the “Edit” button to open the noise window
4. Look at the checkboxes

Expected: “eased” is not checked, because this word does not appear in the string
Actual: “eased” IS checked

[paramat]

Confirmed.

Note this noise has 'eased' as the default.
When i open the advanced settings tree the word 'eased' does not appear ('restore default' does not need to be clicked).
If i 'edit' then 'save' (not 'cancel') the word does appear in the advanced settings tree.
'Restore default' then makes the word disappear.

Entries in .conf don't seem broken though so seems a display issue only. Not a blocker.

Also, after changing this back and forth, look what appears at the end of my .conf:

}
}
}
}
}

When a noise table is removed from .conf the last '}' is not removed.
@srifqi

[nerzhul]

Same as previous PR, not blocking release but can be fixed for 5.0.0

[srifqi]

I found something on dlg_settings_advanced.lua:

minetest/builtin/mainmenu/dlg_settings_advanced.lua

Lines 159 to 165 in 0acdf93

	index = default:find("[^, ]", index)
	local flags = ""
	if index then
	flags = default:sub(index)
	default = default:sub(1, index - 3) -- Make sure no flags in single-line format
	end
	table.insert(values, flags)

Why should the flag be removed from default? Note that I just revived an old PR and didn't even know the reason behind it.

---

About the forgotten closed curly bracket, I think these lines of code makes it (}) marked as invalid and just leave as it is.

minetest/src/settings.cpp

Lines 971 to 973 in 0acdf93

	size_t pos = trimmed_line.find('=');
	if (pos == std::string::npos)
	return SPE_INVALID;

Maybe, we should create a stack for the settings' parent and create special case for closed curly bracket (})? I don't know for now.

minetest/src/settings.cpp

Lines 246 to 248 in 0acdf93

	sanity_check(it->second.group != NULL);
	was_modified |= it->second.group->updateConfigObject(is, os,
	"}", tab_depth + 1);

[paramat]

#8089 merged, fixes the '}'s.

[p-ouellette]

Omitting the flags was intentional, probably because the single line format does not support flags. Still, I think it would be better to show them so that the current value can be seen without opening the edit window.

[srifqi]

Single flag format is removed already from minetest.conf.example.

I found that the flags' place is used for lacunarity (as optional value). Should we make lacunarity a required value?

minetest/src/settings.cpp

Lines 541 to 543 in 245e628

	std::string optional_params = f.next("");
	if (!optional_params.empty())
	np.lacunarity = stof(optional_params);

[Wuzzy2]

I can no longer reproduce this bug in 4445889 (5.2.0-dev).

Should I close it?

[SmallJoker]

#8089