search

ZhengPeiRu21 / mod-individual-progression

AzerothCore Individual Progression Module
MIT License
118 stars 54 forks source link

add optional sql to make buff scrolls stackable #404

Closed sogladev closed 3 weeks ago

sogladev commented 1 month ago

Changes

Buff scrolls are made exclusive in the core by checking their first_spell_id in spell_ranks. Removing these entries make them stackable.

Note that scrolls do stack with elixirs like they do in vanilla. e.g. elixir of major strength (+str) stacks with a scroll of strength (+str). In TBC+ scrolls were made to act like battle/guardian elixirs and should not stack, but this blizzlike handling is not present in the core. this might be classic TBC+ change

query to re-insert `spell_ranks` data DELETE FROM `spell_ranks` WHERE `first_spell_id` IN (8118, 8099, 8112, 8096, 8115, 8091); INSERT INTO `spell_ranks` (`first_spell_id`, `spell_id`, `rank`) VALUES (8091, 8091, 1), (8091, 8094, 2), (8091, 8095, 3), (8091, 12175, 4), (8091, 33079, 5), (8091, 43196, 6), (8091, 58452, 7), (8091, 58453, 8), (8096, 8096, 1), (8096, 8097, 2), (8096, 8098, 3), (8096, 12176, 4), (8096, 33078, 5), (8096, 43195, 6), (8096, 48099, 7), (8096, 48100, 8), (8099, 8099, 1), (8099, 8100, 2), (8099, 8101, 3), (8099, 12178, 4), (8099, 33081, 5), (8099, 48101, 6), (8099, 48102, 7), (8099, 43198, 8), (8112, 8112, 1), (8112, 8113, 2), (8112, 8114, 3), (8112, 12177, 4), (8112, 33080, 5), (8112, 43197, 6), (8112, 48103, 7), (8112, 48104, 8), (8115, 8115, 1), (8115, 8116, 2), (8115, 8117, 3), (8115, 12174, 4), (8115, 33077, 5), (8115, 43194, 6), (8115, 58450, 7), (8115, 58451, 8), (8118, 8118, 1), (8118, 8119, 2), (8118, 8120, 3), (8118, 12179, 4), (8118, 33082, 5), (8118, 43199, 6), (8118, 58448, 7), (8118, 58449, 8);

Issue

How to test

add scroll of intellect, scroll of strength, scroll of protection

use these scrolls and see if they stack

ZhengPeiRu21 commented 3 weeks ago

Thank you for implementing this option!

Grimfeather commented 3 weeks ago

Is this not a problem for TBC and WotLK? It may give the desired effect for vanilla, but if spell scroll stacking was not a thing in TBC and WotLK then it solves 1 issue, but creates a new one.

sogladev commented 3 weeks ago

Is this not a problem for TBC and WotLK? It may give the desired effect for vanilla, but if spell scroll stacking was not a thing in TBC and WotLK then it solves 1 issue, but creates a new one.

Yes, it's universal with this change.

The issue could be re-opened for a better approach that checks for progression status

Maybe the best approach is to use spell scripts on (-8118, -8099, -8112, -8096, -8115, -8091) that check for progression and then apply some kind of stack logic like https://github.com/azerothcore/azerothcore-wotlk/blob/6194cc864bd7613dc24af809794bb81abe06d601/src/server/game/Spells/Auras/SpellAuras.cpp#L2071

Grimfeather commented 3 weeks ago

what about after removing the ranks, new ranks would be added again with scrolls starting in TBC? would that work?

for the strength scrolls as an example:

DELETE FROM spell_ranks WHERE first_spell_id IN (8118, 12179); INSERT INTO spell_ranks (first_spell_id, spell_id, rank) VALUES (12179, 12179, 1), (12179, 33082, 2), (12179, 43199, 3), (12179, 58448, 4), (12179, 58449, 5);

sogladev commented 3 weeks ago

what about after removing the ranks, new ranks would be added again with scrolls starting in TBC? would that work? ...

this would have no effect as the the first_spell_id is hard-coded in the core case 8118: // Strength https://github.com/azerothcore/azerothcore-wotlk/blob/6194cc864bd7613dc24af809794bb81abe06d601/src/server/game/Spells/SpellInfo.cpp#L2180

I thought about not removing all the entries in the rank_table. Only the vanilla ones, that way you can stack vanilla ones together, but not the high level ones. You'd still be able to stack high level scrolls with low level scrolls, e.g. High lvl strength with low lvl agility. It doesn't completely solve it but it's a bit better