Skip to content

Feature winners popup#26

Draft
sica42 wants to merge 31 commits into
obszczymucha:masterfrom
sica42:feature-winners-popup
Draft

Feature winners popup#26
sica42 wants to merge 31 commits into
obszczymucha:masterfrom
sica42:feature-winners-popup

Conversation

@sica42

@sica42 sica42 commented Feb 23, 2025

Copy link
Copy Markdown
Contributor

Think this is pretty much complete now. A couple of notes:

  1. I've added some new elements to GuiElements, but don't think winners_header and winner really belongs there. Maybe I should move them to a separate WinnersPopupGuiElements file? If I do that I could probably move some of the dropdown/checkbox stuff currently in WinnersPopup into that file too.
    Edit I did the split, so all gui elements are in a separate file now.
  2. I really like my new tiny_button :) It supports both modern and classic look and defaults to a close button. I tried using it in OgLootFrameSkin, and it worked perfectly :) One main difference from the close button you currently have is that the frame is only the size of the actual button.
  3. Also implemented tracking of raid trades which seems to work pretty good with the little testing I've done.
  4. In LoowAwardCallback, I'm getting roll values from roll_tracker now (instead of WinnerTracker), but am unsure if local _, current_iteration = roll_tracker.get() will get the correct iteration in case of ties as I've not tested it with any tie results.
    Edit Have confirmed that tie rolls work correctly.
  5. The popup supports classic look, but I've not given the scrollbar the classic look. Not sure if it's really needed and it would require a rewrite of the scrollbar as the classic scrollbar works a bit different and has arrow buttons on top/bottom as well.
    If I do a rewrite, I could maybe try to integrate it into the FrameBuilder?
  6. There's a new keep_award_data option that will prevent the addon from clearing awards data in case of disconnects. Currently not any way to set it tho as I had that in the options GUI.
  7. No new tests atm. The raid trade tracker could maybe use a test?

@sica42 sica42 marked this pull request as draft February 23, 2025 22:35

m.map( winners, function( winner )
winner_tracker.track( winner.name, item.link, winner.roll_type, winner.roll, strategy ) -- TODO: remove from here and subscribe to the event.
winner_tracker.track( winner.name, item.link, winner.roll_type, winner.winning_roll, strategy ) -- TODO: remove from here and subscribe to the event.

@sica42 sica42 Feb 24, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't use roll value from winner_tracker anymore, but this seems like a bug anyway. winner only has winning_roll and not roll

@obszczymucha obszczymucha mentioned this pull request Feb 25, 2025
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.

1 participant