qsa

Thanks.

Sorry I haven't tested under gcc.

Just some missing forward decelerations.

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

Maybe you can find some good ideas here: http://code.google.com/p/trinitycore/source/detail?r=8d26854d19f3247c1216019b14a0ee6ba9d0a591

Thanks for linking.

Unfortunately it is unrelated, nothing to do with my patch, beside changing the field name from unknown.

As the title states. Nothing critical, just few compile warnings less, really minor change.

Against rev 10408 : http://paste2.org/p/969615

Thanks in advance.

Hello,

Fields 229-231 in spell.dbc hold spell bonus data since 3.2.0

This patch loads this data instead/in addition to the data in spell_bonus_data table.

Entries in spell_bonus_data table will override the default DBC data.

NOTES:

1) The patch uses existing mSpellBonusMap to load the DBC data into instead of more logical solution of directly using the data from sSpellStore.

Why? much simpler and centralized code. There are not that many spells anyway.

2) DBC provide us data per-effect, while mangos spell system assumes bonuses are per-spell. I guess we'll have to change it someday. Not directly related to the patch.

It works since so far there are no spells with two or more dots/direct effects.

3) Those fields hold only spell bonus data, no AP bonuses. Therefore we still need spell_bonus_data table for some spells and overrides.

4) At the moment mangos loads only first rank of the spell and assigns same bonus to all ranks, this is wrong. Different ranks may have different bonuses. eg: healing touch ( most 10+rank spells actually).

Data loaded from DBC will properly load the bonuses for different ranks, while data loaded from spell_bonus_data will work as it does now.

5) For now, NPC spells are NOT loaded. Do we need this data?

Can be enabled by commenting line 44 in patch.

6) sql cleanup added for redudndant data in spell_bonus_data table - most of entries in there. Simply since the data provided by DBC is accurate and mostly complete.

TODO: properly test, we may have problems with some hardcoded bonuses for spells.

diff against revision 10408

patch : http://paste2.org/p/969586

sql cleanup : http://paste2.org/p/969587

Testing, remarks and suggestions are always welcome/needed

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.

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.

Actually they should stack - their effect shouldn't.

this aura state can be look at client in some way? For some spell icon...

yes : you can check 51662 - gets activated while active. Just to confirm it is the proper value.

i've trying to apply patch on clean mangos... http://paste2.org/p/565237

but it fails on compilation

2>..\\..\\src\\game\\SpellEffects.cpp(4782) : error C2065: 'AURA_STATE_BLEEDING' : undeclared identifier

2>..\\..\\src\\game\\Unit.cpp(9550) : error C2065: 'AURA_STATE_BLEEDING' : undeclared identifier

oh.. you are totally right, it doesn't compile on clean mangos. I forgot that aura state is not yet implemented on clean.

Will update momentarily. Thanks.

EDIT:

http://paste2.org/p/565788

Updated, aurastate added, patch updated in first post too.

Updating to fit 8967 : still valid.

http://paste2.org/p/564838

revision: 8967

Implementing Rend and Tear both dummies. AURA_STATE_BLEEDING(18) added.

UPDATED

http://paste2.org/p/565788

revision : 8967

Allow SPELL_AURA_PERIODIC_LEECH to crit if they should be able to.

For example: Devouring Plague.

http://github.com/gc/mangos/commit/be729576f7efc42605b7ba56b29357eea61afd98

multiplier correct, read release notes for 3.2.x ...

full damage / 5 tick * 8 = expected damage _if_ no any spell bonuses.

Single problme possible just absent sql record in spell_bonus table witjh 0% coef for any spell damage bonus for dispel damage spell

If someone from _check_ and suggest related patch (in under review section) i will review.

http://github.com/gc/mangos/commit/648edfd80984d57b72406b4d36d110c1170498b0

Something we use for a while now. The best solution I was able to find. Just fill the bonus data of triggered spell to negative value. Problem with spells like VT dispel is that they still gain double bonus from % modifiers etc, and on priest it is very apparent, since it is about 40% + extra.

Please update your patch when you have the time and use git or pastebin.ca

Thank you! Works incredibly well so far.

We're still running 313 code, so updating this patch will take alittle bit, since I'm not able to test it on new. I don't think anything changes, still.

Feel free posting diff for latest mangos revision.

Thanks for testing.

...

Btw, Feral Spirit summon only one wolf, not two. It is related patch or need some improvements in core? :confused:...

multiple summons not yet supported as controllable pets. The best I can do ( besides messing with pet system) is making one of them pet and the other one guardian but thats just ugly workaround ( even more uhly than I have at the moment - which is hard:) ) .

Well, our test server runs this from its very start, well over 2~ month now and I'm yet to have problems.

I'm not sure if this is the usage intended for those DBC files by bliz but it fits well.

Almost all summons fit well into those groups beside few spells ( added exceptions ), which make me think they are just handled not properly in their generic type summon handlers.

Don't know if anyone else tested it besides us.

Kung FU Bump:)

btw this code part is correct for impleting:confused:

   float maxval = pVictim->GetMaxPositiveAuraModifier(SPELL_AURA_MOD_HEALING_PCT);
+    // can SPELL_AURA_MOD_PERIODIC_HEAL be positive?
    if(maxval)
        TakenTotalMod *= (100.0f + maxval) / 100.0f

It is just comment. You can add it if you like - wont change anything.

I wasn't able to figure out from current spells which use this aura if it can be positive. For now (313) it is only used as negative.

In [8648]. Thank you.

As i understand rank req. for new continnent outdoor battle must be checked in BG code.

While it is true this aura used only in BG related spells for now, I don't think it is has anything to do with this specific aura. We don't have this BG implemented by default ( yet ), so dealing with casting those spells used ( when opposite faction have numeric advantage), is irrelevant.

For now, on default mangos, it is to "fill the blanks" in aura implementations.

Thanks for reviewing.

There is problem introduced in http://github.com/mangos/mangos/commit/e5046cd14d44c4bdc12226a30e0ab0b6764ef0c8

@@ -1361,9 +1360,8 @@ void Spell::SetTargetMap(uint32 effIndex,uint32 targetMode,UnitList& TagUnitMap)
{
radius *= sqrt(rand_norm()); // Get a random point in circle. Use sqrt(rand) to correct distribution when converting polar to Cartesian coordinates.
            float angle = 2.0 * M_PI * rand_norm();
-            float dest_x = m_targets.m_destX + cos(angle) * radius;
-            float dest_y = m_targets.m_destY + sin(angle) * radius;
-            float dest_z = m_caster->GetMap()->GetHeight(dest_x, dest_y, MAX_HEIGHT);
+            float dest_x, dest_y, dest_z;
+            m_caster->GetClosePoint(dest_x, dest_y, dest_z, 0.0f, radius, angle);
            m_targets.setDestination(dest_x, dest_y, dest_z);            

This part should not be there, simply since central coordinates taken from m_targets.m_destX / Y and not m_caster, Braking TARGET_RANDOM_NEARBY_DEST.

The other case is fine (TARGET_RANDOM_CIRCUMFERENCE_POINT).

Thanks in advance.

why you declare

float APCoeffMod = 1.0f;

if you just doesn't use it?

Thanks for noticing, that's actually a copy-paste typo from other cases - should have been used in line 8625.

should ave been "APCoeffMod += (*i)->GetModifier()->m_amount / 100.0f;"

You are right, I totally overlooked that iscontroled() check.

Strangely it didn't work in my tests prior to this change, but thats maybe be due to wrongly handled summons, or some custom additions of mine.

In any case, I think it still can be good having this option for controlled pets.

I thought that mirror image or snake trap summons GUARDIANS, not combat pets, so they should've been scriptable even before your patch, or? (according to comment - "// Allow scripting AI for normal creatures and not controlled pets (guardians and mini-pets)")

Every subtype of pets (aka : guardians, huter's, mini, controlled, whatever), considered to be a pet, created directly as Pet() class.

So, no - there is no way currently assigning any scripted ai to any of them besides petAI.

Something I made while ago, including Impurity.

http://github.com/abcxaos/mangos/commit/4de2fa02101039b508b171cb3811d6529cc8d6ec

Here's the info you requested.

Everything else that's known is already in MaNGOS' source.

Thanks. Updated the patch accordingly - nothing major, just few renames and wrong case for dk:DRW.

First port updated.

PS: anyone tried it and have negative/positive input?

You better thank Mangos team permanent members, that do the real work, not me.

Cheers guys.