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

feat(Core/Items): Implemented elemental weapon damage. Source: Trinit… #13050

Merged
merged 9 commits into from Oct 8, 2022

Conversation

UltraNix
Copy link
Contributor

…yCore.

Closes #12165

Issues Addressed:

Tests Performed:

  • Not tested.

How to Test the Changes:

More info here

How to Test AzerothCore PRs

When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].

You can help by testing PRs and writing your feedback here on the PR's page on GitHub. Follow the instructions here:

http://www.azerothcore.org/wiki/How-to-test-a-PR

REMEMBER: when testing a PR that changes something generic (i.e. a part of code that handles more than one specific thing), the tester should not only check that the PR does its job (e.g. fixing spell XXX) but especially check that the PR does not cause any regression (i.e. introducing new bugs).

For example: if a PR fixes spell X by changing a part of code that handles spells X, Y, and Z, we should not only test X, but we should test Y and Z as well.

@Yehonal Yehonal added CORE Related to the core file-cpp Used to trigger the matrix build Script labels Sep 18, 2022
@UltraNix UltraNix added ChromieCraft Generic No specific levelrange Priority-High Waiting to be Tested BREAKING CHANGE Change that will require data re-extraction or major DB table alterations labels Sep 18, 2022
@Nefertumm
Copy link
Member

/home/runner/work/azerothcore-wotlk/azerothcore-wotlk/src/server/game/Entities/Unit/Unit.cpp:1452:54: fatal error: unused parameter 'damage' [-Wunused-parameter]
void Unit::CalculateMeleeDamage(Unit* victim, uint32 damage, CalcDamageInfo* damageInfo, WeaponAttackType attackType, const bool sittingVictim)

from checks

@Gultask
Copy link
Contributor

Gultask commented Sep 18, 2022

Tested with custom melee weapons with fixed elemental damage.
If anyone has any, please give me any more ideas of what to test, this is everything I came up with so far:

Only arcane damage is affected by resistances, while other types of damage have no partial resists nor full resists.
Wands and melee elemental damage bypasses immunities such as elementals. Even for arcane damage.

Melee weapon damage is increased by Ossirian Crystal debuff (Fire Weakness, Frost Weakness, etc)
Wand damage is not affected by Ossirian Crystal debuff

Melee weapon damage is increased and decreased by Chromaggus Elemental Shield.
Wand damage is not affected by Chromaggus Elemental Shield.

Copy link
Contributor

@acidmanifesto acidmanifesto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

@UltraNix
Copy link
Contributor Author

@Gultask I need all items you used during the tests.

@Gultask
Copy link
Contributor

Gultask commented Sep 24, 2022

Wands:
Excavation Rod: .additem 5246
Moonbeam Wand: .additem 5818
Gravestone Scepter: .additem 7001
Spellcrafter Wand: .additem 6677
Freezing Shard: .additem 10572

Melee Weapons:

