Skip to content

Arcane Combat dialog

Samuel Wegner requested to merge Effigy/t-engine4:arcane_combat_dialog into master

Allow selecting spell for Arcane Combat

-Arcane Combat talent now opens a dialog upon activation to allow selecting which spell will be triggered. A "Random spells" option is also available, which uses the existing behavior. The dialog and related code are based on the Contingency dialog.

-NPCs automatically use the random spell selection mode. Players will use random selection if a selected talent is not stored.

-If the selected spell is not valid (unlearned, not enough mana, etc.) when Arcane Combat triggers, then we default to using random spell selection. Let me know if you think this should be changed, but I thought it would be better than just failing to trigger.

-Rather than having the Arcane Combat logic directly list which talents can be triggered, it now checks a property "allow_for_arcane_combat" in the talent definition; allowed talents should have the property set to true. This paradigm makes it easier for addons to modify the logic.

-Arcane Combat talent description dynamically lists the allowed talents and (if sustained) which spell is currently selected. The description is getting a bit long, so let me know if you have any suggestions for making it more concise.

-Added Pulverizing Auger to the allowed spells. Since players can now choose which spell is triggered, this won't be a problem for builds that leave Auger at level 1 for utility. I thought this would be a good change to allow a non-elemental proc before unlocking Spell/Stone. Let me know if you have any objections.

KNOWN ISSUES:

-Hotkeys do not work in the dialog. The same issue exists with the Contingency dialog, so these could probably be fixed at the same time.

-Arcane Combat trigger logic assumes allowed talents use mana. This is fine for the allowed talents in the core game, but would prevent addons from allowing talents that use a different resource (or possibly cause bugs if they do allow them). I might look into abstracting the resource check to allow compatibility with different resource types, but that would be a future enhancement.

Merge request reports

Loading