Skip to content

📦 NEW: wb housing databar#228

Open
billriess wants to merge 1 commit into
Toxicom:developmentfrom
billriess:development
Open

📦 NEW: wb housing databar#228
billriess wants to merge 1 commit into
Toxicom:developmentfrom
billriess:development

Conversation

@billriess

Copy link
Copy Markdown
Contributor

Closes #227

Summary of Changes

  1. Adds support for housing to wb databar.

Description

  • Needs review.

@Toxicom Toxicom left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good, small housekeeping notes. I can do them myself later too, let me know.

function DB:OnEvent(event, ...)
if event == "PLAYER_ENTERING_WORLD" then
-- I don't like this but its a workaround for fresh logins where the housing data isn't available yet. Might be a better solution.
C_Timer.After(3, function()

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

use E:Delay(), same syntax and same functionality, just a preference

-- If smart mode changes, force update
function DB:OnEvent(event, ...)
if event == "PLAYER_ENTERING_WORLD" then
-- I don't like this but its a workaround for fresh logins where the housing data isn't available yet. Might be a better solution.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Have you looked at /etrace to see if there's an event that fires when housing data is ready?


if self:UpdateSmartMode() then event = "ELVUI_FORCE_UPDATE" end

if event == "HOUSE_LEVEL_FAVOR_UPDATED" and TXUI.IsRetail then

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Event is registered only for Retail, so additional and TXUI.IsRetail is unnecessary imo


function DB:OnClick()
if self.mode == DB.const.mode.housing and TXUI.IsRetail then
if not _G.HousingDashboardFrame or not _G.HousingDashboardFrame:IsShown() then _G.HousingFramesUtil.ToggleHousingDashboard() end

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We don't need to check for IsShown(). The function toggles the dashboard eitherway, should allow the user to both open and close the dashboard via click.

end

function DB:OnClick()
if self.mode == DB.const.mode.housing and TXUI.IsRetail then

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Unnecessary and TXUI.IsRetail

DT.tooltip:Show()
end

function DB:UpdateHousingTooltip()

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I don't see the OnClick event reflected in the housing tooltip

and self.const.mode.exp
or self.const.mode.rep
local mode
if self.db.mode == "housing" and TXUI.IsRetail then

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
if self.db.mode == "housing" and TXUI.IsRetail then
if self.db.mode == "housing" then

if self.db.infoEnabled then
self.infoText:SetText(E:Round((self.mode == DB.const.mode.exp) and self.data.expPercentage or self.data.repPercentage) .. "%")
self.infoText:SetText(
E:Round((self.mode == DB.const.mode.exp) and self.data.expPercentage or (self.mode == DB.const.mode.housing) and self.data.housingPercentage or self.data.repPercentage)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Line is getting difficult to read, can we move the logic outside the E:Round() function for better readability?

"UPDATE_EXHAUSTION",
"UPDATE_FACTION",
"COMBAT_TEXT_UPDATE",
"CVAR_UPDATE",

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Since CVAR_UPDATE is currently used exclusively for housing, let's move it to retail's table

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.

WB Housing Databar

2 participants