QoL improvements for parts with a large number of subtypes#228
QoL improvements for parts with a large number of subtypes#228Scialytic wants to merge 8 commits intoblowfishpro:masterfrom
Conversation
blowfishpro
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I left a few comments, please let me know what you think about them.
| if (buttonHeight * options.Count < 0.75f * Screen.height) return options.ToArray(); | ||
|
|
||
| // UnityEngine.TextRenderingModule must be added to project References | ||
| options.Insert(0, new DialogGUIContentSizer(ContentSizeFitter.FitMode.Unconstrained, ContentSizeFitter.FitMode.PreferredSize)); |
There was a problem hiding this comment.
If this needs to be first maybe we can just add it before the loop?
There was a problem hiding this comment.
I think so. I was a little paranoid about not altering the existing popup behavior in cases where my changes aren't needed. From a quick test, it doesn't look like having that element causes any changes to the normal popup, so it's probably fine to just always add it first. I can commit the change if you want.
There was a problem hiding this comment.
I think it makes sense to do that yeah
| // Each button is effectively 28 units wide, meaning the viewport is 7+(1/6) buttons wide. | ||
| // If we scroll past the last button, the buttons will bounce back so that the last button is touching the viewport's right edge. | ||
| // That bounce could make a player lose their place in the button list (plus it just looks bad). Therefore, we don't use 1.0 to set the viewport full right. | ||
| const float viewportWidthInButtons = 7+1/6f; |
There was a problem hiding this comment.
Maybe we could put the width in a constant somewhere and derive the rest of the numbers from it?
There was a problem hiding this comment.
Is the 28 unit button width already accessible somewhere? I couldn't find it, but maybe I'm just not looking in the right place. Everything I tried is still 0 or 1 when this code runs. Without that, it's two constants (viewportWidth and buttonWidth) that would be needed. I could certainly do it that way. Let me know what you think/prefer.
There was a problem hiding this comment.
I'm not aware of it being accessible anywhere. I could go either way on this
There was a problem hiding this comment.
It's unavoidably awkward either way, but overall I think a single viewportWidthInButtons constant is preferable to splitting it into two (e.g., viewportWidth and buttonWidth). We only ever need the ratio (viewportWidth / buttonWidth); there's currently no use for either part individually. I think there's more risk of confusion for future contributors with viewportWidth and buttonWidth, whereas viewportWidthInButtons sounds like a special-purpose value that shouldn't be used for anything else.

If there's anything you don't like or want me to change, I'm happy to fix it.
I think this is mostly relevant for Conformal Decals. #199 With a big atlas texture, 50+ (or even 100+) subtypes is not outside the realm of possibility.
The popup selector is unchanged if it fits nicely on screen:

But if it would be too tall, a scrolling version is used instead:

The in-PAW selector now scrolls automatically to keep the current subtype button visible. The selected button will be centered unless it's too near the start/end of the list. I also boost the scroll sensitivity according to the number of buttons, for people who prefer mouse-wheeling to dragging their way through a long list.
