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

Core: KivyMD and Launcher overhaul #3934

Open
wants to merge 57 commits into
base: main
Choose a base branch
from
Open

Conversation

Silvris
Copy link
Collaborator

@Silvris Silvris commented Sep 13, 2024

What is this fixing or adding?

Shifts the contents of kvui.py, and thus all CommonClient-based clients as well as Launcher, to using KivyMD. KivyMD is an extension for Kivy that is almost fully compatible with pre-existing Kivy components, while providing Material Design support for theming and overall visual design as well as useful pre-existing built in components such as Snackbars, Tooltips, and a built-in File Manager (not currently being used).

As a part of this shift, the launcher was completely overhauled, adding the ability to filter the list of components down to each type of component, the ability to define favorite components and filter to them, and add shortcuts for launcher components to the desktop. An optional description field was added to Component for display within the new launcher.

The theme (Light/Dark) and primary palette have also been exposed to users via client/user.kv.

How was this tested?

Manually, across the span of numerous unsupported games. I'm considering putting out a pre-release version for wider testing as well.

The Launcher changes were tested manually, both in a frozen build and on source.

If this makes graphical changes, please attach screenshots.

Launcher - Dark/Green
image

Launcher - Light/Lavenderblush
image

Launcher - Dark/Violet
image

Text Client - Dark/Red
image

Text Client - Dark/Cyan
image

@qwint
Copy link
Contributor

qwint commented Sep 19, 2024

with the removal of the broken code and some more futzing I got parity with pre-md working, thanks <3

Copy link
Collaborator

@Ziktofel Ziktofel left a comment

Choose a reason for hiding this comment

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

Looks fine for SC2
image

requirements.txt Outdated Show resolved Hide resolved
@qwint
Copy link
Contributor

qwint commented Oct 17, 2024

FYI #2779 when handling the player picking between text client and a game client seems broken with this branch (the automatically launching text client seems to work without issue)

Copy link
Contributor

@qwint qwint left a comment

Choose a reason for hiding this comment

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

this doesn't look pretty but it doesn't crash at least
image

Launcher.py Outdated Show resolved Hide resolved
Launcher.py Outdated Show resolved Hide resolved
Launcher.py Outdated Show resolved Hide resolved
@qwint
Copy link
Contributor

qwint commented Nov 5, 2024

some other small things ive noticed while using this branch
the launcher components don't scroll by click and drag (and imo the movement per scroll-wheel click is too small, no idea if that's changeable)
When the hint tab is sorted back and forth the size of each row seems to get confused and some entries start clipping (I tested this side by side with main and couldn't get main to run into the same issue
ex.
image

@Silvris
Copy link
Collaborator Author

Silvris commented Nov 28, 2024

some other small things ive noticed while using this branch the launcher components don't scroll by click and drag (and imo the movement per scroll-wheel click is too small, no idea if that's changeable)

I doubled the movement on each scroll wheel tick, but we can probably adjust that one to taste with a proper beta version. The "not scrolling by click and drag" was intentional because of some weirdness with it, but I think this is more an issue of the base KivyMD scroll effect. Gonna mess with it some and see if I can't just swap in the base Kivy version.

When the hint tab is sorted back and forth the size of each row seems to get confused and some entries start clipping (I tested this side by side with main and couldn't get main to run into the same issue ex.

Fixed, to the point we don't even need the fix_heights hack anymore (probably worth porting this into current main).

@qwint
Copy link
Contributor

qwint commented Nov 28, 2024

code and functionality looks good on first blush, will test a bit more extensively later

@qwint
Copy link
Contributor

qwint commented Nov 30, 2024

FYI the imageloader stuff in #3629 does require use of the ApAsyncImage class in kvui instead of AsyncImage directly, else it should be plug and play

Comment on lines -347 to +413
def set_height(self, instance, value):
def set_height(self, _, _2):
Copy link
Collaborator

Choose a reason for hiding this comment

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

In Python, using underscores for unused function parameter names is not good practice.
It's good for local variables, but not for function parameters.

In Python (different from some other languages), the parameter names are an important part of the function signature.
If this is an override or if it's implementing some interface, and the interface is defined as:

def set_height(self, instance, value):
    ...

That means that someone could call an implementor of that interface with:

x.set_height(instance=foo, value=bar)

And that would crash with this override, because it doesn't see those parameter names.

If a linter is complaining about these unused parameters, it shouldn't,
because it can't know whether you're implementing some structural interface that requires the parameters (with certain names) to be there.

Copy link
Contributor

@qwint qwint left a comment

Choose a reason for hiding this comment

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

all my previous comments were addressed and noticed no new issues in using the UI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: enhancement Issues requesting new features or pull requests implementing new features. is: refactor/cleanup Improvements to code/output readability or organizization. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants