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

Feature Flags! #4844

Merged
merged 20 commits into from
Feb 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/workflows/build-beta.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ jobs:
- name: Update Build Date
run: sed -i "s/@build-time@/`date +%Y%m%d%H%M%S`/" WeakAuras/Init.lua

- name: Flag Non-Experimental Build
run: |
sed -i -e "s/--@experimental@/--\[=====\[@experimental@/" WeakAuras/Init.lua \
-e "s/--@end-experimental@/--@end-experimental\]=====\]/" WeakAuras/Init.lua \
-e "s/--\[=====\[@non-experimental@/--@non-experimental@/" WeakAuras/Init.lua \
-e "s/--@end-non-experimental@\]=====\]/--@end-non-experimental/" WeakAuras/Init.lua

- name: Create Package
if: github.event_name == 'push' && contains(github.ref, 'refs/tags/')
uses: BigWigsMods/packager@master
Expand Down
7 changes: 7 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ jobs:
- name: Update Build Date
run: sed -i "s/@build-time@/`date +%Y%m%d%H%M%S`/" WeakAuras/Init.lua

- name: Flag Non-Experimental Build
run: |
sed -i -e "s/--@experimental@/--\[=====\[@experimental@/" WeakAuras/Init.lua \
Copy link
Contributor Author

@emptyrivers emptyrivers Feb 12, 2024

Choose a reason for hiding this comment

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

5 ='s, because bigwigs packager will use at most 4 ='s in its own @flag@ replacements

-e "s/--@end-experimental@/--@end-experimental\]=====\]/" WeakAuras/Init.lua \
-e "s/--\[=====\[@non-experimental@/--@non-experimental@/" WeakAuras/Init.lua \
-e "s/--@end-non-experimental@\]=====\]/--@end-non-experimental/" WeakAuras/Init.lua \

- name: Create Package
uses: BigWigsMods/packager@v2
env:
Expand Down
7 changes: 7 additions & 0 deletions .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ jobs:
- name: Update Build Date
run: sed -i "s/@build-time@/`date +%Y%m%d%H%M%S`/" WeakAuras/Init.lua

- name: Flag Experimental Build
run: |
sed -i -e "s/--\[=====\[@experimental@/--@experimental@/" WeakAuras/Init.lua \
-e "s/--@end-experimental@\]=====\]/--@end-experimental/" WeakAuras/Init.lua \
-e "s/--@non-experimental@/--\[=====\[@non-experimental@/" WeakAuras/Init.lua \
-e "s/--@end-non-experimental@/--@end-non-experimental\]=====\]/" WeakAuras/Init.lua

- name: Create Package
uses: BigWigsMods/packager@v2
with:
Expand Down
163 changes: 163 additions & 0 deletions WeakAuras/Features.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
---@type string, Private
local addon, Private = ...

---@alias BuildType "dev" | "pr" | "alpha" | "beta" | "release"

---@class feature
---@field id string
---@field autoEnable? BuildType[]
---@field requiredByAura? fun(self: self, aura: auraData): boolean
---@field enabled? boolean
---@field persist? true
---@field sub? SubscribableObject

---@class Features
---@field private db? table<string, boolean>
---@field private __feats table<string, feature>
---@field private hydrated boolean
local Features = {
__feats = {},
hydrated = false,
}
Private.Features = Features

---@param id string
function Features:Exists(id)
return self.__feats[id] ~= nil
end

---@param id string
function Features:Enabled(id)
return self.hydrated and self:Exists(id) and self.__feats[id].enabled
end

---@param id string
function Features:Enable(id)
if not self:Exists(id) then return end
if not self.hydrated then
error("Cannot enable a feature before hydration", 2)
elseif not self.__feats[id].enabled then
self.__feats[id].enabled = true
if self.__feats[id].persist then
self.db[id] = true
end
self.__feats[id].sub:Notify("Enable")
end
end

---@param id string
function Features:Disable(id)
if not self:Exists(id) then return end
if not self.hydrated then
error("Cannot disable a feature before hydration", 2)
elseif self.__feats[id].enabled then
self.__feats[id].enabled = false
if self.__feats[id].persist then
self.db[id] = false
end
self.__feats[id].sub:Notify("Disable")
end
end

---@return {id: string, enabled: boolean}[]
function Features:ListFeatures()
if not self.hydrated then return {} end
local list = {}
for id, feature in pairs(self.__feats) do
table.insert(list, {
id = id,
enabled = feature.enabled
})
end
table.sort(list, function(a, b)
return a.id < b.id
end)
return list
end

