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

Settings GUI: Noise parameter setting fixes #13797

Merged
merged 5 commits into from Sep 16, 2023
Merged

Conversation

grorp
Copy link
Member

@grorp grorp commented Sep 12, 2023

This PR makes two small fixes regarding noise parameter settings to the settings GUI:

  • [Manual Squash] Redesign/unify mainmenu settings interface #12480 accidentally didn't copy over these lines from the deleted dlg_settings_advanced.lua to the newly added dlg_change_mapgen_flags.lua:

    for name, value in pairs(fields) do
    if name:sub(1, 3) == "cb_" then
    checkboxes[name] = value == "true"
    end
    end

    This means that if you change noise parameter flags using the checkboxes at the bottom of the noise parameter dialog, your changes won't be saved. This PR fixes that.

  • Also, noise parameter settings now show a reset button if they have been changed.

Of course, the noise parameter dialog could be improved a lot more, see #13476.

To do

This PR is a Ready for Review.

How to test

Verify that the noise parameter dialog saves your changes to the flags (aka the checkboxes).

Verify that a reset button is shown for changed noise parameter settings.

I can't really make sense of the comment in the removed line.
This *should* not break anything, not even in `generate_from_settingtypes.lua`.
(That file uses `setting.default`, but only for settings which aren't noise parameter settings.)
Copy link
Member

@rubenwardy rubenwardy left a comment

Choose a reason for hiding this comment

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

.

builtin/mainmenu/settings/dlg_change_mapgen_flags.lua Outdated Show resolved Hide resolved
@rubenwardy rubenwardy mentioned this pull request Sep 12, 2023
17 tasks
Co-authored-by: rubenwardy <rw@rubenwardy.com>
@grorp
Copy link
Member Author

grorp commented Sep 12, 2023

You can consider the concerns I expressed in the commit message of 3b9759e as resolved, I tested this PR against the master branch and it doesn't change the generated minetest.conf.example.

Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

lgtm

@grorp grorp merged commit 4f735fb into minetest:master Sep 16, 2023
2 checks passed
@grorp grorp deleted the noise-dialog branch December 18, 2023 16:38
kawogi pushed a commit to kawogi/minetest that referenced this pull request Dec 19, 2023
cosin15 pushed a commit to cosin15/minetest that referenced this pull request Feb 24, 2024
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

3 participants