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

Bag Frames Don't Update On Category Change #124

Closed
doadin opened this issue Jul 22, 2024 · 7 comments
Closed

Bag Frames Don't Update On Category Change #124

doadin opened this issue Jul 22, 2024 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@doadin
Copy link
Owner

doadin commented Jul 22, 2024

notes:

From what I noticed in testing function Baggins:Baggins_CategoriesChanged() calls function Baggins:Baggins_RefreshBags() and at least for the "temp disable cmopression" option bags did not change unless I commented out "sectionframe.dirty" part of function Baggins:Baggins_RefreshBags() . Not sure if this would help on category change or if its a "proper" fix.

@doadin
Copy link
Owner Author

doadin commented Jul 22, 2024

@Elberet
what if we changed:
https://github.com/doadin/Baggins/blob/main/Baggins-Options.lua#L3703 to:

local function disableCompressionTemp()
    Baggins.tempcompressnone = not Baggins.tempcompressnone
    Baggins:RebuildSectionLayouts()
    for bagid,bagframe in pairs(Baggins.bagframes) do
        for _,sectionframe in pairs(bagframe.sections) do
            sectionframe.dirty = true
        end
    end
    Baggins:Baggins_RefreshBags()
end

and

https://github.com/doadin/Baggins/blob/main/Baggins-Options.lua#L3810 to:

local function setArgValue(info, value)
    info.arg[info[#info]] = value
    for bagid,bagframe in pairs(Baggins.bagframes) do
        for _,sectionframe in pairs(bagframe.sections) do
            sectionframe.dirty = true
        end
    end
    Baggins:Baggins_RefreshBags()
end

@Elberet
Copy link
Contributor

Elberet commented Jul 22, 2024

Is that enough, tho? Suppose a character has dozens of stacks of cloth in their bag, wouldn't disabling compression almost certainly require resizing the bag frame? And if you also mark all bag frames as dirty, why not just call Baggins:UpdateBags instead?

Alternatively, this appears to work, and requires much less new code:

diff --git a/Baggins-Options.lua b/Baggins-Options.lua
index d2fcf65..f9ca85b 100644
--- a/Baggins-Options.lua
+++ b/Baggins-Options.lua
@@ -3703,7 +3703,7 @@ end
 local function disableCompressionTemp()
     Baggins.tempcompressnone = not Baggins.tempcompressnone
     Baggins:RebuildSectionLayouts()
-    Baggins:Baggins_RefreshBags()
+    Baggins:Baggins_RefreshBags(true)
 end
 
 local function openBagCategoryConfig()
@@ -3809,6 +3809,7 @@ end
 
 local function setArgValue(info, value)
     info.arg[info[#info]] = value
+    Baggins:Baggins_RefreshBags(true)
 end
 
 local function moveSection(info, down)
diff --git a/Baggins.lua b/Baggins.lua
index 5da098a..88515b2 100644
--- a/Baggins.lua
+++ b/Baggins.lua
@@ -558,7 +558,7 @@ function Baggins:OnEnable()
 end
 
 function Baggins:Baggins_CategoriesChanged()
-    self:Baggins_RefreshBags()
+    self:Baggins_RefreshBags(true)
     self.doInitialBankUpdate = true
 end
 
@@ -796,8 +796,8 @@ function Baggins:ScheduleRefresh()
     end
 end
 
-function Baggins:Baggins_RefreshBags()
-    if self.dirtyBags then
+function Baggins:Baggins_RefreshBags(forceRelayout)
+    if self.dirtyBags or forceRelayout then
         --Baggins:Debug('Updating bags')
         self:ReallyUpdateBags()
     end

@doadin
Copy link
Owner Author

doadin commented Jul 23, 2024

@Elberet It was not enough to make the frames adjust, no but I think enough to update the sections for new settings. In my testing Baggins:RunBagUpdates() has a huge performance hit.(i was trying to improve this function as can be seen here: 6f1ca07) which cause some recent issues. Not sure about Baggins:ReallyUpdateBags() I am going to make those changes and test, thanks!

@doadin
Copy link
Owner Author

doadin commented Jul 23, 2024

Also im noticing you change Baggins_RefreshBags to take a force arg but don't set any frame or section as "dirty". does that not mean that none of the rest of the code is run other than FireSignal? could we not just run ReallyUpdateBags instead directly?

@doadin
Copy link
Owner Author

doadin commented Jul 23, 2024

@Elberet Instead of modifying Baggins_RefreshBags I tried replacing the call to the function with just Baggins:ReallyUpdateBags() (no call to Baggins_RefreshBags needed) instead and it seems to update just fine as well, maybe you could help verify this?

@doadin
Copy link
Owner Author

doadin commented Jul 23, 2024

These are the changes: 4d575f9

@Elberet
Copy link
Contributor

Elberet commented Jul 23, 2024

Looks good. I've added a similar call to the equipment set filter, see #126

@doadin doadin closed this as completed Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants