[patch] Group Loot rules for chests

[Auntie Mangos]

* What bug does the patch fix? What features does the patch add?

Implements Group Loot rules for chests

* For which repository revision was the patch created?

10227

* Is there a thread in the bug report section or at lighthouse? If yes, please add a link to the thread.

http://getmangos.eu/community/viewtopic.php?id=13160

* Who has been writing this patch? Please include either forum user names or email addresses.

darkstalker, based on patch by Wowka321

http://paste2.org/p/920493

Edit by Schmoo: In [11878]

[Xfurry]

Patch works fine.

We are waiting for it to be accepted into the main core.

[Br3Ak3R]

Bump

Any news for implementing in main core?

[The_Game_Master]

A reply with, not good enough, needs more work should do as well.

[Unkle Nuke]

I can accept that, but, for the sake of clarity and speeding up development, why is it "...not good enough,needs more work..."?

Learning minds want to know!

[Xfurry]

I can accept that, but, for the sake of clarity and speeding up development, why is it "...not good enough,needs more work..."?

Learning minds want to know!

As I said before, this patch works fine.

The only problem, (which actually isn't a problem), may be that loot rules are applied to both corpses and boxes. That's why some segments of this patch must be written just once for both corpses and boxes somewhere in Object.cpp to avoid redundancy.

[qsa]

As I said before, this patch works fine.

The only problem, (which actually isn't a problem), may be that loot rules are applied to both corpses and boxes. That's why some segments of this patch must be written just once for both corpses and boxes somewhere in Object.cpp to avoid redundancy.

I was thinking implementing it using multiple inheritance instead of stacking needless data into Object class. Since Object, worldObject, Unit do not hold any loot, but their derived classes do.

Making class Lootable and GroupLootable to inherit from it. Those two can abstract all or loot cases : Item, Corpse, Creature, GO without any redundancy.

However, multiple inheritance comes with a price. Its not like have "diamond shape inheritance" here, or can ever have, but still.

I'd love to hear what people here have to say about this idea, before I start working on it.

[darkstalker]

type casting of classes with mutiple inheritance causes this pointer adjustment, that might break code that uses C style casts

[qsa]

Well, after thinking about the idea little bit more, I don't think anything can be broken here, atleast not how we currently use it.

So, I made a patch giving a different solution to above.

There are two new classes Lootable and GroupLootable ( 2nd derived from first ).

Those classes abstract all types of lootable objects we currently have.

Corpse, Item by Lootable and Creature, GO by GroupLootable.

This way there's no code duplication in a price of multiple inheritance from those classes.

Code is mostly copy-paste from Creature case just rearrange differently.

Patch against rev.10403 http://pastie.org/1129545 ( updated )

(don't forget to add 2 new files to your project files )

more or less tested.

TODO: add proper documentation and properly test.

Suggestions are always welcome.

[The_Game_Master]

Included project files in it http://pastebin.com/R1gBHrJe

Works good on the first sight, will test more and reply back.

gcc generated some errors

./src/game/Camera.cpp
In file included from ../../../src/game/GridNotifiers.h:26,
                from ../../../src/game/GridNotifiersImpl.h:22,
                from ../../../src/game/Camera.cpp:2:
../../../src/game/Corpse.h:50: error: expected class-name before '{' token
In file included from ../../../src/game/GridNotifiers.h:29,
                from ../../../src/game/GridNotifiersImpl.h:22,
                from ../../../src/game/Camera.cpp:2:
../../../src/game/GameObject.h:580: error: expected class-name before '{' token

[qsa]

Thanks.

Sorry I haven't tested under gcc.

Just some missing forward decelerations.

updated patch http://pastie.org/1129545 also old link updated.

[darkstalker]

whats wrong with my patch version? i tested it for months and works fine

[Schmoozerd]

nothing wrong, but it is better (redundancy!) to move similar code into one place, and this is for looting-objects either WorldObject or like qsa did it: a new class which is only used by lootable objects

Now only a dev would need to say something how he thinks this should be handled best

[Darky88]

./src/game/Camera.cpp
In file included from ../../../src/game/GridNotifiers.h:26,
                from ../../../src/game/GridNotifiersImpl.h:22,
                from ../../../src/game/Camera.cpp:2:
../../../src/game/Corpse.h:50: error: expected class-name before '{' token
In file included from ../../../src/game/GridNotifiers.h:29,
                from ../../../src/game/GridNotifiersImpl.h:22,
                from ../../../src/game/Camera.cpp:2:
../../../src/game/GameObject.h:580: error: expected class-name before '{' token

Error persist with updated code.

[qsa]

Error persist with updated code.

Can anyone please confirm?

Tested again, just to be sure on clean rev. 10400 under gcc 444, fedora core 12 -- patch applies and compiles like a charm.

PS: please make sure the patch is applied properly, especially the new files and you reconfigured before "making".

[Xfurry]

Any news about this? It's still an important patch and it works fine!!!

[Xfurry]

Sorry for bump-ing again, but doesn't really any developer care about this patch?

Both patch versions work fine and are blizzlike. There may be some adjustments to be done, but still a developer should review this and at least make some notes about it.

Why is this taking so long to review?

[Schmoozerd]

because devs are lazy (or don't get enough free beer)

(jk)

[Xfurry]

I'm starting to believe that you don't like this patch at all...

[Darkknight900]

Is there a patchfile for this patch for latest rev. or something?

[qsa]

Is there a patchfile for this patch for latest rev. or something?

Patch in 11th post should be fine, you may have to patch some parts manually tho.

[darkstalker]

this probably needs a rewrite..

[Marik]

That would be very nice.

I've tried the patch on the last revision, but there are way to many conflicts

Marik

[Schmoozerd]

qsa's patch will need a major extention, as item-loot now is stored in a table

darkstalkers patch should be not too hard to adapt to recent version.

Personally I still like qsa's idea to write one(or three) classes for loot, but afaik a multi-heritage OOP design throws a few problems (C-casts everywhere) - and I didn't understand why he didn't implement these classes as member variables of the creatures, corpses, or whatever

[darkstalker]

or instead use another class for storing loot information, and make it member variable of Creature and Gameobject, then you use GetLootMgr() or something like that for calling that functions