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

kvui: use all flags in Item Class tooltip #3011

Merged
merged 1 commit into from
Apr 14, 2024
Merged

Conversation

Berserker66
Copy link
Member

What is this fixing or adding?

Allows the kivy UI to multi-class the items tooltip

How was this tested?

image

If this makes graphical changes, please attach screenshots.

image

@github-actions github-actions bot added affects: core Issues/PRs that touch core and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Mar 22, 2024
Copy link
Contributor

@Jouramie Jouramie left a comment

Choose a reason for hiding this comment

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

LGTM! Does not hurt to support that, tho I'm not exactly sure what could be a usecase for an item being both progression and useful, for instance. 🤔

Is it just a UI thing, or does something in the fill, for instance, behave differently?

@Silvris
Copy link
Collaborator

Silvris commented Mar 24, 2024

LGTM! Does not hurt to support that, tho I'm not exactly sure what could be a usecase for an item being both progression and useful, for instance. 🤔

Is it just a UI thing, or does something in the fill, for instance, behave differently?

Mostly a visual thing, but there was discussion about using the combination of progression + useful to note progression that is significantly better than other progression in the world (ie LttP Moon Pearl vs Shovel, or Stardew Shipping Bin vs a Friendsanity heart). No matter the route taken eventually, this is likely a change that would need done anyways. For fill, I believe the progression flag would take priority.

@Jouramie
Copy link
Contributor

That make total sense. Especially in Stardew, we have so many progression items that just unlock one or two checks without being significant progress. Giving more hint to the fill algorithm on what is really important and what is not seems useful.

@alwaysintreble alwaysintreble added the is: enhancement Issues requesting new features or pull requests implementing new features. label Mar 28, 2024
Copy link
Member

@Exempt-Medic Exempt-Medic left a comment

Choose a reason for hiding this comment

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

It functions just fine and the code looks good

@Exempt-Medic Exempt-Medic added waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. and removed waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Apr 5, 2024
@Berserker66 Berserker66 merged commit f67e849 into main Apr 14, 2024
24 checks passed
@Berserker66 Berserker66 deleted the kvui_multi_class branch April 14, 2024 18:36
EmilyV99 pushed a commit to EmilyV99/Archipelago that referenced this pull request Apr 15, 2024
EmilyV99 pushed a commit to EmilyV99/Archipelago that referenced this pull request Apr 15, 2024
qwint pushed a commit to qwint/Archipelago that referenced this pull request Jun 24, 2024
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. waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants