Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Need Before Greed loot method, roll flags in general + some fixes in loot roll system #112

Closed
wants to merge 4 commits into from

Conversation

MesoX
Copy link
Contributor

@MesoX MesoX commented Feb 7, 2012

Hey!

I've somehow found that ArcEmu supports only 3 base loot methods (master looter, ffa, group loot), so I added support for Need Before Greed too.
This means I've implemented roll flags, which were previously sent into client only for NEED and GREED. This also includes disenchant roll flag, which can be selected, when anybody in group has at least same enchanting skill as item requires.
Need Before Greed - this works almost same as group loot, with addition that players, who cant use the item, can't select Need for it, like when there is plate, mage can't select for it, but it works vice-versa also for warriors, even they can use cloth. Works also with skill requirement, class and race requirement and skills for weapons are needed too.

There are few other things fixed too with this patch:
Previously, system printed roll into chat immediately after player chosen the option, but now, it prints only which option he chosen and after all players in group choose some option (or when timer runs out), it prints actual rolls for everyone and assign, who won.

I've tried to test this as much as possible and haven' found any bugs on it, but my tests are limited only to 2 players, so if anybody would mind testing this better, it would be nice.

There is 1 issue, I wasn't able to track down. When there is any kind of loot method, and somebody loots gray or white quality item, it should print, that he looted some item for all players, and even there is SendItemPushResult, it doesn't. Maybe I was just too blind or tired, when looking on it. Hope somebody will make help me making it working.

* Added support for disenchanting roll flag
…condition

* Fixed adding disenchant loot into stack, if it's possible
* roll flags can now be different for every player, that means that some player can select both need and greed, while second player can select only Greed, this is known as Need Before Greed loot method
* Actually send who selected which option first and when all selects something (or time pass out) it shows the rolls
* When time pass for everyone in group (yes, even this can happen), send to group that everyone passed and make loot lootable for everyone, currently on ArcEmu it started roll again when somebody looted it
* Fixed Shields for Need Before Greed too
@jackpoz
Copy link
Member

jackpoz commented Feb 7, 2012

does Need priority over Greed work in Group Loot? (can't check atm on wowwiki how that's supposed to work, just asking to be sure)

@MesoX
Copy link
Contributor Author

MesoX commented Feb 7, 2012

Yes, Need has higher priority and works. Group Loot is simple, it only pop ups the loot roll window for each item, with need and greed (and disenchant if possible), while Need Before GReed allows only greed and disenchant, if you cant use that item, otherwise there is no difference.

@ghost
Copy link

ghost commented Aug 6, 2012

@MesoX I tested your patch and everything was alright. BUT: My test was also limited to two players (actually it was just me playing one character and let the second one follow him). Btw the server crashed during the test... Maybe you should contact Magnifikator, he has a project with an additional development server.

Edit:
Ok, maybe your patch caused the crash... -> #231

@dfighter1985
Copy link
Member

@MesoX #231

@dfighter1985
Copy link
Member

@MesoX please make sure that your code

  • works
  • doesn't crash

before submitting it.
You can do this simply, all you have to do is:

  • read carefully and THINK before modifying stuff
  • review your changes before commiting
  • TEST your changes before commiting

When these are done, you are welcome to resubmit.
Thanks!

@MesoX
Copy link
Contributor Author

MesoX commented Aug 7, 2012

I haven't got any crashes half of year ago, maybe it become old?

Btw dfighter, i am not interested in your commets. If you are not willing to help, don't comment it. :)

@dfighter1985
Copy link
Member

@MesoX I pointed out the reason why it crashes in the linked issue.
How is that not helping?
Besides it has nothing to do with age, I checked your commit without merging. Without even downloading it and I could find the reason for the crash.
However sure go ahead and feel offended and say you are "not interested".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants