[Patch][8249] Item Enchantment PPM Patch

[nos4r2zod]

I know a thread for this already exists on getmangos.ru, but unfortunately, I don't know any Russian, yet I wanted to make an update to the patches there.

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

• * Added a new SQL table, spell_proc_item_enchant, for "custom" ppmRates on Item Enchants that do not use auras. This allows us to fix the proc rate of Instant Poison, Wound Poison, Frostbrand Weapon, etc.
  * Changed GetPPMProcChance so that it actually returns a float value.
  * Implemented SPELLMOD_FREQUENCY_OF_SUCCESS (26). It functions the same as SPELLMOD_CHANCE_OF_SUCCESS, only 26 is used exclusively with PCT_MODIFIERs (This is for spells like Improved Poisons, Glyph of Seal of Command, and T8 Enhancement 4P Bonus)
  

For which repository revision was the patch created?

8236

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

I didn't find any besides the topic on .ru

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

Myself, based partly off of MaS0n's patch, which seems remotely inspired by thenecromancer's patch.

I did the following differently:

• * I used the procced spell's id (instead of the enchant id) to lookup entries in the table, like MaS0n's and unlike thenecromancer's patches. This is because it allows me to...
  * Use spell_chain for looking up ranks of enchant spells. While adding spell_chain data seems like it adds bulk to this patch's SQL data, the spells also need spell_bonus_data, so less entries are needed for that.
  * Put MaS0n's "HandleReloadSpellProcItemEnchantCommand" in the CommandTable so it can be used as its own chat command.
  * Switched the GetAttackTime calls in CastItemCombatSpell to using the itemProto's Delay. This is a really miniscule issue, but after a couple of attack speed changing mods are applied, floating point errors leads to an inaccuracy of a few centiseconds. It's not like it's necessary to use GetAttackTime anyway, considering the itemProto needs to be declared regardless.
  

Patch

DROP TABLE IF EXISTS `spell_proc_item_enchant`;
CREATE TABLE `spell_proc_item_enchant` (
 `entry` mediumint unsigned NOT NULL,
 `ppmRate` float NOT NULL default '0',
 PRIMARY KEY  (`entry`)
) ENGINE=MYISAM DEFAULT CHARSET=utf8;

INSERT INTO `spell_proc_item_enchant` (`entry`, `ppmRate`) VALUES
(8034, 9), -- Frostbrand Weapon
(8680, 8.5714), -- Instant Poison
(13218, 21.4286); -- Wound Poison

DELETE FROM `spell_chain` WHERE `spell_id` IN
(8034, 8037, 10458, 16352, 16353, 25501, 58797, 58798, 58799, 8680, 8685, 8689, 11335, 11336, 11337, 26890, 57964, 57965, 13218, 13222, 13223, 13224, 27189, 57974, 57975);

INSERT INTO `spell_chain` (`spell_id`, `prev_spell`, `first_spell`, `rank`, `req_spell`) VALUES
(8034,0,8034,1,0),     -- Frostbrand Attack
(8037,8034,8034,2,0),
(10458,8037,8034,3,0),
(16352,10458,8034,4,0),
(16353,16352,8034,5,0),
(25501,16353,8034,6,0),
(58797,25501,8034,7,0),
(58798,58797,8034,8,0),
(58799,58798,8034,9,0),
(8680,0,8680,1,0),     -- Instant Poison
(8685,8680,8680,2,0),
(8689,8685,8680,3,0),
(11335,8689,8680,4,0),
(11336,11335,8680,5,0),
(11337,11336,8680,6,0),
(26890,11337,8680,7,0),
(57964,26890,8680,8,0),
(57965,57964,8680,9,0),
(13218,0,13218,1,0),   -- Wound Poison
(13222,13218,13218,2,0),
(13223,13222,13218,3,0),
(13224,13223,13218,4,0),
(27189,13224,13218,5,0),
(57974,27189,13218,6,0),
(57975,57974,13218,7,0);

[VladimirMangos]

- uint32 WeaponSpeed = GetAttackTime(attType);

+ uint32 WeaponSpeed = proto->Delay;

I still think that this not correct. weapon speed can be affected by auras

And ppm expected to be still same (calculated base at current weapon attack rate)...

// Use first rank if current rank has no data

Maybe for speed _require_ first rank and check this at loading table.

SpellMgr::LoadSpellProcItemEnchant

You need at least check spell existance.

`entry` smallint(6) UNSIGNED NOT NULL,

You need at least middleint for possibility store current used spell ids range values.

[nos4r2zod]

- uint32 WeaponSpeed = GetAttackTime(attType);

+ uint32 WeaponSpeed = proto->Delay;

I still think that this not correct. weapon speed can be affected by auras

And ppm expected to be still same (calculated base at current weapon attack rate)...

This was intentional on my part, based on what I read a while ago here. But I am not sure if this applies the same way iit does item enchant procs as it does with procs from auras.

The rest of the comments, I'll try to look into; thank you

Edit:

You need at least check spell existance.

I see your point; and spell_threat table loading should do the same. I notice that are still ranks of Druid's Faerie Fire in that table, but it was reduced to one rank in 3.1.x.

Edit2:

Off topic, but is there even any plan to keep the spell_threat table? If so, this small patch turns the SQLStorage into a map so spell existence can be checked,

and here are the entries that can be removed:

DELETE FROM `spell_threat` WHERE `entry` IN (778,9749,9907,14274,15629,15630,15631,15632,17390,17391,17392,26993,27011);

Edit3:

Patch updated following Vladimir's 2nd, 3rd, and 4th comments. Will wait for more input on the first issue.

[VladimirMangos]

turns the SQLStorage into a map

This can checked and with SQLStorage, just do full loop by values with skip empty. From other side you have currently 3 spells in table but SQLStorage will alocate index size 0...maxSpellInTable with lot NULLs. In like cases better use map.

Off topic, but is there even any plan to keep the spell_threat table

Please suggest unrelated patch in different threads always.

[VladimirMangos]

Patch in [8249]. Thank you

[added]spell_threat related patch added in [8250]. Thank you But please next time post like patches in new thread.