Verubato / framesort

A simple WoW add-on that sorts party, arena, and raid frames.
9 stars 3 forks source link

Feature request: frame spacing #6

Closed Verubato closed 1 year ago

Verubato commented 1 year ago

Add a feature to configure some padding/spacing between frames. e.g. bottom padding of 10 would add 10 points of padding at the bottom of each frame to add some spacing.

XyzKangUI commented 1 year ago

Currently running a test without too many options and done this:

Result: image

The FrameSort experimental mode truly has been a bliss.

Verubato commented 1 year ago

What approach did you take to achieve this? Did you modify the values passed to source.Frame:SetPoint(...)?

XyzKangUI commented 1 year ago

What approach did you take to achieve this? Did you modify the values passed to source.Frame:SetPoint(...)?

Yes, I did. Here is how i've done it:

local spacing = 10
local spacingStep = -10

function addon:ShuffleFrames(orderedUnits, framesByUnit, framesByIndex)
    -- probably too complicated to calculate positions due to the whole flow container layout logic
    -- so instead we can just re-use the existing positions and shuffle them
    -- probably safer and better supported this way anyway
    local prevSpacing = 0
    for i = 1, #orderedUnits do
        local sourceUnit = orderedUnits[i]
        local source = framesByUnit[sourceUnit]
        local target = framesByIndex[i]

        assert(source ~= nil)
        assert(target ~= nil)

        source.Frame:ClearAllPoints()

        local unitSpacing = prevSpacing + spacing
        prevSpacing = prevSpacing + spacingStep

        for j = 1, #target.Points do
            local point = target.Points[j]

            point[5] = point[5] + unitSpacing
            source.Frame:SetPoint(unpack(point))
        end
    end
end
XyzKangUI commented 1 year ago

I notice now that the above snippet might look good in a party, but in a raid its will unalign the 6th/11/16/21/26/31/36 frame. Guess ill need to take the remainder of the division i-1 % 5 or something.

Verubato commented 1 year ago

Thanks for the code snippet! That is very handy for me.

Think I'd also need to handle:

Bit harder than I originally thought.

I will get started on it!

XyzKangUI commented 1 year ago

A few notes:

  1. It would be nice to hide the CompactPartyFrame sliders for wrath users
  2. The assertion on pets throws an error all the time - maybe add a raidOptionDisplayPets cvar check?
  3. The spacing doesn't persist through reload.
  4. when hiding group 2-8 and only showing the first it will not create the vertical spacing. It only works when there are minimal 2 groups shown. Meaning that the vertical spacing won't either work in a normal party of 5 people.
Verubato commented 1 year ago

Thanks for the preliminary testing and feedback! I've just pushed some more commits and still have more to go.

I've got Retail working pretty nicely but it seems the same code doesn't work well at all for Wotlk. I'll get Wotlk working and then figure out what code can be made reusable between the two.

It's turning out to be fairly tricky as there are lots of things to cater for:

Who would have thought a small spacing feature would be so tricky!

Verubato commented 1 year ago

Would you happen to know if it's safe to store values on blizzard objects?

    -- TODO: is it safe to store our properties on a blizzard object?
    if not frame.FrameSort then
        frame.FrameSort = {
            OriginalPosition = currentPosition,
            CurrentPosition = currentPosition
        }
    end

So far it's not caused any issues, and I don't think it should cause any taint as it's nothing that blizzard will use, so I think it should be fine?

XyzKangUI commented 1 year ago

Should be fine. Sorry for the preliminary tests.

Verubato commented 1 year ago

Nah it's fine, you've been very helpful to me.

Verubato commented 1 year ago

I've made further progress on this with spacing added to Wotlk.

If you have some time it'd be great if you can take a look and let me know what issues you see? I've only been able to test with my 3 PTR accounts so I'm unsure how it goes with say 10+ members (do you have any way to check?).

Known issues from me are:

XyzKangUI commented 1 year ago

When keep groups together is enabled it will convert the frames to CompactPartyFrame / CompactRaidGroup(?). I believe you cannot sort these in Wrath. Anyway ill see what happends with 10+ members soon, will keep you posted in aprox 1-2h.

XyzKangUI commented 1 year ago

When you show more than 5 frames (e.g. in a raid with multiple groups) it will not do the 'vertical' sorting while horizontal still works. However just enabling 1 group and hiding the other groups will restore the vertical sorting functionality, while it breaks the horizontal sorting. There are no errors.

TLDR: With 5 frames showing horizontal spacing doesn't work, while vertical works. When there are more than 5 frames showing horizontal spacing works, vertical doesn't.

I can test it whenever you want, if you are in no rush. Otherwise you could probably invite offline units (relogging characters) to get 5+ members on PTR.

Edit: Actually I notice now that its related to the rows. If you increase the containerframe to fit lets say 10 frames per row it will create a spacing for the first row, but the 2nd row has no spacing.

XyzKangUI commented 1 year ago

I also notice now that pet frames somehow create a gap after the sorting been re- applied for some reason. I guess it happened when a warlock died and got resurrected at the GY and his pet frame was created again? Funny thing is I play with pet frames disabled

Verubato commented 1 year ago

Thanks for that. I've made more fixes and think I've got it working fairly nicely now with the exception of pets (that is on my to-do list).

Verubato commented 1 year ago

Pets are becoming very painful to get working and there's still some fixes/work I need to do in that area. I'm thinking of making a release without pet spacing being fully functional as it might take me a while to figure that out.

I'd be grateful if you could try out the latest changes and see if you discover any non-pet issues?

Verubato commented 1 year ago

Published release 3.3.0 with this feature in it. I'll keep this ticket open for a little bit longer in case XyzKang find's any bugs/issues. Regarding pets, I'll working on getting them working nicely eventually.

XyzKangUI commented 1 year ago

It is looking great, only one thing I've noticed is that when you hide groups (e.g. 2-8) it requires an update. Great job btw.