Improve error display logic for modules (#1124)

* Improve error display logic for modules

Module functions like onInit, onStart, or onDisable can return a (bool,
string) pair that act similarly to the returns of a pcall to trigger a
"soft error" for the module and display it as failed in the module
listing.

Some of our modules use this when gracefully disabling themselves, but
confusingly end up being displayed as errored in the module listing -
chief among them are the Blizzard nameplates when they've been disabled
due to the presence of other known nameplate addons.

We now support returning a module status enum code from the module
functions in-place of the leading boolean, which will force the status
of the module to appear as returned.

Additionally to better support the use case by Blizzard nameplates, a
new status code has been added for conflicted modules. These will
display as grey in the list rather than appearing as errors.

* Apply missing dependency change to chat and tooltip skin modules

---------

Co-authored-by: Solanya <[email protected]>

totalRP3/Locales/enUS.lua

@@ -528,6 +528,7 @@ The description doesn't have to be limited to |cnGREEN_FONT_COLOR:physical descr
 	CO_MODULES_STATUS_3 = "Total RP 3 update required",
 	CO_MODULES_STATUS_4 = "Error on initialization",
 	CO_MODULES_STATUS_5 = "Error on startup",
+	CO_MODULES_STATUS_6 = "Disabled due to conflict",
 	CO_MODULES_TT_NONE = "No dependencies";
 	CO_MODULES_TT_DEPS = "Dependencies";
 	CO_MODULES_TT_TRP = "%sFor Total RP 3 build %s minimum.|r",

totalRP3/Modules/ChatFrame/ElvUI.lua

@@ -9,18 +9,18 @@ local loc = TRP3_API.loc;
 local function onStart()
 	-- Stop right here if ElvUI is not installed
 	if not ElvUI then
-		return false, loc.MO_ADDON_NOT_INSTALLED:format("ElvUI");
+		return TRP3_API.module.status.MISSING_DEPENDENCY, loc.MO_ADDON_NOT_INSTALLED:format("ElvUI");
 	end
 
 	local ElvUI = ElvUI[1];
 	local ElvUIChatModule = ElvUI:GetModule("Chat", true);
 	if not ElvUIChatModule then
-		return false, "Your version of ElvUI doesn't need this module to function.";
+		return TRP3_API.module.status.MISSING_DEPENDENCY, "Your version of ElvUI doesn't need this module to function.";
 	end
 	local ElvUIGetColoredName = ElvUIChatModule.GetColoredName;
 
 	if not ElvUIGetColoredName then
-		return false, "Your version of ElvUI doesn't need this module to function.";
+		return TRP3_API.module.status.MISSING_DEPENDENCY, "Your version of ElvUI doesn't need this module to function.";
 	end
 
 	-- Build the fallback, using ElvUI's function
@@ -36,7 +36,7 @@ end
 
 -- Register a Total RP 3 module that can be disabled in the settings
 TRP3_API.module.registerModule({
-	["name"] = "ElvUI",
+	["name"] = "ElvUI Chat",
 	["description"] = loc.MO_CHAT_CUSTOMIZATIONS_DESCRIPTION:format("ElvUI"),
 	["version"] = 1.0,
 	["id"] = "trp3_elvui_chat",

totalRP3/Modules/ChatFrame/WIM.lua

@@ -9,7 +9,7 @@ local loc = TRP3_API.loc;
 local function onStart()
 	-- Stop right here if WIM is not installed
 	if not WIM then
-		return false, loc.MO_ADDON_NOT_INSTALLED:format("WIM");
+		return TRP3_API.module.status.MISSING_DEPENDENCY, loc.MO_ADDON_NOT_INSTALLED:format("WIM");
 	end
 
 	-- Import Total RP 3 functions

totalRP3/Modules/ModuleManagement.lua

@@ -22,7 +22,8 @@ TRP3_API.module.status = {
 	ERROR_ON_INIT = 2,
 	ERROR_ON_LOAD = 3,
 	DISABLED = 4,
-	OK = 5
+	OK = 5,
+	CONFLICTED = 6,
 };
 local MODULE_STATUS = TRP3_API.module.status;
 
@@ -139,12 +140,6 @@ end
 --- Invokes a given module function with any additional given parameters,
 --  capturing error information and returning it as if via pcall or xpcall.
 --
---  In release builds, error information is not reported via the global
---  error handler (as if it were called via pcall).
---
---  In debug builds, error information is forwarded to the global error
---  handler, and is additionally returned.
---
 --  If a function does not fail with an error but returns an explicit false,
 --  the function is treated as failed and any additional message is returned.
 --
@@ -154,14 +149,23 @@ end
 --
 --  @return true if no error occurred, false and an error message if not.
 local function callModuleFunction(module, funcName, ...)
-	local ok, message = false, nil;
-	securecallfunction(function(...) ok, message = module[funcName](...); end, ...);
+	local called = false;
+	local ok, message;
 
-	if ok == nil then
-		ok = true;
-		message = nil;
-	elseif ok == false and message == nil then
+	local function InvokeFunction(...)
+		ok, message = module[funcName](...);
+		called = true;
+	end
+
+	securecallfunction(InvokeFunction, ...);
+
+	if not called then
+		-- Execution of the function didn't complete, assume a script error.
+		ok = false;
 		message = loc.MO_SCRIPT_ERROR;
+	elseif called and ok == nil and message == nil then
+		-- The invoked function returned no values; assume success.
+		ok = true;
 	end
 
 	return ok, message;
@@ -192,7 +196,11 @@ function initModule(module)
 
 	-- Call the lifecycle function and update the module status on failure.
 	local ok, err = callModuleFunction(module, "onInit");
-	if not ok then
+
+	if type(ok) ~= "boolean" then
+		module.error = err;
+		module.status = ok;
+	elseif not ok then
 		module.error = err;
 		module.status = MODULE_STATUS.ERROR_ON_INIT;
 	end
@@ -223,7 +231,11 @@ function startModule(module)
 
 	-- Call the lifecycle function and update the module status on failure.
 	local ok, err = callModuleFunction(module, "onStart");
-	if not ok then
+
+	if type(ok) ~= "boolean" then
+		module.error = err;
+		module.status = ok;
+	elseif not ok then
 		module.error = err;
 		module.status = MODULE_STATUS.ERROR_ON_LOAD;
 	end
@@ -249,7 +261,11 @@ local function disableModule(module)
 
 	-- Call the lifecycle function and update the module status on failure.
 	local ok, err = callModuleFunction(module, "onDisable");
-	if not ok then
+
+	if type(ok) ~= "boolean" then
+		module.error = err;
+		module.status = ok;
+	elseif not ok then
 		module.error = err;
 		module.status = MODULE_STATUS.ERROR_ON_LOAD;
 	end
@@ -274,6 +290,8 @@ local function moduleStatusText(statusCode)
 		return "|cffff0000" .. loc.CO_MODULES_STATUS_5;
 	elseif statusCode == MODULE_STATUS.MISSING_DEPENDENCY then
 		return "|cff999999" .. loc.CO_MODULES_STATUS_0;
+	elseif statusCode == MODULE_STATUS.CONFLICTED then
+		return "|cff999999" .. loc.CO_MODULES_STATUS_6;
 	end
 	error("Unknown status code");
 end
@@ -335,6 +353,7 @@ local function GetSuggestedBorderColor(status)
 		[MODULE_STATUS.ERROR_ON_LOAD] = RED_BORDER_COLOR,
 		[MODULE_STATUS.DISABLED] = GREY_BORDER_COLOR,
 		[MODULE_STATUS.OK] = GREEN_BORDER_COLOR,
+		[MODULE_STATUS.CONFLICTED] = GREY_BORDER_COLOR,
 	};
 
 	return STATUS_BORDER_COLORS[status] or GOLD_BORDER_COLOR;

totalRP3/Modules/NamePlates/NamePlates_Blizzard.lua

@@ -85,7 +85,7 @@ local TRP3_BlizzardNamePlates = {};
 
 function TRP3_BlizzardNamePlates:OnModuleInitialize()
 	if not C_AddOns.IsAddOnLoaded("Blizzard_NamePlates") then
-		return false, L.NAMEPLATES_MODULE_DISABLED_BY_DEPENDENCY;
+		return TRP3_API.module.status.MISSING_DEPENDENCY, L.NAMEPLATES_MODULE_DISABLED_BY_DEPENDENCY;
 	end
 
 	-- Quick hack to make these nameplates "cowardly"; if any of the below
@@ -101,7 +101,7 @@ function TRP3_BlizzardNamePlates:OnModuleInitialize()
 
 	for _, addon in ipairs(addons) do
 		if TRP3_API.utils.IsAddOnEnabled(addon) then
-			return false, L.NAMEPLATES_MODULE_DISABLED_BY_EXTERNAL;
+			return TRP3_API.module.status.CONFLICTED, L.NAMEPLATES_MODULE_DISABLED_BY_EXTERNAL;
 		end
 	end
 
@@ -111,7 +111,7 @@ function TRP3_BlizzardNamePlates:OnModuleInitialize()
 	if ElvUI then
 		local E = ElvUI[1];
 		if E and E.NamePlates and E.NamePlates.Initialized then
-			return false, L.NAMEPLATES_MODULE_DISABLED_BY_EXTERNAL;
+			return TRP3_API.module.status.CONFLICTED, L.NAMEPLATES_MODULE_DISABLED_BY_EXTERNAL;
 		end
 	end
 end
@@ -454,6 +454,7 @@ TRP3_API.module.registerModule({
 	description = L.BLIZZARD_NAMEPLATES_MODULE_DESCRIPTION,
 	version = 1,
 	minVersion = 92,
+	requiredDeps = { {"Blizzard_NamePlates", "external"} },
 	onInit = function() return TRP3_BlizzardNamePlates:OnModuleInitialize(); end,
 	onStart = function() return TRP3_BlizzardNamePlates:OnModuleEnable(); end,
 });

totalRP3/Modules/TooltipSkins/ElvUI.lua

@@ -16,7 +16,7 @@ local function ResolveFrame(tbl, name, ...)
 end
 
 TRP3_API.module.registerModule({
-	["name"] = "ElvUI",
+	["name"] = "ElvUI Tooltips",
 	["id"] = "trp3_elvui",
 	["description"] = loc.MO_TOOLTIP_CUSTOMIZATIONS_DESCRIPTION:format("ElvUI"),
 	["requiredDeps"] = {
@@ -28,7 +28,7 @@ TRP3_API.module.registerModule({
 
 		-- Stop right here if ElvUI is not installed
 		if not ElvUI then
-			return false, loc.MO_ADDON_NOT_INSTALLED:format("ElvUI");
+			return TRP3_API.module.status.MISSING_DEPENDENCY, loc.MO_ADDON_NOT_INSTALLED:format("ElvUI");
 		end
 
 		local skinFrame, skinTooltips;

totalRP3/Modules/TooltipSkins/TinyTooltip.lua

@@ -13,7 +13,7 @@ TRP3_API.module.registerModule({
 
 		-- Check if the add-on TinyTooltip is installed
 		if not TinyTooltip then
-			return false,  loc.MO_ADDON_NOT_INSTALLED:format("TinyTooltip");
+			return TRP3_API.module.status.MISSING_DEPENDENCY,  loc.MO_ADDON_NOT_INSTALLED:format("TinyTooltip");
 		end
 
 		-- List of the tooltips we want to be customized by TinyTooltips

totalRP3/Modules/TooltipSkins/Tiptac.lua

@@ -13,7 +13,7 @@ TRP3_API.module.registerModule({
 
 		-- Stop right here if TipTac is not installed
 		if not TipTac then
-			return false, loc.MO_ADDON_NOT_INSTALLED:format("TipTac");
+			return TRP3_API.module.status.MISSING_DEPENDENCY, loc.MO_ADDON_NOT_INSTALLED:format("TipTac");
 		end
 
 		-- List of the tooltips we want to be customized by TipTac