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

[Do not squash me] Fix group setting bugs #8089

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@SmallJoker
Copy link
Member

SmallJoker commented Jan 10, 2019

Fixes tailing } reported in #8076 (comment)
Fixes low-priority tailing comma #8077 (patch from @srifqi )

EDIT: How to test

  1. In current master: Edit noise settings, save, reset to default, repeat. A bunch of closing brackets will be left over.
  2. Same for this PR: The group setting are removed properly
  3. With this PR: remove all mapgen flags on a setting. No tailing comma is written to the config.
@paramat

This comment has been minimized.

Copy link
Member

paramat commented Jan 11, 2019

Thanks, will test when i have time to.

@paramat

This comment has been minimized.

Copy link
Member

paramat commented Jan 17, 2019

Now testing (sorry for delay, have been very tired).

Random brackets fixed.
Trailing comma fixed.

Lua changes are fine, but i don't well understand the C++ changes.

I'd like this merged for 5.0.0 although not a blocker.

@SmallJoker

This comment has been minimized.

Copy link
Member

SmallJoker commented Jan 19, 2019

Thanks for testing. If the code works for you as expected, then please approve the PR (dunno who else would test it otherwise). I also tested it and it works fine.

@paramat

This comment has been minimized.

Copy link
Member

paramat commented Jan 19, 2019

👍 i trust you.

@paramat

This comment has been minimized.

Copy link
Member

paramat commented Jan 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment