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

Better Item Selection UI #5921

Closed
wants to merge 23 commits into from

Conversation

MEEPofFaith
Copy link
Contributor

@MEEPofFaith MEEPofFaith commented Sep 2, 2021

Edit: Here, have a demonstration video conveniently placed at the top so that you don't need to scroll down through the commits and comments.

Finalized.Selection.UI.Improvements.mp4

Due to the sheer amount of things you can select (especially with mods installed), I decided to make the Payload Source's selection UI wider.
Wide UI

If your pull request is not translation or serverlist-related, read the list of requirements below and check each box:

  • I have read the contribution guidelines.
  • I have ensured that my code compiles, if applicable.
  • I have ensured that any new features in this PR function correctly in-game, if applicable.

@MEEPofFaith
Copy link
Contributor Author

Smaller UIs not affected.

sJ7V75Hphh.mp4

@MEEPofFaith
Copy link
Contributor Author

I found that each row was 53 1/3 tall, and used that in the row calculations.

@sk7725
Copy link
Contributor

sk7725 commented Sep 2, 2021

test it in non-100% ui scaling

@USMP-lancer
Copy link

test it in non-100% ui scaling

yes

@MEEPofFaith
Copy link
Contributor Author

Good point, I play on 75% UI scale, dunno about other scales

@MEEPofFaith
Copy link
Contributor Author

aaa this is uncomfortable
image

@MEEPofFaith
Copy link
Contributor Author

image
uhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh

Yea I'll make "8 rows" be based off of 100%

I did not know that it had 5 rows by default :I

Though it was 40 * 5 so I guess it was... I just only play in 75% UI scale
@MEEPofFaith
Copy link
Contributor Author

Ok, made the rows based on 100% UI scaling so that it's correct.

iGWi6WFELk.mp4

I am uncomfortable with how large everything is.

@USMP-lancer
Copy link

I am uncomfortable with how large everything is.
reverse compensation

@MEEPofFaith MEEPofFaith changed the title Wider Payload Source UI Better Item Selection UI Sep 3, 2021
@MEEPofFaith
Copy link
Contributor Author

MEEPofFaith commented Sep 3, 2021

This footage is in 100% UI scale.

Search.Bar.mp4

Mods active to bloat the amount of possible selections:

  • BetaMindy
  • Exotic Mod
  • Exogenesis
  • Endless Rusting
  • Progressed Materials

@MEEPofFaith
Copy link
Contributor Author

MEEPofFaith commented Sep 3, 2021

image
Now to fix it in other UI scales...

(This is 25%)

@MEEPofFaith
Copy link
Contributor Author

MEEPofFaith commented Sep 3, 2021

Why didn't this exist before?

Tooltips.mp4

(Also I'm back down to a comfortable 75% UI scale)

@MEEPofFaith
Copy link
Contributor Author

MEEPofFaith commented Sep 3, 2021

Made the displayed images larger so that there's some more detail (especially on really large sprites) and easier to actually highlight over the image to make the name tooltip appear.

Larger.Icons.mp4

@MEEPofFaith
Copy link
Contributor Author

image
Now to fix it in other UI scales...

(This is 25%)

The width of the text box doesn't change with the scaling, and I don't know how to make it do that.

@buthed010203
Copy link
Contributor

I tried to make it work with scaling and gave up, im convinced its impossible.
What you can try tho is setting the width to Scl.scl(40 * columns), it works ok but isnt perfect

@MEEPofFaith
Copy link
Contributor Author

image

@buthed010203
Copy link
Contributor

Why is rebuild a Runnable[] of length 1 rather than just Runnable rebuild = ...? both approaches work the same

@MEEPofFaith
Copy link
Contributor Author

Why is rebuild a Runnable[] of length 1 rather than just Runnable rebuild = ...? both approaches work the same

unsure, but the attribute stats on AttributeCrafter use a single term array as well

@MEEPofFaith
Copy link
Contributor Author

MEEPofFaith commented Sep 3, 2021

Now changing the ui scale doesn't change the number of rows visible at once
image

@MEEPofFaith
Copy link
Contributor Author

25% is so tiny why does it exist

@MEEPofFaith
Copy link
Contributor Author

Merge when

@buthed010203
Copy link
Contributor

buthed010203 commented Sep 13, 2021

No
might want to ping anuke considering he gave feedback thats since been implemented

@MEEPofFaith
Copy link
Contributor Author

No
might want to ping anuke considering he gave feedback thats since been implemented

Well I did post in pulls and Anuke has talked about it.
image

@buthed010203
Copy link
Contributor

Ah ok

@MEEPofFaith
Copy link
Contributor Author

MEEPofFaith commented Sep 27, 2021

Anything missing that I should add to this?
I'm too lazy to scroll down and look for stuff

@sk7725
Copy link
Contributor

sk7725 commented Sep 28, 2021

Anything missing that I should add to this? I'm too lazy to scroll down and look for stuff

Maybe set the threshold to 1.5x the scrollpane (ceil it), not just the x1 the scrollpane height;
e.g. for item selection pane (height = 5), the search bat would show up if you have more than 8 rows of items (5 * 1.5 ceil = 8).

@MEEPofFaith
Copy link
Contributor Author

makes sense, you don't need to scroll a lot

@USMP-lancer
Copy link

id say is nice

@MEEPofFaith
Copy link
Contributor Author

please no more

@MEEPofFaith
Copy link
Contributor Author

oh no issues

buthed010203 added a commit to mindustry-antigrief/mindustry-client that referenced this pull request Feb 14, 2022
@MEEPofFaith
Copy link
Contributor Author

I swear to god if this has private branch conflicts.

@buthed010203
Copy link
Contributor

I will end up merging them when v7 comes out either way

@BalaM314
Copy link
Contributor

why is this not merged the payload source ui is so awkward

@Eclipse-04
Copy link

this is a 6 month post wha-

@MEEPofFaith
Copy link
Contributor Author

I am surprised there are no merge conflicts

@buthed010203
Copy link
Contributor

I am surprised there are no merge conflicts

just wait for the private branch

@Anuken
Copy link
Owner

Anuken commented Mar 25, 2022

I could try merging this soon, I haven't modified the selection UI at all.

@buthed010203
Copy link
Contributor

I've merged this into my client and it seems to work well. For some reason the selection background gets really dark sometimes but thats probably just my doing

@Anuken
Copy link
Owner

Anuken commented Sep 1, 2022

Merged with a bunch of tweaks.

@Anuken Anuken closed this Sep 1, 2022
@MEEPofFaith
Copy link
Contributor Author

epic

@MEEPofFaith MEEPofFaith deleted the wider-payload-source-ui branch September 2, 2022 00:27
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.

None yet

9 participants