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

Improvement: Cake Tracker Scrollable and AH Tips #2939

Merged
merged 8 commits into from
Dec 5, 2024

Conversation

DavidArthurCole
Copy link
Contributor

@DavidArthurCole DavidArthurCole commented Nov 16, 2024

What

Instead of having a maximum length, make cake tracker implement scrollable, and add tips about there being entries above/below. I tried to do a scroll bar renderable, that failed, so I instead added common logic for adding these tips that's default false. (See images).
Also adds a note to the on-hover for prices that indicates that clicking them will open AH.

Images

image

image

image

Changelog Improvements

  • Improved the UI for the Cake Tracker. - Daveed
    • Made the Cake Tracker scrollable instead of having a fixed size.

@github-actions github-actions bot added the Wrong Title/Changelog There is an error in the title or changelog label Nov 16, 2024
Copy link

I have detected some issues with your pull request:

Body issues:
Change should end with a full stop in text: Now scrollable instead of being a fixed size

Please fix these issues. For the correct format, refer to the pull request template.

@github-actions github-actions bot removed the Wrong Title/Changelog There is an error in the title or changelog label Nov 16, 2024
@hannibal002 hannibal002 added this to the Version 0.28 milestone Nov 17, 2024
@hannibal002 hannibal002 added the Soon This Pull Request will be merged within the next couple of betas label Nov 17, 2024
Copy link
Owner

@hannibal002 hannibal002 left a comment

Choose a reason for hiding this comment

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

After scrolling 2–3 times in the new year cake tracker, the size of lines visible at any time is changing by one line.

The last line "more items below" is already disappearing on the second last scroll, instead of just on the last scroll.

@DavidArthurCole
Copy link
Contributor Author

After scrolling 2–3 times in the new year cake tracker, the size of lines visible at any time is changing by one line.

The last line "more items below" is already disappearing on the second last scroll, instead of just on the last scroll.

The first issue is probably an underlying issue with the way that scrollList is choosing to render height, as the tips add their height to the renderY correctly. I can't replicate the second issue at all...

@Thunderblade73
Copy link
Contributor

After changing the size in the config and then opening the cake bag

SkyHanni 0.28.Beta.16: Caught a ClassCastException in CakeTracker at ChestGuiOverlayRenderEvent: java.lang.Float cannot be cast to java.lang.Integer
 
Caused by java.lang.ClassCastException: java.lang.Float cannot be cast to java.lang.Integer
	at SH.features.inventory.caketracker.CakeTracker.getMaxTrackerHeight(CakeTracker.kt:55)
	at SH.features.inventory.caketracker.CakeTracker.buildCakeRenderables(CakeTracker.kt:432)
	at SH.features.inventory.caketracker.CakeTracker.drawDisplay(CakeTracker.kt:392)
	at SH.features.inventory.caketracker.CakeTracker.reRenderDisplay(CakeTracker.kt:198)
	at SH.features.inventory.caketracker.CakeTracker.onBackgroundDraw(CakeTracker.kt:171)
	at SH.data.RenderData.onBackgroundDraw(RenderData.kt:52)
	at FML.common.eventhandler.EventBus.post(EventBus.java:140)

@Thunderblade73
Copy link
Contributor

The changes to the scrollable list don't account for the correct size. Which is not allowed.
The green box is the bounding box of the renderable (simply do a .renderbounds()).
image
image

Also a tip how to fix hannibals concern is to give it 1-2 pixel more space.

Copy link
Owner

@hannibal002 hannibal002 left a comment

Choose a reason for hiding this comment

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

I cant reproduce the original two bugs reported.

But the pixel error is visible and ideally fixed before merging the pr.

use this patch to see it clearly:

Patch
Index: src/main/java/at/hannibal2/skyhanni/utils/renderables/Renderable.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/main/java/at/hannibal2/skyhanni/utils/renderables/Renderable.kt b/src/main/java/at/hannibal2/skyhanni/utils/renderables/Renderable.kt
--- a/src/main/java/at/hannibal2/skyhanni/utils/renderables/Renderable.kt(revision 72dc1a5ed5bda9acccacda29313c7260fb81b0e6)
+++ b/src/main/java/at/hannibal2/skyhanni/utils/renderables/Renderable.kt(date 1733059457763)
@@ -998,8 +998,9 @@
             private val end get() = scroll.asInt() + height
 
             override fun render(posX: Int, posY: Int) {
+                val hovered = isHovered(posX, posY)
                 scroll.update(
-                    isHovered(posX, posY) && shouldAllowLink(true, bypassChecks),
+                    hovered && shouldAllowLink(true, bypassChecks),
                 )
 
                 var renderY = 0
@@ -1008,9 +1009,13 @@
 
                 // If showScrollableTipsInList is true, and we are scrolled 'down', display a tip indicating
                 // there are more items above
-                if (showScrollableTipsInList && scroll.asInt() > 0) {
+                if (hovered) {
                     width = maxOf(width, scrollUpTip.width)
-                    scrollUpTip.renderXAligned(posX, posY, width)
+                    if (showScrollableTipsInList && scroll.asInt() > 0) {
+                        scrollUpTip.renderXAligned(posX, posY, width)
+                    } else {
+                        string("§7§ono items above (dont scroll)").renderXAligned(posX, posY, width)
+                    }
                     GlStateManager.translate(0f, scrollUpTip.height.toFloat(), 0f)
                     renderY += scrollUpTip.height
                     virtualY += scrollUpTip.height

Copy link

github-actions bot commented Dec 5, 2024

This pull request has conflicts with the base branch "beta". Please resolve those so we can test out your changes.

@github-actions github-actions bot added the Merge Conflicts There are open merge conflicts with the beta branch. label Dec 5, 2024
@github-actions github-actions bot removed the Merge Conflicts There are open merge conflicts with the beta branch. label Dec 5, 2024
Copy link

github-actions bot commented Dec 5, 2024

Conflicts have been resolved! 🎉

Copy link
Owner

@hannibal002 hannibal002 left a comment

Choose a reason for hiding this comment

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

those few pixels will not break the pr

@hannibal002 hannibal002 merged commit e2c7ca7 into hannibal002:beta Dec 5, 2024
6 checks passed
@github-actions github-actions bot removed the Soon This Pull Request will be merged within the next couple of betas label Dec 5, 2024
@DavidArthurCole DavidArthurCole deleted the CakeTrackerImps branch December 5, 2024 01:45
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