-
Notifications
You must be signed in to change notification settings - Fork 263
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
#4961 #5008 #4954 Bug Fixes #5007
Conversation
RPTools#4954 User Overlays break with new statsheets
This overlay may be used in other functionality in the future so the name can't be hard coded like this |
Reverted usage of AppConstants.
edit |
@Jmr3366 At a glance this look good to me. Is this comment still true? |
I think it is all good but wanted you or Craig to QC it. |
@@ -117,6 +119,20 @@ public ConcurrentSkipListSet<HTMLOverlayManager> getOverlays() { | |||
return overlays.clone(); | |||
} | |||
|
|||
public HTMLOverlayManager init() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this approach to fixing #4954 actually masks the issue rather than fixing it. Did some testing, I believe the only thing that needs to change is in showOverlay()
- we need needsSorting
to be true even for internal overlays. Otherwise the new overlay ends up in front of the front
region.
Also I have a personal motivation to not have overlays automatically open 🙂 - see #5044. Overlays on my system cause a lot of input lag, and this would force me to endure it despite not using the stat sheet feature yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're correct on both points (masking,sorting) and I looked right past that, thank you sir!
sorting bool served no immediate purpose
Fixes:
#4961 NPE on mouse off (needs testing, I can't validate)
#5008 Stat sheet not in front
#4954 User Overlays break with new statsheets
Description of the Change
#4961 adds an empty var
if
check. Needs testing as I don't have the OS to validate this change on.#5008 changed variable used from min to max
#4954 forces sorting for all overlays (removed
needsSorting
bool)Possible Drawbacks
None
This change is