Skip to content

Conversation

@Scialytic
Copy link

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:
B9PS_0

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

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.
B9PS_3

Copy link
Owner

@blowfishpro blowfishpro left a comment

Choose a reason for hiding this comment

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

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));
Copy link
Owner

Choose a reason for hiding this comment

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

If this needs to be first maybe we can just add it before the loop?

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Owner

Choose a reason for hiding this comment

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

I think it makes sense to do that yeah

Copy link
Author

Choose a reason for hiding this comment

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

Ok, done.

// 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;
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we could put the width in a constant somewhere and derive the rest of the numbers from it?

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Owner

Choose a reason for hiding this comment

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

I'm not aware of it being accessible anywhere. I could go either way on this

Copy link
Author

Choose a reason for hiding this comment

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

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.

@Grimmas
Copy link

Grimmas commented Mar 21, 2023

We were discussing on Discord today and this came up. This is definitely a feature I want to see in B9, as I have a few patches out that convert other mods to B9 and they have very many tank setups to the point that the window overflows and it's impossible to select some of the tanks.
screenshot130

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants