View Single Post
10-31-16, 04:03 AM   #4
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
You can check for normal vs instance (premade) group by passing a specifier to IsInRaid:
http://wow.gamepedia.com/API_IsInRaid

However, your code will currently not work, for several reasons. Most importantly:

Code:
if method ~= master or freeforall then
This is the equivalent of saying "if the value of the method variable is different than the value of the master variable, or if the value of the freeforall variable is not false/nil, then..." which is not what you want at all. The words "master" and "freeforall" should be quoted, since they are strings, not variables. Also, you need to explicitly compare each one against the method variable; Lua doesn't have a shorthand "compare X against multiple values" syntax. Here's the correct syntax:

Code:
if method ~= "master" or method ~= "freeforall" then
Less importantly, you probably don't want to check like this at this level anyway. Currently, if you become the leader of a guild raid, and the loot method is already "freeforall", then your code will not change it to "master". Likewise, if the method is "master" and someone from outside the guild joins (making it no longer a guild raid) then your code will not change the loot method to "freeforall".

Apart from that, some other issues / suggestions / hints:
  • Indentation. Yours is mostly missing, and makes your code very hard to read. Each "if/end" statement should add another level of indentation for everything inside of it. You shouldn't write "then" keywords on a separate line, but if you really want to for some reason, at least be consistent and do it everywhere (but really, just don't do it!).
  • PARTY_LOOT_METHOD_CHANGED is not the correct event to listen for in your case; it will not fire when you become the group leader, or when your group becomes a guild/non-guild group, etc. Use GROUP_ROSTER_UPDATE like in your original code. That is the correct event to use when you want to know about group state changes.
  • You don't need to check the event inside your handler. Your frame only registered for one event, so it will never receive any other events.
  • You don't need to define the event handler function outside of the SetScript call. The only reason to do so would be to make it easier to call the same function from some other place in your code.
  • You don't need to check both IsInRaid() and UnitInRaid("player") -- they both do (essentially) the same thing. Just check IsInRaid().
  • You shouldn't define the method variable until after you've checked if you're the leader in a raid -- if you're not, then you won't use this variable, so there's no reason to have spent CPU time calling GetLootMethod() and memory storing its return value in the variable at that point. Generally speaking, you should always define variables in the most limited scope possible.
  • You should also check, and not call SetLootMethod again, if the current loot method is already what you want.
  • Since you only use the first value returned by GetLootMethod, and you only use it once, there's no need to assign it to a variable; just call it directly in the comparison statement.
  • You can just store the value from UnitName("player") in a variable, so you don't have to call it again each time.
  • Since you know your event doesn't have any arguments, and you don't use the self and event arguments, you can save some typing by not naming any arguments in the function declaration.

Here's an updated version:

Code:
local player = UnitName("player")

local frame = CreateFrame("Frame")
frame:RegisterEvent("GROUP_ROSTER_UPDATED")
frame:SetScript("OnEvent", function()
	if IsInRaid(LE_INSTANCE_TYPE_HOME) and UnitIsGroupLeader("player") then
		local wanted = InGuildParty() and "master" or "freeforall"
		if GetLootMethod() ~= wanted then
			SetLootMethod(wanted, player)
		end
	end
end)
(Passing a player name to SetLootMethod has no effect if the method isn't "master", so you don't need two different code paths to call it with or without the second argument.)
__________________
Retired author of too many addons.
Message me if you're interested in taking over one of my addons.
Don’t message me about addon bugs or programming questions.
  Reply With Quote