DELETE FROM `item_template` WHERE `entry` BETWEEN 60001 AND 60006;
INSERT INTO `item_template` (`entry`, `class`, `subclass`, `SoundOverrideSubclass`, `name`, `displayid`, `Quality`, `Flags`, `FlagsExtra`, `BuyCount`, `BuyPrice`, `SellPrice`, `InventoryType`, `AllowableClass`, `AllowableRace`, `ItemLevel`, `RequiredLevel`, `RequiredSkill`, `RequiredSkillRank`, `requiredspell`, `requiredhonorrank`, `RequiredCityRank`, `RequiredReputationFaction`, `RequiredReputationRank`, `maxcount`, `stackable`, `ContainerSlots`, `StatsCount`, `stat_type1`, `stat_value1`, `stat_type2`, `stat_value2`, `stat_type3`, `stat_value3`, `stat_type4`, `stat_value4`, `stat_type5`, `stat_value5`, `stat_type6`, `stat_value6`, `stat_type7`, `stat_value7`, `stat_type8`, `stat_value8`, `stat_type9`, `stat_value9`, `stat_type10`, `stat_value10`, `ScalingStatDistribution`, `ScalingStatValue`, `dmg_min1`, `dmg_max1`, `dmg_type1`, `dmg_min2`, `dmg_max2`, `dmg_type2`, `armor`, `holy_res`, `fire_res`, `nature_res`, `frost_res`, `shadow_res`, `arcane_res`, `delay`, `ammo_type`, `RangedModRange`, `spellid_1`, `spelltrigger_1`, `spellcharges_1`, `spellppmRate_1`, `spellcooldown_1`, `spellcategory_1`, `spellcategorycooldown_1`, `spellid_2`, `spelltrigger_2`, `spellcharges_2`, `spellppmRate_2`, `spellcooldown_2`, `spellcategory_2`, `spellcategorycooldown_2`, `spellid_3`, `spelltrigger_3`, `spellcharges_3`, `spellppmRate_3`, `spellcooldown_3`, `spellcategory_3`, `spellcategorycooldown_3`, `spellid_4`, `spelltrigger_4`, `spellcharges_4`, `spellppmRate_4`, `spellcooldown_4`, `spellcategory_4`, `spellcategorycooldown_4`, `spellid_5`, `spelltrigger_5`, `spellcharges_5`, `spellppmRate_5`, `spellcooldown_5`, `spellcategory_5`, `spellcategorycooldown_5`, `bonding`, `description`, `PageText`, `LanguageID`, `PageMaterial`, `startquest`, `lockid`, `Material`, `sheath`, `RandomProperty`, `RandomSuffix`, `block`, `itemset`, `MaxDurability`, `area`, `Map`, `BagFamily`, `TotemCategory`, `socketColor_1`, `socketContent_1`, `socketColor_2`, `socketContent_2`, `socketColor_3`, `socketContent_3`, `socketBonus`, `GemProperties`, `RequiredDisenchantSkill`, `ArmorDamageModifier`, `duration`, `ItemLimitCategory`, `HolidayId`, `ScriptName`, `DisenchantID`, `FoodType`, `minMoneyLoot`, `maxMoneyLoot`, `flagsCustom`, `VerifiedBuild`) VALUES
(60006, 2, 7, -1, 'Teebu\'s Holy Longsword', 19997, 4, 0, 0, 1, 364684, 72936, 13, -1, -1, 65, 60, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1000, 1000, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1000, 0, 0, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 2, '', 0, 0, 0, 0, 0, 1, 3, 0, 0, 0, 0, 105, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 225, 0, 0, 0, 0, '', 65, 0, 0, 0, 0, 12340),
(60005, 2, 7, -1, 'Teebu\'s Arcane Longsword', 19997, 4, 0, 0, 1, 364684, 72936, 13, -1, -1, 65, 60, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1000, 1000, 6, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1000, 0, 0, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 2, '', 0, 0, 0, 0, 0, 1, 3, 0, 0, 0, 0, 105, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 225, 0, 0, 0, 0, '', 65, 0, 0, 0, 0, 12340),
(60004, 2, 7, -1, 'Teebu\'s Shadowy Longsword', 19997, 4, 0, 0, 1, 364684, 72936, 13, -1, -1, 65, 60, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1000, 1000, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1000, 0, 0, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 2, '', 0, 0, 0, 0, 0, 1, 3, 0, 0, 0, 0, 105, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 225, 0, 0, 0, 0, '', 65, 0, 0, 0, 0, 12340),
(60003, 2, 7, -1, 'Teebu\'s Nature Longsword', 19997, 4, 0, 0, 1, 364684, 72936, 13, -1, -1, 65, 60, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1000, 1000, 3, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1000, 0, 0, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 2, '', 0, 0, 0, 0, 0, 1, 3, 0, 0, 0, 0, 105, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 225, 0, 0, 0, 0, '', 65, 0, 0, 0, 0, 12340),
(60002, 2, 7, -1, 'Teebu\'s Chilling Longsword', 19997, 4, 0, 0, 1, 364684, 72936, 13, -1, -1, 65, 60, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1000, 1000, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1000, 0, 0, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 2, '', 0, 0, 0, 0, 0, 1, 3, 0, 0, 0, 0, 105, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 225, 0, 0, 0, 0, '', 65, 0, 0, 0, 0, 12340),
(60001, 2, 7, -1, 'Teebu\'s Warming Longsword', 19997, 4, 0, 0, 1, 364684, 72936, 13, -1, -1, 65, 60, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1000, 1000, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1000, 0, 0, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 0, 0, 0, 0, -1, 0, -1, 2, '', 0, 0, 0, 0, 0, 1, 3, 0, 0, 0, 0, 105, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 225, 0, 0, 0, 0, '', 65, 0, 0, 0, 0, 12340);

and .additem 60001 .additem 60002 .additem 60003 .additem 60004 .additem 60005 .additem 60006

@UltraNix
Copy link
Contributor Author

Only arcane damage is affected by resistances, while other types of damage have no partial resists nor full resists.
obraz_2022-09-25_100337464

Tested with fire wand and fire weapon with 58740 aura on dummy.

Wands and melee elemental damage bypasses immunities such as elementals. Even for arcane damage. What do you mean byimmunities ? Absorption auras like 53911 works okay.
obraz_2022-09-25_100746620

