Guild Management addon help

[Caellian]

Hi,

So, here's an addon i've written to help my wife manage her HUGE guild since between raiding and work, there isn't much time left.

Here's the code, i'll explain the issues after.

http://pastebin.com/WLhBsPpQ

It has been working quite well for the past month or so but it has one issue i am unable to fix myself.

I would like a way to feed whoever needs to be promoted or kicked into a table and then do what needs to be done on each of the elements of that table 1 by 1 on a timer (1 or 2 sec between each action) and then obviously remove what's done from the table.

Can anyone help me with this ?

Thanks

[Phanx]

if not (myChars or herChars) then return end

Based on this line, the code you posted will never run at all, since myChars and herChars are not defined when that line is read (or anywhere else in the file). Please post your entire, actual code when asking for help with code. Posting partial or fake code is just a waste of time -- both for the person who reads your code and makes suggestions that may not even be relevant to your real code, and for you to read the suggestions and figure out whether they are relevant.

If your real code contains private information (such as your character/server names) you can change them to dummy values, but don't alter the basic structure or the types of variables (eg. if myChars is a table in your real code, it should be a table in the code you post).

Originally Posted by Caellian I would like a way to feed whoever needs to be promoted or kicked into a table and then do what needs to be done on each of the elements of that table 1 by 1 on a timer (1 or 2 sec between each action) and then obviously remove what's done from the table.

I'd suggest using an animation group instead of your GuildManagement function. Replace the whole function with this:

http://pastebin.com/qiVTsemb

Then, anywhere you would call GuildManagement(), call timer:Play() instead.

Since you cannot pass extra variables (eg. enabled) this way, it uses a flag set on the timer object instead. By default the flag is set, so no actions will actually be performed (you'll just see prints describing the actions that would otherwise happen). To make it actually promote/remove people, set timer.testMode = false before timer:Play().

Other notes:

(1) Rather than mentally switching between 0-based and 1-based rank indices, just add 1 to the rank from GetGuildRosterInfo so it matches the 1-based rank you'll be passing to SetGuildMemberRank and you don't have to think about it.

(2) I cleaned up some of the redundant if (X and Y and Z) or (X and Y and Q) or (X and Y and A) then logic. If you really like checking X every time, I guess you can change it back, but remember that rankIndex needs to be adjsuted by +1 in anything copied from your old code.

(3) There is some basic protection against infinite looping if an action is failing. Rather than attempting the same action over and over, it should print a message telling you about the failed action, and then stop running.

(4) Finally, I did not test anything in-game, so there may be typos. If there are, and you can't figure them out on your own, please post your new complete code and the error message from BugSack.

[Caellian]

Wow, animations, this is such a neat trick, thanks Phanx.

But i see two things happening with the testing mode:

1. PROMOTE playerX nil

so, eventhough they're in the right rank (rank change being nil), it still goes through them, is it intended ?

2. It still prints loads of them at the same time, meaning that, if i disable the testing mode, it will still promote/kick a gazillion of players at the same time, correct ?

[Phanx]

http://pastebin.com/KniSdGbT

(1) Oops, I forgot to add a check to make sure a target rank was actually set before trying to do the promotion.

(2) No, I just forgot to stop the loop when faking an action in test mode (vs. actually performing the action).

Both issues should be fixed in the above paste.

[Caellian]

So, this is what it looks like now:

http://pastebin.com/CMhPYmJK

The kicking part was just fine, or so i think.

The promoting part seems to have a problem it didn't have before. Right after a reloadUI it does the promoting but if someone joins the guild, it doesn't get promoted right away, i have to reloadUI to make it happen.

[Phanx]

The way you're handling GUILD_ROSTER_UPDATE seems pretty weird in general. Rather than imposing an arbitrary 5-second delay, I'd suggest re-registering it only after all actions have completed. Change:

	-- Start it again if we did something.
	if didAction then
		self:Play()
	end

To:

	if didAction then
		-- Start it again if we did something.
		self:Play()
	else
		-- All done, start listening again:
		caelCore.management:RegisterEvent("GUILD_ROSTER_UPDATE")
	end

Then get rid of all the PLAYER_ENTERING_WORLD event stuff, and change your GUILD_ROSTER_UPDATE code to:

		self:UnregisterEvent("GUILD_ROSTER_UPDATE")

		-- No need to do this function call and comparison twice:
		local inTheGuild = GetGuildInfo("player") == "We Did It"

		if inTheGuild then
			SetGuildRosterShowOffline(true)
			GuildRosterShowOfflineButton:Disable()
		end

		if GuildFrame and not GuildFrame:IsVisible() then
			SortGuildRoster("online")
			CalendarManagement()
			if inTheGuild and (CanGuildPromote() or CanGuildRemove()) then
				-- The timer will reregister the event when it's done:
				return timer:Play()
			end
		end

		-- If we got this far we didn't run the timer, so no more changes will be made:
		self:RegisterEvent("GUILD_ROSTER_UPDATE")

Also:

(1) Instead of "if event == A then X end, if event == B then Y end" you should use "if event == A then X elseif event == B then Y end", so you're not processing a bunch of if checks that you already know won't pass.

(2) Don't use LoadAddOn directly on Blizzard addons. The calendar, in particular, can break if you load it too early. Instead, just listen for those addons' ADDON_LOADED events and delay running any code that depends on them until it fires. If you absolutely need to run that code without waiting for the user to open the calendar, use Blizzard's special methods, such as Calendar_LoadUI() and GuildFrame_LoadUI(), to ensure that any dependencies get loaded in the right order.

(3) Rather than using SPELLS_CHANGED as an initialization event, you should really use an event that's actually intended for initialization purposes, such as your addon's own ADDON_LOADED event, or PLAYER_LOGIN.

(4) Why on earth are you using a while loop with a manually-incremented index, instead of for i = 1, GetNumGuildMembers() ?