evil-morfar/RCLootCouncil2

v2.7

Closed this issue · 135 comments

Just a thread to discuss some changes for 2.7

I changed the layout/text for the descriptions in cdb4600, but I'm not sure if it could be better still. Any inputs @SafeteeWoW ?

ilvl needs to be center aligned in session frame. Does not look good otherwise

Except it looks awful when tokens with a + in their ilvl is also there

Then prefix by some spaces

You could argue a left align needs a space prefix. A prefixed center align doesn't work. What happens when ilvls changes? Next raid a ilvl 1000 item drops.

To my knowledge, it's very unlikely that the ilvl 1000 legendary trinket drops by master loot because this creates problem in the guild. People suspects that it drops in personal with low chance, or obtained by a long quest chain. Even if it does drop in master loot, I don't need to beautify the frame for an single item. Therefore, all ilvl should have 3 digits. Even in the next expansion, because there is a ilvl squash, ilvl remains below 1000. It's likely ilvl stays with 3 digits in the near future.

Center align still doesn't work well.

Eitherway, it's either both ML and PL or a questline. Doesn't really matter, I was only trying to put a 4 digit ilvl in perspective (i.e. 970+).

The other solution will be dropping "+". It looks better in session fame and loot frame if all ilvls are 3 digits.

Side note: I spell the word trade-able as "tradable" (American English), while British English tends to spell as "tradeable". Shouldn't matter though

You should offer version check in GroupGear and EU module.

I'll give you until tomorrow then.

While I don't really care, I think tradeable is more universally approved.

Removing the + is not a good solution. Why is it so bad to left align it? I don't think it looks that bad, and a space before it will make it look just fine.

EU and GG doesn't really need a version check at this point. I've also decided you're right - doing a module version check in the core addon is probably preferable.

Reannounce and roll improvement #92 is ready. Can you have a look?

Yes, left align it and prefixes by some spaces. I had some misunderstandings.

If you are going to release it soon, can you upload the localization to Curse? I gonna work on the Chinese translation.

A space, as in one. More than that and it might become too big, which is the reason spaces doesn't work with center align

I uploaded the locale yesterday with the current merges. I won't do the release today as I only had time this morning and won't be home for the rest of the day

When are you going to release it? We'd better release it before US gets Antorus raid

My isp decided to fuck up, so I've been without net all day. I have a raid in 20 mins, but hopefully I'll get it done today - definitely before Tuesday.

I have finished the code of RCLootCouncil-EPGP and I am doing the complete testing right now.

There is a bug that the text of moreInfo of voting frame is out of frame after resizing.

Side note: In RC v3.0, you should fix the name of Russian characters. It's very easy to make mistake in RC-EPGP when some times I need to remember to convert RC name into EPGP name, but sometime I shouldn't. I probably should make a character with special name to test this out.

I'm still not sure if #88 should be in 2.7. #92 seems good enough to go.

The namefixing doesn't necessarily have to go into v3.0, it just breaks backward compability to fix it, and requires a council reset. As far as I'm aware the issue lies in my forcing of titlecases - it only capitalizes ascii characters. Afaik epgp uses the name returned by UnitName and equivilant.

My plan for v3.0 is to use GUID's to handle players, with name only being used on displays.

Anyway, I'll push 2.7 tonight, I just need to do some final tests.

RC-EPGP should be ready. Just the changelog and README and translation will take me some more time.

I have no issues with the more info frame when resizing.

There's another kinda huge issue though. Reawarding an item doesn't change the response of the previous winner. I tried something quick (from line 180 in votingframe.lua):

local oldWinner
for k, v in ipairs(lootTable) do
	if addon:ItemIsItem(v.link, lootTable[s].link) then
		if v.awarded or oldWinner then -- This is a reaward - restore old response.
			oldWinner = v.awarded or oldWinner
			self:SetCandidateData(k, oldWinner, "response", self:GetCandidateData(k, oldWinner, "real_response"))
		end
		self:SetCandidateData(k, winner, "real_response", self:GetCandidateData(k, winner, "response"))
		self:SetCandidateData(k, winner, "response", "AWARDED")
	end
end
lootTable[s].awarded = winner

However this will not work if the items are not awarded in order, and it will remove awarded response from 1 winner when two equal items are awarded to two different persons. Can you think of anything other than checking all candidates for "real_response"?

I would also like to change the name "Group" from the reannounce and request roll dropdown to "Everyone" - thoughts?

I would also like to change the name "Group" from the reannounce and request roll dropdown to "Everyone" - thoughts?

Sure

Let's use another approach.
Instead of change the response, let's modify SetCellResponse/ResponseSort to display the the response as "awarded". We don't modify the response. Technically, awarded is not a response.

Doing it in SetCellResponse would only allow us to show 1 candidate as the winner, unless doing serveral loops. Anyway, found a way to do it effortlessly.

Several loops, per candidate, per update - that's too much.

Line 187 should be swapped with line 186.

						end
						self:SetCandidateData(k, winner, "real_response", self:GetCandidateData(k, winner, "response"))

But this has problem when the item is awarded multiple times to the same person

Hmm. No and no.

-- Check modules. Parse the strings.
for _, str in pairs(moduleData) do

Need to check if moduleData is nil before this line, got a nil error

Luckily Curse rejected the tag, otherwise it would have been pushed by now. Any other last minute finds?

I am not testing it, because I'm writing the documentation. Just randomly found this bug.

It just went through. The total tally is 5.932 changes (!). Let's try not to do such a massive update next time. Either way, thanks for your work so far.