"/rc add" can now be run by everyone

[SafeteeWoW]

Changelog

• Add '/rc startLoot' and '/rc stopLoot' to start/stop auto loot management.

  • '/rc startLoot' starts the loot management as long as the player is not group admin and not in LFG. This command is not affected by usage options such as only use in raid.
  • These are not good name for commands though
  • The NewMLCheck is rewrited. The logic of this function is very important and the return statement in the middle of function makes the code chaotic.

• Support to auto loot handling when the loot method is not master.

  • There is an option to auto start loot handling even if the loot method is not master (disable by default) and also you can always start it by '/rc startLoot'.
  • If the the loot handling is enabled in non-master mode, RC parses chat loot msg and adds the item if it is CanWeLootItems into the session frame, with the owner's name.
  • It's possible to add items manually specifying the owner's name by "/rc add NAME link1 link2 ..."
  • It's very likely that some item in this session frame needs to be removed. so I made some adjustments to the session frame.
    • ilvl is an important number to know. A 915 random heroic non-set piece is probably not worth to discuss in peronalloot. One extra column of ilvl is useful without costing consuming much space.
    • The owner of the item is also very important to know in the session frame.
  • Voting frame changes:
  • Owner has an loot icon next to his name for indication
  • Owner is always sorted on the top by default

• I made some changes to locale, especially the text of usage options. But definitely more needs to be added or changed.

• Add award keyword "&o": The item owner

• RC applies usage option after ML option frame hides, if usage option changes.

[evil-morfar]

So, if I understand this correctly, if the group leader enters "/rc startloot" it monitors any loot given out by the game (PL, grouploot, etc.) and adds those to the session frame. And it's only the groupleader that can do that?

Why would you need to sort the owner on top? That doesn't really matter imo.

Honestly this should probably be a module of its own, I don't think I'd like this in the core RCLootCouncil.

[SafeteeWoW]

Why should it be a another module? A lot of changes actually not belong to this PR. If I drop the sorting owner on the top and all locale changes, there are only 50 lines of changes. imo, sorting the owner on the top makes perfect sense though, the information and response of the item owner should always be considered. I can simplify the code of sorting. I don't think there is more positive than negative effects.

[evil-morfar]

It's not about the amount of changes, it's a question of providing the help to those who need it, and not litter everyone else with (for them) useless features. It's practically the same as asking why EPGP isn't a part of RCLootCouncil - the basics could easily be integrated with only a few lines of code, but it isn't, as people specificly opt in to using EPGP.

The only thing going against making it a seperate module is that everyone would require to download it to have the owner of the item displayed.

While the owner's response is of course important, the response still takes priority. Imo the owner should only be on top if his response is of the highest normal sorting in the session.

[SafeteeWoW]

EPGP is not a few lines of code, tbh though.

---

Do only few people want to do this? There is really no addon in WoW right now that can help loot distribution when the loot method is not master loot, but in fact there are tons of guild that are not using master loot. If things exist, people will use it.

---

Fixing merge conflict

[evil-morfar]

The basics pretty much are.

There's probably a reason for that. While ML was restricted with Legion, PL have been a thing for a while, so there's been plenty of time to develop an addon for it. I'm all up for facilitating PL, but I don't like it being part of the core RCLootCouncil.

I'd also expect more people to complain about PL if it's such a requested feature.

[SafeteeWoW]

The feature of this PR will be changed. Instead of group leader sees everyone's loot in the session frame, raid member can ~link the item to the raid chat~ use a command, in order to add the item to the session frame of the ML.

TLDR, This PR will alter "/rc add" so non-ML can also use it and send the command to ML, depend on the setting of ML. How is this? I will implement tomorrow if looks good for you

[SafeteeWoW]

I use a different approach. Now "/rc add" can be run by non-ML and send the item to the ML's session frame. To avoid non-ML to abuse it and to avoid error, there are several restrictions to run this command by non-ML

1. ML has enabled this feature.
2. ML is not in combat.
3. The session is not running.
4. The item is in the non-ML's bag.
5. The item quality is not lower than the loot threshold.
6. The item is tradable.

I think now this feature is purely enchancement and there is no need to put this feature into a separate module because there is benefit to do this even in master loot method, like someone is willing to give out his boe.

---

• The feature to sort the owner on top has been removed.
• Session frame looks the same as above. I think it's awesome to see ilvl in the session frame, even if master loot method.
• I'm going to add another chat command '/rc qadd' to be a shortcut of /rc add, so you can omit the item link (Add the last tradable item you get since the last encounter ends)

[SafeteeWoW]

• Add the shortcut command "/rc qadd" in order to avoid linking the item. It adds the last tradable not below the loot threshold the bag got since the last encounter ends.

