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

Restore simple settings tab and add advanced settings as dialog #3740

Closed
wants to merge 1 commit into from
Closed

Conversation

kilbith
Copy link
Contributor

@kilbith kilbith commented Feb 19, 2016

Rebased version of #3514. Fixes #3486 (blocker).

It has 2 approvals on the original PR and just stands there to be merged.

Credit is of course given to @BlockMen.

Cc: @paramat

@paramat
Copy link
Contributor

paramat commented Feb 19, 2016

Thanks for doing this. I'll review and test as soon as i can.

@paramat
Copy link
Contributor

paramat commented Feb 19, 2016

Some minor code style issues.

@kilbith
Copy link
Contributor Author

kilbith commented Feb 19, 2016

Fixed'em all.

@PilzAdam
Copy link
Contributor

Which window is displayed after clicking "Save" in the change settings dialog? The advanced menu again or the simple settings tab? (sorry, I currently can't test this myself)

@kilbith
Copy link
Contributor Author

kilbith commented Feb 19, 2016

@PilzAdam As currently : the advanced menu.

@PilzAdam
Copy link
Contributor

@kilbith that's good. 👍

@paramat
Copy link
Contributor

paramat commented Feb 19, 2016

Tested and seems to work.
One strange thing is when shaders are disabled and i toggle a graphics setting like 'smooth lighting', the shaders bars go green:

screenshot from 2016-02-19 19 00 33

+1 once that's fixed.

@kilbith
Copy link
Contributor Author

kilbith commented Feb 19, 2016

@paramat This glitch has always existed but fixed now.

By the way, I trashed the code of the viewing range scrollbars that I added in the past. BlockMen removed these scrollbars already but not some remaining code. Anyways it was destined to be obsolete as you (core-devs) intend to trash the auto viewing range very soon.

@paramat
Copy link
Contributor

paramat commented Feb 19, 2016

That all sounds good 👍

@Fixer-007
Copy link
Contributor

I think this also needs to have at least two additions:

@paramat
Copy link
Contributor

paramat commented Feb 21, 2016

Tone mapping, maybe, but could be added later.
View range slider, no, that's easily adjusted in-game (the only place you know what range is optimum) and sliders are imprecise.

@paramat
Copy link
Contributor

paramat commented Feb 21, 2016

This PR should go in fast because it's delaying other stuff, so above issues can be another commit.

@paramat paramat mentioned this pull request Feb 21, 2016
@paramat
Copy link
Contributor

paramat commented Feb 21, 2016

👍 Tested, i will merge soon.

@paramat
Copy link
Contributor

paramat commented Feb 21, 2016

ecc8b70

@paramat paramat closed this Feb 21, 2016
@paramat paramat mentioned this pull request Feb 21, 2016
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

5 participants