function Features:Hydrate()
self.db = Private.db.features
for id, feature in pairs(self.__feats) do
local enable = false
if self.db[id] ~= nil then
enable = self.db[id]
else
for _, buildType in ipairs(feature.autoEnable or {}) do
if WeakAuras.buildType == buildType then
enable = true
break
end
end
end
feature.enabled = enable
end
self.hydrated = true
for _, feature in pairs(self.__feats) do
-- cannot notify before hydrated flag is set, or we risk consumers getting wrong information
feature.sub:Notify(feature.enabled and "Enable" or "Disable")
end
end

---@param feature feature
function Features:Register(feature)
if self.hydrated then
error("Cannot register a feature after hydration", 2)
end
if not self.__feats[feature.id] then
self.__feats[feature.id] = feature
feature.sub = Private.CreateSubscribableObject()
end
end

---@param id string
---@param enabledFunc function
---@param disabledFunc? function
---hide a code path behind a feature flag,
---optionally provide a disabled path
function Features:Wrap(id, enabledFunc, disabledFunc)
return function(...)
if self:Enabled(id) then
return enabledFunc(...)
else
if disabledFunc then
return disabledFunc(...)
end
end
end
end

---@param data auraData
---@return boolean, table<string, boolean>
function Features:AuraCanFunction(data)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Presumably we'd communicate to the user if an aura they had required a disabled feature to function properly, but that sounds touchy to do & i figured we would cross that bridge when we got there

local enabled = true
local reasons = {}

for _, feature in pairs(self.__feats) do
if feature.requiredByAura and not feature:requiredByAura(data) then
enabled = false
reasons[feature.id] = false
end
end

return enabled, reasons
end

---@param id string
---@param enable function
---@param disable function
function Features:Subscribe(id, enable, disable)
local tbl = {
Enable = enable,
Disable = disable
}
if self:Exists(id) then
self.__feats[id].sub:AddSubscriber("Enable", tbl)
self.__feats[id].sub:AddSubscriber("Disable", tbl)
end
end


Features:Register({
id = "debug",
autoEnable = {"dev"}
})
20 changes: 20 additions & 0 deletions WeakAuras/Init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ local GetAddOnMetadata = C_AddOns and C_AddOns.GetAddOnMetadata or GetAddOnMetad
--- @field ExecEnv table
--- @field event_prototypes table<string, prototypeData>
--- @field event_categories table<string, {name: string, default: string }>
--- @field Features Features
--- @field FindUnusedId fun(prefix: string?): string
--- @field FixGroupChildrenOrderForGroup fun(data: auraData)
--- @field frames table<string, table>
Expand Down Expand Up @@ -371,9 +372,28 @@ local flavor = flavorFromTocToNumber[flavorFromToc]
if versionStringFromToc == "@project-version@" then
versionStringFromToc = "Dev"
buildTime = "Dev"
WeakAuras.buildType = "dev"
end
--@end-debug@

--[=====[@experimental@
WeakAuras.buildType = "pr"
--@end-experimental@]=====]

--@non-experimental@
--[[@alpha@
WeakAuras.buildType = "alpha"
--@end-alpha]]

--[[@non-alpha@
InfusOnWoW marked this conversation as resolved.
Show resolved Hide resolved
if versionString:find("beta") then
WeakAuras.buildType = "beta"
else
WeakAuras.buildType = "release"
end
--@end-non-alpha@]]
--@end-non-experimental@

WeakAuras.versionString = versionStringFromToc
WeakAuras.buildTime = buildTime
WeakAuras.newFeatureString = "|TInterface\\OptionsFrame\\UI-OptionsFrame-NewFeatureIcon:0|t"
Expand Down
52 changes: 48 additions & 4 deletions WeakAuras/WeakAuras.lua
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,9 @@ function SlashCmdList.WEAKAURAS(input)

for v in string.gmatch(input, "%S+") do
if not msg then
msg = v
msg = v:lower()
else
insert(args, v)
insert(args, v:lower())
end
end