[SafeteeWoW]

• This PR is finished now. This PR also includes some bugfixes to the logic of get new ML

[evil-morfar]

Besides this being imo being a too major change to include in 2.7, there's still some issues with it, thus I'll delay this for 2.8.

• The commands: list, startLoot, stopLoot should probably have an explaination. One concern is the list of commands has grown big, but I still think they are vital to include for this feature. I can make them if need be.
• As ML, /rc add [item] should set the ML as the owner. It seems like you purposely didn't do that?

  There is an option to auto start loot handling even if the loot method is not master (disable by default)

Where is that?

[evil-morfar]

I'm going to add the ilvl portion to 2.7. If 45px really required? 40 seems better imo.

[evil-morfar]

I discovered a minor, but breaking bug with "Autoloot all BoE" - e9bfcf1aebb90517472bffa679a0c044311e20fd. While inspecting an issue report on Curse, I realized that this setting also sends any tradeable items to the ML when the group is using personal loot. This really needs this PR to be implemented to be really useful, so I decided to let remain broken until v2.8, which hopefully isn't too far away.

The feature was released almost two months ago, and since no one has complained, it doesn't hurt to wait a few more weeks.

[SafeteeWoW]

I indeed want this to be implement it, as I already see some guild farming in personal loot with the help of RC. I prefer to release this next version though.

Things I want to change:

1. If the loot method is personal, when ask usage for leader, use a three button dialog instead of two: (1)Use RC and switch to master loot, (2)Use RC in personal loot. When selected, print a help message such as "Under personal loot, group member with RCLootCouncil v2.7+ installed will automatically add tradable items they looted to the session frame of the group leader." (3)Dont use RC. A checkbox in RC option to control whether to use RC in non-master makes old user hard to realize the new feature and it is a bit confusing.
2. Add option "Always use RC in master loot and never in personal", and another option "Always use RC in master and personal" In both case, no popup in any loot method.
3. Forget how I write this PR, but loot handling shouldn't be enabled under Group Loot or free for all.
4. The item owner to get a special notice from RC when the his item was won by some1.

In conclusion, the most important concerns is that how the loot handling should be enabled.

[SafeteeWoW]

I just dont see the reason why you don't fix the autoloot boe bugs in 2.7.6, when it is a simple bug.

[SafeteeWoW]

Other ideas (Probably not going to implement them in this PR):

1. More convenient trade feature1: When the group leader assigns a gear in personal loot in a RC session, it is the item owner who initiates the trade, not the group leader, I would like to auto add the item to the trade window for the item owner.
2. More convenient trade feature2: To enhance the above feature, show a popup to open trade window when the item winner in the trade range
3. Ultimately, can we implement RCTradeFrame to handle the who we should trade, what is traded and what is not, etc?

[SafeteeWoW]

I hope in the future, with the help of RCLootCouncil, more guilds will use personal loot as the loot method.

Pros of personal loot:

1. Loot drops in personal loot is always equal or more than the loot drops in master loot (I am very sure this is true. Blizzard does this to encourage personal loot)

Cons of personal loot:

1. Cant trade if it is an upgrade.
2. You have to trade, you cant put the item directly into some1else's bag. This is not convenient (Hope RC can help this)
3. If we don't want to waste a gear in the large group, many people are involved in the loot distribution, trade between each other and need lots of communication. This is hard. (Hope RC can help this)
4. Set piece issues. Set piece in personal loot is class restricted, highly increases the chance that the gear is wasted. An issue in this expansion, but not at all in the next expansion because of no more set pieces.

[evil-morfar]

I'll release v2.7.6 today, and this PR isn't ready for that. It'll go in 2.8 as planned, which will be in 2-3 weeks. Your suggested changes is part of the reason why I have delayed this - it has always felt "incomplete" to me. I think your suggestions will help alleviate this, and as you said, loot handling is the primary concern:

Add option "Always use RC in master loot and never in personal", and another option "Always use RC in master and personal" In both case, no popup in any loot method.

This needs to be done carefully - potentially auto enabling loot handling with PL could have many unintended side effects.

Auto Loot BoE Here's the reasons for not fixing it now:

1. It has never worked, so noone really misses it.
2. It doesn't do what it says it does. It says on the label (and that's what I thought the intent was) that it'll do a "/rc add" with any BoE items looted by anyone - but it takes any tradable items, including enchanting materials (at least until I added an ignore check). In fact, it actually does most of what this PR is for, but automatically (provided addon.handleLoot is enabled).
3. Your second pop-up button suggestion would, as it stands, simply do db.autoLootBoE = true and addon.handleLoot = true. While the tradable command could probably be used for both BoE items in ML and all items in PL, the "Auto Loot BoE" option needs changing.
4. Finally, this PR is so closely related that it makes sense to band them together. Seeing who "owns" the item will also help a lot.

Your other ideas They need to be handled separately, but could definitely go into 2.8.

1. Agreed. The whole trade deal could just be moved out of ml_core so it can handle more than the ML's trade - maybe even in a separate file.
2. While very convenient, I'm not sure if this is smart - I image having to scan constantly could be ressource demanding. That would need testing.
3. That would make sense. The functionality mostly already exists for this, so it's basically just a question of putting that info into a frame.

Personal Loot - future Blizzard has confirmed that PL will, on average, give more loot than ML to compensate for the randomness. I think they said 20% more (can't find the source).

I really don't hope people will use PL. It complicates things on the developing side, and your pros/cons really shows why it's inferior. That being said, I do want to make RCLootCouncil the best tool it can be for those that decides to use PL - as long as it doesn't negatively affect those using ML.

[SafeteeWoW]

It doesn't do what it says it does. It says on the label (and that's what I thought the intent was) that it'll do a "/rc add" with any BoE items looted by anyone - but it takes any tradable items, including enchanting materials (at least until I added an ignore check). In fact, it actually does most of what this PR is for, but automatically (provided addon.handleLoot is enabled).

Several ways to fix this:

1. Currently, it is adding non bop items. We can change it to add BoE items only.
2. I talked about abandoning the current ignore feature and we should just filter item by its type rather than add item id manually into the ignore table. The item lootable by RC should satisfy one of the following.
3. The item is equipable ("EquipLoc" field of GetItemInfo is not empty string)
4. The item is a mount
5. The item is a set token (I talked about how to determine this in #141. The item type is Miscellaneous and subType is Junk. The item quality is rare. The item maxStack is 1. The item is class restricted. I don't see any exception any set token doesn't satisfy the condition above, and I don't see any item satisfy the condition above which is not a set token.)

We can also use blacklist instead of whitelist item type, if you concerns there could be some exceptions.

1. Always ignore quest item
2. Always ignore crafting reagents
3. Always ignore crafted items (Item that is made by some1). etc.

[evil-morfar]

While we could easily alter it to only accept BoE items, that would not make the tradeable comm useable with PL (at least not in its current form - not that it necessarily needs to).

To clarify what I think it should be:

Auto Loot all BoE As long as RCLootCouncil handles loot (ML or PL) if this is enabled, all BoE looted by anyone should be added automatically.

Two ways of doing this:

1. Include settings in the mldb, and make sure people only send tradable for things we want. This will reduce comm messages.
2. Filter it when received, as now. Loads more comms, but saves space in the mldb and is probably more flexible.

I'd probably prefer the first.

Personal Loot As you mentioned, a separate button (or automatically) should enable handling of loot in PL mode. With this, any tradable BoP items should be automatically added. BoE should still only be added with the setting above.

This could probably use the same comm command, but otherwise we'll just create a new.

I'm not entirely sure what rules tradeable BoP items follows when it comes to PL, but they're definitely not listed as 1 or LE_ITEM_BIND_ON_ACQUIRE(edit: unsure). However, I can confirm that BoA (on account) are treated as LE_ITEM_BIND_ON_ACQUIRE along with any other soulbound items.

Doing this would also remove the need for raiders to do a "/rc add", which I've never liked anyway.

---

A blacklist is probably worth considering.

[SafeteeWoW]

I'm not entirely sure what rules tradeable BoP items follows when it comes to PL, but they're definitely not listed as 1 or LE_ITEM_BIND_ON_ACQUIRE. However, I can confirm that BoA (on account) are treated as LE_ITEM_BIND_ON_ACQUIRE along with any other soulbound items.

I know this. Therefore, the tradable command is only sent when the item looted is trabable, checked by tooltip parsing

BIND_TRADE_TIME_REMAINING 
-- "You may trade this item with players that were also eligible to loot this item for the next %s."

[evil-morfar]

That's not really what I ment. I'm aware of the tooltip parsing (which also causes non bound items to get sent, hence the need to check ignored items). I was referring to the check in ML:OnCommReceived() where any tradable items with LE_ITEM_BIND_ON_ACQUIRE gets discarded.

Also, I made a mistake in the previous comment; I'm not sure of the bindType returned by GetItemInfo() for tradable BoP items.

[evil-morfar]

Most of these addition has been included in v2.8. I still don't think it's needed for everyone to be able to do "/rc add", and with PL this is done automatically anyway. As for the /"rc startloot" this is already done with "/rc add". Closing.