verify Challenge Mode isn't active when toggling logging

[ljosberinn]

• fixes Autologging breaks logs in M+ keys #1330
• issues soon celebrating 2nd birthday so I figured I might aswell

[funkydude]

Does it stop logging after defeating the final boss?

[ljosberinn]

No, after the first boss

example log https://www.warcraftlogs.com/reports/8BkfD7t6yTpNrVmn#fight=last

[funkydude]

I'm talking about with your change, not what the current issue is.

[ljosberinn]

Right, unsure, doubt the event order is ENCOUNTER_END before CHALLENGE_MODE_END. A separate listener that watches CHALLENGE_MODE_END would be better suited. A zone-based approach using GetDifficutlyInfo would be better for dungons in general however.

[funkydude]

When submitting a PR, there's a general expectation that you've tested the code. If you don't know if logging stops after the last boss, then what scenario are you expecting to end logging?

[ljosberinn]

When filing a bug, there's a general expectation that the maintainers take action. This hasn't happened for nearly two years, the bug wasn't even recognized despite exhaustive examples.

I've outlined above that the logging approach is inherently flawed for dungeons as it shouldn't start/stop logging based on encounters regardless of M+ or not, a zone-based + GetDifficultyInfo approach yields more accurate results, I'm not in a position however to refactor this entire thing accordingly as I'm not familiar with the code. Stopping to log after the last boss is also incorrect, which you know, as it's somewhat common to pull the last missing count after encounter end, e.g. in Mists. Similarily, logging trash in raid is relevant for Classic speed runs.

My understanding was that filing the bug this would be fixed quickly, especially considering its technically a small change. Since that hasn't happened, I have at least tried to fix it and at the same time certainly won't brick logging my keys after this issue has been this long standing. Yet I'm met with a continued condescending tone, both here and on Discord.

Edit: looking further through the code, BigWigs doesn't listen to CHALLENGE_MODE_* events at all currently, which further complicates PR'ing this. What I came up with is:

# Pull.lua
	function plugin:OnPluginEnable()
		self:RegisterMessage("BigWigs_ProfileUpdate", updateProfile)
		updateProfile()

		self:RegisterMessage("BigWigs_PluginComm")
		self:RegisterMessage("DBM_AddonMessage")

		self:RegisterMessage("BigWigs_OnBossWin")
+		self:RegisterMessage("BigWigs_OnChallengeModeEnd")
		self:RegisterMessage("BigWigs_OnBossWipe", "BigWigs_OnBossWin")

		self:RegisterMessage("BigWigs_OnBossEngage")

		self:RegisterMessage("Blizz_StartCountdown")
		self:RegisterMessage("Blizz_StopCountdown")
	end
	
# ...
	
+ function plugin:BigWigs_OnChallengeModeEnd()
+	if isLogging then
+		isLogging = false
+		LoggingCombat(false)
+	end
+end

# Core.lua
if loader.isRetail or loader.isCata then
	function mod:ENCOUNTER_START(_, id)
		for _, module in next, bosses do
			if module:GetEncounterID() == id and not module:IsEnabled() then
				module:Enable()
				if UnitGUID("boss1") then -- Only if _START fired after IEEU
					module:Engage()
				end
			end
		end
	end

+	if loader.isRetail then
+		function mod:CHALLENGE_MODE_END()
+			self:SendMessage("BigWigs_OnChallengeModeEnd", self)
+		end
+	end
else
	function mod:ENCOUNTER_START(_, id)
		for _, module in next, bosses do
			if module:GetEncounterID() == id then
				if not module:IsEnabled() then
					module:Enable()
				end
				module:Engage()
			end
		end
	end
end

which would be something I'm willing to test if this is how it should be done.

[EKE00372]

Does it stop logging after defeating the final boss?

Main issue is that using ENCOUNTER_START and ENCOUNTER_END for logging is only suitable for raid. In mythic+, except the original issue (logging stops after first boss defeated), stop logging after defeating the final boss' isn't quite suitable also, because if the progress hasn't reached 100% after defeating the final boss, the log will be incompleted.

[funkydude]

PRs are a trust based relationship. We trust that you are operating in good faith. Comparing that to your disappointment and expectation of a bug fix is strange.

Regardless, when questioned on that trust, instead of providing reassurance, you decided to have a rant. So I think that's the end of this, as I don't see how we can realistically trust any changes you submit.