doadin / Plexus

Other
9 stars 2 forks source link

[Bug] My frame keeps moving on re-log. #69

Open rainx0r opened 1 year ago

rainx0r commented 1 year ago

Describe the bug What it says on the tin. My raid / party frames keep moving. Through debugging, I found this is always exactly 5 pixels off on each dimension each re-log. Say my frames are on PosX = -93, PosY=76, anchor="BOTTOMRIGHT", next re-log this will be -88, 71.

For whatever reason, this ONLY happens when re-logging, not when reloading.

To Reproduce I can give you my profile / SavedVariables and other things upon request,

I did manage to do a band-aid fix, but I'd like to use this issue to describe to you my scenario, testing and poking around work, and hopefully since you know your codebase better than me maybe you can derive some better, generally applicable solution!

The scenario

Every time I re-log, the following things happen:

My "band-aid" fix In PlexusLayout:SavePosition()

    ...
    if x and y and s then
        x, y = floor(x - 5 + 0.5), floor(y + 5 + 0.5)
        self.db.profile.PosX = x
        self.db.profile.PosY = y
        ...
    end

For whatever reason, this fixes the problem on re-log, and the values that get saved agree with the values there before / the ones RestorePosition originally used when loading the frame, and my frame does not move every re-log any more.

But then the problem starts happening on /reload instead. The problem did not happen on /reload before. However, this demonstrates that it's literally just 5 pixels off.

My second fix was this: PlexusLayout:PostEnable()

    -- position and scale frame
    self:RestorePosition()
    -- self:Scale()
    self.frame:SetScale(self.db.profile.scale)
    self:RestorePosition()

Now the frame works perfectly both on re-log and reload.

Potential clues

Desktop (please complete the following information):

doadin commented 1 year ago

I have had a few people say this happened to them in the past however, I myself have gone from 1080p to 4k and never noticed this bug so I was never really able to reproduce. Since you seem to have a fix that works for you I will try and see if it breaks anything in my setup where I have not seen this bug and if it works maybe I can just merge it, thank you for your thorough research into this.

I was never able to find out exactly what was happening because I could never get it to happen for me.

rainx0r commented 1 year ago

Honestly, I can only assume it is a bug on Blizzard's end, or it only happens under certain circumstances. For clarity, my fix is only the second code block I posted, the first code block should not be used, just it was the first thing I tried.

doadin commented 1 year ago

@evangelos-ch yes, I beleieve I follow what you said, I tested just loading in the game and loggging in/out a few times and /reload a few times and it seems to be fine, I just want to look into it a bit more before making it a release but if it works for you and me without issue then maybe I will push it out, let me know if it continues to work for you.

doadin commented 1 year ago

I was just looking and trying to track the code flow to see what is called when and by what, I was just thinking do we need double self:RestorePosition()? Or can

    -- position and scale frame
    self:RestorePosition()
    self.frame:SetScale(self.db.profile.scale)
    self:RestorePosition()

become

    -- position and scale frame
    self.frame:SetScale(self.db.profile.scale)
    self:RestorePosition()

I think the second still works no?

rainx0r commented 1 year ago

Probably?

    -- position and scale frame
    self:RestorePosition()
    self.frame:SetScale(self.db.profile.scale)

might work too. The frame is already created, just RestorePosition() calculates its coordinates given the coords and scale saved in the profile, and SetScale() just applies said saved scale to the frame, which can happen before or after it's positioned and it shouldn't matter. But you're right since none of the "dependent" variables of RestorePositon() change between these two calls, the duplicate call is probably not needed.

rainx0r commented 1 year ago

Just wanted to let you know that this fix has been working fine for me for over a month now, so I'd definitely recommend incorporating it into one of your new releases (would really save me the trouble of doing it manually when I update the addon from curseforge)

doadin commented 1 year ago

@evangelos-ch to be clear before I push this out

` -- position and scale frame

self.frame:SetScale(self.db.profile.scale)

self:RestorePosition()

`

or

` -- position and scale frame

self:RestorePosition()

self.frame:SetScale(self.db.profile.scale)

`

doadin commented 1 year ago

I was playing with the code change for a while and didn't notice any difference but almost forgot about the change with the other addon work i've been doing.

doadin commented 1 year ago

Fixed: https://github.com/doadin/Plexus/commit/02644768704e443d07a6316ccff5974699f31c86

doadin commented 1 year ago

reverted because others started saying this fix broke it for them, neither had any effect for me so idk.