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.

Sign up for GitHub

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

Jump to bottom

Slot Text Stuff #1134

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Slot Text Stuff #1134

wants to merge 4 commits into from

Conversation

AzureAaron
Copy link
Collaborator

  • HOTM Perk Level Slot Text (Implements HoTM levels display #1130)
  • Add Discrite as an evolving item
  • Replace magic values for colours with constants
    • Wasn't sure what to name the blue colours but I guess its fine for now and can be changed later

@AzureAaron AzureAaron added documentation Improvements or additions to documentation new feature This issue or PR is a new feature labels Jan 12, 2025
@LifeIsAParadox LifeIsAParadox added the reviews needed This PR needs reviews label Jan 12, 2025
@AzureAaron AzureAaron linked an issue Jan 12, 2025 that may be closed by this pull request
HoTM levels display #1130
Open
Emirlol
Emirlol requested changes Jan 12, 2025
View reviewed changes
Copy link
Collaborator

@Emirlol Emirlol left a comment

Choose a reason for hiding this comment

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

About time someone refactored them into constants lol

src/main/java/de/hysky/skyblocker/skyblock/item/slottext/adders/HotmPerkLevelAdder.java Outdated
private static final ConfigInformation CONFIG_INFORMATION = new ConfigInformation(
"hotm_perk_level",
"skyblocker.config.uiAndVisuals.slotText.hotmPerkLevel");
private static final Pattern LEVEL = Pattern.compile("Level (?<level>\\d+)\\/(?<max>\\d+)");
Copy link
Collaborator

@Emirlol Emirlol Jan 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

/ doesn't need to be escaped

Suggested change
private static final Pattern LEVEL = Pattern.compile("Level (?<level>\\d+)\\/(?<max>\\d+)");
private static final Pattern LEVEL = Pattern.compile("Level (?<level>\\d+)/(?<max>\\d+)");

Also, this doesn't match maxed hotm perks. This doesn't seem intentional given that you handled the case where level == max for a different color.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have no idea what the text shows for that, it was a best effort guess

src/main/java/de/hysky/skyblocker/skyblock/item/slottext/adders/HotmPerkLevelAdder.java Outdated Show resolved Hide resolved
src/main/java/de/hysky/skyblocker/skyblock/item/slottext/SlotText.java Outdated Show resolved Hide resolved
@LifeIsAParadox LifeIsAParadox added changes requested This PR need changes and removed reviews needed This PR needs reviews labels Jan 12, 2025
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed changes requested This PR need changes labels Jan 12, 2025
@AzureAaron AzureAaron added changes requested This PR need changes and removed reviews needed This PR needs reviews labels Jan 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Emirlol Emirlol Emirlol requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
changes requested This PR need changes documentation Improvements or additions to documentation new feature This issue or PR is a new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

HoTM levels display
3 participants
@AzureAaron @Emirlol @LifeIsAParadox