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

fix initialization of icontransition. #5387

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amnykon
Copy link
Contributor

@amnykon amnykon commented Dec 3, 2024

Variables need to be set when widget initializes and when toggling back to dynamic mode. The Initialization is important after the game begins (/luaUI reload).

spSendCommands("disticon " .. 100000)
showing_icons = false
drawIcons = true
elseif not showing_icons and testHeight > options.icontransitiontop.value + tolerance then
elseif (shouldInit or not showing_icons) and testHeight >= options.icontransitiontop.value + tolerance then
Copy link
Member

Choose a reason for hiding this comment

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

Note that here it's +tolerance and in the one above it's -tolerance so this will still fail to apply anything if testHeight happens to be within the tolerance bound

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem I have is sowing_icons is false (after switching from "Off" mode to "Dynamic") or nil (on initial widget load after /luaUI reload) and I have options.icontransitiontop maxed out. Since this code is written to toggle based on zoom height, the code never triggers for me once one of those two conditions is meet.

I have not had issues with the tolerances, thought I can see that could be a problem. I see 2 fixes for it.

  1. Remove tolerance altogether, as I don't think a dead zone of 50 really does much.
  2. Remove the tolerance for initial load only. If I do this, I think adding a setModeToDynamic function may be better, and just copy the logic over without the sowing_icons and tolerance.

which approach do you prefer?

Copy link
Member

Choose a reason for hiding this comment

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

setModeToDynamic function sounds good.

@@ -369,7 +369,7 @@ function widget:Initialize()
target_mode = nil
kp_timer = nil
current_mode = "Dynamic"
UpdateDynamic()
UpdateDynamic(true)
Copy link
Member

Choose a reason for hiding this comment

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

IMO instead of adding the bool param, do a manual check here without the tolerance bound.

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.

2 participants