Remember that spells like Divine Shield works only with spells (includin wands), not melee. Blessing of Protection works only with phsycial melee.

Melee weapon damage is increased by Ossirian Crystal debuff (Fire Weakness, Frost Weakness, etc)
Melee weapon damage is increased and decreased by Chromaggus Elemental Shield.
It should be increased, because like you posted in 1. sentence, all elemental damage is modified by school resistances and auras.

Wand damage is not affected by Ossirian Crystal debuff
Wand damage is not affected by Chromaggus Elemental Shield.
Fixed.

@Gultask Please, retest it.

@Gultask
Copy link
Contributor

Gultask commented Sep 25, 2022

Oh, yes, I posted that to show the differences between melee and wands, I should've worded it better 😛

For the partial resistances, I tested it with creatures that have entries in the creature_template_resistances table, such as Ossirian, that has 1000 resistances to every school.
Tested again:
image

For the immunities I tested elementals such as Burning Exile (.go c id 2760)
Tested again:
image

@Gultask
Copy link
Contributor

Gultask commented Sep 30, 2022

Oh yes, I forgot about re-testing Ossirian and Chromaggus' weaknesses. They work fine now with wands.

@UltraNix
Copy link
Contributor Author

UltraNix commented Oct 1, 2022

@Gultask
For the partial resistances, I tested it with creatures that have entries in the creature_template_resistances table, such as Ossirian, that has 1000 resistances to every school.
I see Ossirian has 1000 resistance only to arcane.

@UltraNix
Copy link
Contributor Author

UltraNix commented Oct 1, 2022

For the immunities I tested elementals such as Burning Exile (.go c id 2760)
It's SPELL school immunity, not melee.

@Gultask
Copy link
Contributor

Gultask commented Oct 1, 2022

I messed up Ossirian in another PR. I'm sorry 🤦

@UltraNix
Copy link
Contributor Author

UltraNix commented Oct 1, 2022

@Gultask So everything is okay now?

@Gultask
Copy link
Contributor

Gultask commented Oct 1, 2022

I'll try again ASAP

@Gultask
Copy link
Contributor

Gultask commented Oct 1, 2022

This is WotLK Classic. Wands are affected by spell immunities.

image

@Gultask
Copy link
Contributor

Gultask commented Oct 1, 2022

Yes, my mistake. The resistances look fine, sorry about that.
Only the immunities left, I think.

@UltraNix
Copy link
Contributor Author

UltraNix commented Oct 2, 2022

@Gultask Should be okay now.

@Gultask
Copy link
Contributor

Gultask commented Oct 2, 2022

  • Immunities on wands work.
  • Immunities on melee register as a 'miss' and not 'immune', but in this case I don't know the correct behaviour.
  • Resistances on wands work.
  • Resistances on melee work.
  • Ossirian's weaknesses work.
  • Chromaggus' weaknesses work.
  • Frost attacks on Viscidus with wands or melee do not count towards the hit counter, but I believe that's for another PR?

Other than that I don't know of any other bosses where a testing could be useful, so I'm a bit limited here.

@UltraNix
Copy link
Contributor Author

UltraNix commented Oct 2, 2022

@Gultask Melee immunity should be okay now.

@Gultask
Copy link
Contributor

Gultask commented Oct 2, 2022

Spell, melee and wand immunities still work ok 👍

@UltraNix
Copy link
Contributor Author

UltraNix commented Oct 2, 2022

@Gultask Viscidus should be okay as well :)

@Gultask
Copy link
Contributor

Gultask commented Oct 2, 2022

😮
Tested latest commit.
Wand work now with Viscidus but not melee, but to be honest I don't see how that could ever be a problem xd
Spells still work as intended with Viscidus.
Thanks!

@UltraNix
Copy link
Contributor Author

UltraNix commented Oct 2, 2022

@Gultask Big kudos to you for testing this :)

@Gultask
Copy link
Contributor

Gultask commented Oct 2, 2022

Thanks for making the PR ^^

@Gultask Gultask added Merge Conflicts There are merge conflicts that need to be solved Tested This PR has been tested and is working. and removed To Be Merged Tested This PR has been tested and is working. labels Oct 2, 2022
@Gultask
Copy link
Contributor

Gultask commented Oct 2, 2022

Merge conflicts due to QAston

Edit: It has been reverted :)

@Gultask Gultask removed the Merge Conflicts There are merge conflicts that need to be solved label Oct 6, 2022
@Nefertumm Nefertumm merged commit e390087 into azerothcore:master Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE Change that will require data re-extraction or major DB table alterations ChromieCraft Generic No specific levelrange CORE Related to the core file-cpp Used to trigger the matrix build Script To Be Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Elemental weapon damage
5 participants