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

fix initialization of icontransition. #5387

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

fix initialization of icontransition. #5387

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions LuaUI/Widgets/gui_icontransition.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ local renderAtPos = {}

-- Forward function declarations
local GotHotkeypress = function() end
local UpdateDynamic = function() end
local UpdateDynamic = function(shouldInit) end

-- Localized Spring functions
local echo = Spring.Echo
Expand Down Expand Up @@ -178,7 +178,7 @@ function GotHotkeypress()
target_mode = nil
kp_timer = nil
current_mode = "Dynamic"
UpdateDynamic()
UpdateDynamic(true)
else
waiting_on_double = true
kp_timer = spGetTimer()
Expand All @@ -194,7 +194,7 @@ function GotHotkeypress()
end
end

function UpdateDynamic()
function UpdateDynamic(shouldInit)
local cs = spGetCameraState()
local gy = spGetGroundHeight(cs.px, cs.pz)
testHeight = cs.py - gy
Expand All @@ -208,12 +208,12 @@ function UpdateDynamic()
if showing_icons and drawIcons then
drawIcons = false
end
if showing_icons and testHeight < options.icontransitiontop.value - tolerance then

if (shouldInit or showing_icons) and testHeight < options.icontransitiontop.value - tolerance then
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.

spSendCommands("disticon " .. 0)
showing_icons = true
end
Expand Down Expand Up @@ -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.


local allUnits = spGetAllUnits()
for _,unitID in pairs (allUnits) do
Expand Down