Expand All @@ -193,6 +193,46 @@ function SlashCmdList.WEAKAURAS(input)
Private.PrintHelp();
elseif msg == "repair" then
StaticPopup_Show("WEAKAURAS_CONFIRM_REPAIR", nil, nil, {reason = "user"})
elseif msg == "ff" or msg == "feat" or msg == "feature" then
if #args < 2 then
local features = Private.Features:ListFeatures()
local summary = {}
for _, feature in ipairs(features) do
table.insert(summary, ("|c%s%s|r"):format(feature.enabled and "ff00ff00" or "ffff0000", feature.id))
end
prettyPrint(L["Syntax /wa feature <toggle|on|enable|disable|off> <feature>"])
prettyPrint(L["Available features: %s"]:format(table.concat(summary, ", ")))
else
local action = ({
toggle = "toggle",
on = "enable",
enable = "enable",
disable = "disable",
off = "disable"
})[args[1]]
if not action then
prettyPrint(L["Unknown action %q"]:format(args[1]))
else
local feature = args[2]
if not Private.Features:Exists(feature) then
prettyPrint(L["Unknown feature %q"]:format(feature))
elseif not Private.Features:Enabled(feature) then
if action ~= "disable" then
Private.Features:Enable(feature)
prettyPrint(L["Enabled feature %q"]:format(feature))
else
prettyPrint(L["Feature %q is already disabled"]:format(feature))
end
elseif Private.Features:Enabled(feature) then
if action ~= "enable" then
Private.Features:Disable(feature)
prettyPrint(L["Disabled feature %q"]:format(feature))
else
prettyPrint(L["Feature %q is already enabled"]:format(feature))
end
end
end
end
else
WeakAuras.OpenOptions(msg);
end
Expand Down Expand Up @@ -1159,7 +1199,6 @@ end
function Private.Login(initialTime, takeNewSnapshots)
local loginThread = coroutine.create(function()
Private.Pause();

if db.history then
local histRepo = WeakAuras.LoadFromArchive("Repository", "history")
local migrationRepo = WeakAuras.LoadFromArchive("Repository", "migration")
Expand All @@ -1173,6 +1212,10 @@ function Private.Login(initialTime, takeNewSnapshots)
coroutine.yield();
end


Private.Features:Hydrate()
Copy link
Contributor Author

@emptyrivers emptyrivers Jan 30, 2024

Choose a reason for hiding this comment

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

Hydration of feature flags is timed so that it should be the first thing that happens on PLAYER_LOGIN (the bit of code above is a saved variables migration that would only happen if a user played for a brief period in 2019 but then quit until now). So, even if a user opts into a persisted feature, the code will still briefly think the feature is disabled until PLAYER_LOGIN.

coroutine.yield()

local toAdd = {};
loginFinished = false
loginMessage = L["Options will open after the login process has completed."]
Expand Down Expand Up @@ -1265,7 +1308,7 @@ loadedFrame:SetScript("OnEvent", function(self, event, addon)
if(addon == ADDON_NAME) then
WeakAurasSaved = WeakAurasSaved or {};
db = WeakAurasSaved;

Private.db = db
-- Defines the action squelch period after login
-- Stored in SavedVariables so it can be changed by the user if they find it necessary
db.login_squelch_time = db.login_squelch_time or 10;
Expand All @@ -1278,6 +1321,7 @@ loadedFrame:SetScript("OnEvent", function(self, event, addon)

db.displays = db.displays or {};
db.registered = db.registered or {};
db.features = db.features or {}
db.migrationCutoff = db.migrationCutoff or 730
db.historyCutoff = db.historyCutoff or 730

Expand Down
3 changes: 2 additions & 1 deletion WeakAuras/WeakAuras.toc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ ArchiveTypes\Repository.lua
DefaultOptions.lua

# Core files
SubscribableObject.lua
Features.lua
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure exactly how to order the toc - since i'm using SubscribableObject obviously that needs to go before Features, but theoretically we could write feature flagged code at any point. So i just put it near the beginning, and we can muck with load order later if its a problem

Atlas_Retail.lua
Types_Retail.lua
Types.lua
Expand All @@ -60,7 +62,6 @@ AuraEnvironment.lua
AuraEnvironmentWrappedSystems.lua
DebugLog.lua
Dragonriding.lua
SubscribableObject.lua

# Region support
RegionTypes\SmoothStatusBarMixin.lua
Expand Down
3 changes: 2 additions & 1 deletion WeakAuras/WeakAuras_Vanilla.toc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ ArchiveTypes\Repository.lua
DefaultOptions.lua

# Core files
SubscribableObject.lua
Features.lua
Atlas_Vanilla.lua
Types_Vanilla.lua
Types.lua
Expand All @@ -51,7 +53,6 @@ AuraWarnings.lua
AuraEnvironment.lua
AuraEnvironmentWrappedSystems.lua
DebugLog.lua
SubscribableObject.lua

# Region support
RegionTypes\SmoothStatusBarMixin.lua
Expand Down
3 changes: 2 additions & 1 deletion WeakAuras/WeakAuras_Wrath.toc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ ArchiveTypes\Repository.lua
DefaultOptions.lua

# Core files
SubscribableObject.lua
Features.lua
Atlas_Wrath.lua
Types_Wrath.lua
Types.lua
Expand All @@ -54,7 +56,6 @@ AuraWarnings.lua
AuraEnvironment.lua
AuraEnvironmentWrappedSystems.lua
DebugLog.lua
SubscribableObject.lua

# Region support
RegionTypes\SmoothStatusBarMixin.lua
Expand Down