Skip to content

Comments

Use y-clipping settings from the map view when creating a new scene or loading chunks from it#1770

Merged
leMaik merged 7 commits intochunky-dev:masterfrom
leMaik:y-clipping-defaults
Sep 20, 2025
Merged

Use y-clipping settings from the map view when creating a new scene or loading chunks from it#1770
leMaik merged 7 commits intochunky-dev:masterfrom
leMaik:y-clipping-defaults

Conversation

@leMaik
Copy link
Member

@leMaik leMaik commented Sep 23, 2024

Also fixes requiring two reloads after changing the y-clipping settings of a scene.

Closes #1741

@leMaik leMaik requested a review from Peregrine05 September 24, 2024 12:19
Copy link
Member

@Peregrine05 Peregrine05 left a comment

Choose a reason for hiding this comment

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

Things seem slightly confusing now.

Chunky appears to use the Map Y clip when Load selected chunks or New scene from selection in the Map tab context menu is used, but it uses the Scene Y clip when Load selected chunks or Reload chunks in the Scene tab is used. Additionally, changing the Map Y clip while the Scene tab is open causes the Scene Y clip values to apparently reset to their default values; however, the internal values are not changed, and the UI values are set properly once it refreshes.

I think that either there should be one set of sliders that controls both the map view and the scene clip, or there should be two separate sets of sliders, as Chunky was previously, but with a note that points users to the Scene tab to change the Scene Y clip.

As I recall, the main issue was that the Scene Y clip values were not being set properly, which resulted in portions of the world not being loaded. Users would see the Map Y clip controls, and assume that they changed the Scene Y clip, so a note next to them, I believe, would be effective.

@leMaik
Copy link
Member Author

leMaik commented Sep 24, 2024

Chunky appears to use the Map Y clip when Load selected chunks or New scene from selection in the Map tab context menu is used, but it uses the Scene Y clip when Load selected chunks or Reload chunks in the Scene tab is used.

@Peregrine05 This is exactly the intended behavior. I do see that it might be confusing if you create a scene from the left panel – I didn't consider that.

Additionally, changing the Map Y clip while the Scene tab is open causes the Scene Y clip values to apparently reset to their default values; however, the internal values are not changed, and the UI values are set properly once it refreshes.

That is not supposed to happen… I'll look into this. The scene tab is supposed to represent the scene state, regardless of whatever the map view does.

Users would see the Map Y clip controls, and assume that they changed the Scene Y clip, so a note next to them, I believe, would be effective.

Users would expect that the y clip controls from the map are applied when creating a new scene – which is what this PR does. After a scene has been created, the y clip (as well as any other scene settings) can be controlled in the scene tab.

two separate sets of sliders, as Chunky was previously, but with a note that points users to the Scene tab to change the Scene Y clip.

A UI that needs a note to explain how it works is the worst solution we could find.

@leMaik
Copy link
Member Author

leMaik commented Sep 14, 2025

image

Edit: Changed it to "The map Y clip also applies to new scenes. To change the Y clip of the current scene, use the sliders in the Scene panel and then reload the chunks."

@Peregrine05 What do you think? 🤔

@leMaik leMaik force-pushed the y-clipping-defaults branch from fffe570 to c0828d6 Compare September 14, 2025 11:25
@Peregrine05
Copy link
Member

Peregrine05 commented Sep 14, 2025

Ah; so how it works now is:

Map right-click > New scene from selection: Use Map Y clip
Map right-click > Load selected chunks (First usage, equivalent to New scene from selection): Use Map Y clip
Scene tab > Load selected chunks: Use Scene Y clip

If so, I think should work well for the moment (a dialog box might be a better solution in the long run, but a dialog box also requires more user input to get the same job done, so this solution would be more convenient for the users).

@leMaik
Copy link
Member Author

leMaik commented Sep 14, 2025

@Peregrine05 Exactly 💯

Regarding the dialog box: Yes, I also considered that, but the hint is mostly for first time users who don't know about the two sliders yet. Once they know, there's no reason to bother them with a dialog. But the content of a dialog with a "Don't show this again" checkbox might be forgotten at some point.

Let's roll with this and see if the y clip confusion is reduced. 🚀

Also regarding "y clip": Should we consistently rename this to "Y-clip"? Same for -axis etc.?

@Peregrine05
Copy link
Member

I think that's a good idea, as it helps define the two separate words as a single "word".

@leMaik leMaik force-pushed the y-clipping-defaults branch from c0828d6 to 2cadc4c Compare September 15, 2025 21:27
@leMaik leMaik merged commit 41492ea into chunky-dev:master Sep 20, 2025
1 check passed
@leMaik leMaik deleted the y-clipping-defaults branch September 20, 2025 18:35
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.

Fix y clipping defaults

2 participants