Ironbreaker career perk/ability fixes by Dannyboi802 · Pull Request #31 · TheOldRealms/TOR_Core

Dannyboi802 · 2026-09-07T20:46:28Z

Correct troop targeting, ammunition bonuses, and upgrade discounts.
Fix shield-block charging, Shieldwall targeting, and explosion/friendly-fire defenses.
Increase Impenetrable duration and separate Gromril/Rune perk activation.
Update descriptions.

- Correct troop targeting, ammunition bonuses, and upgrade discounts. - Fix shield-block charging, Shieldwall targeting, and explosion/friendly-fire defenses. - Increase Impenetrable duration and separate Gromril/Rune perk activation. - Update descriptions.

RandyChihuahua · 2026-09-07T20:59:36Z

+        public static float ApplyIronbreakerFriendlyFireReduction(Agent attacker, Agent victim, float damage)
+        {
+            if (Campaign.Current == null ||
+                Hero.MainHero?.HasCareerChoice("GromrilArmorPassive4") != true ||


Could pattern matching in conjunction with a NOT apply to any of these? might simplify it a bit.

!(attacker.Team is Team.Valid) for example.. food for thought. Close if wrong.

I like the guard clauses though.

SlyDevil · 2026-09-08T03:52:05Z

            if (chargeType == ChargeType.DamageTaken && affectedAgent == Agent.Main && affectedAgent.GetHero() == Hero.MainHero)
            {
-                return chargeValue * 5;
+                return (collisionFlag & CareerHelper.ChargeCollisionFlag.HitShield) != 0


Should this be shield only, or the enum is expanded to include weapon blocks and any sort of blocking counts?

it should be the shield only. ill test it

The question is about design intent, not functionality.
The restricted mechanical space is self-imposed. We can change the underlying flags to permit detecting other forms of defending oneself beyond blocking with a shield.

If shield-only blocks are the intention, the career ability description should probably specify that.

SlyDevil · 2026-09-08T04:00:29Z

@@ -1186,6 +1186,12 @@ public float CalculateWardSaveFactor(Agent attacker, Agent victim, float[] resis
            {
                result.LimitMin(0.11f);


Limit the min to 0.1f so an ironbreaker can actually benefit from the 90% ward save listed instead of 89%.

SlyDevil · 2026-09-08T04:02:52Z

            {
                result.LimitMin(0.11f);
            }
+            else if (victim.HasAttribute("Impenetrable"))


Add attributes to TORConstants.CharacterAttibutes. Their spelling only needs to exist in 1 location to limit errors.

SlyDevil · 2026-09-08T04:08:38Z

-                                            {
-                                                ammoCount.AddFactor(0.1f);
-                                            }
+                                            ammoCount.AddFactor(0.01f * ironbeardCount);


Store the 1% value in the PassiveEffect on the career choice, then fetch the choice and use choice.GetPassiveValue().

SlyDevil · 2026-09-10T17:58:25Z

-                                            }
-                                        }
-
+                                        ammoCount.Add(12);


Store specific values where possible on the choice so description and value are defined in the same place.

There's probably other lines this applies to, but not going to leave comments on them all.

SlyDevil · 2026-09-10T18:35:53Z

            if (characterObject.IsUndead()) return new ExplainedNumber(0);

            var explainedNumber = base.GetGoldCostForUpgrade(party, characterObject, upgradeTarget);
+            var applyIronbreakerDiscountLast = party.LeaderHero?.HasCareerChoice("IronPricePassive3") == true &&


Use an early out here for all non-MainParties as all subsequent evaluations don't matter to them.

Suggested change

var applyIronbreakerDiscountLast = party.LeaderHero?.HasCareerChoice("IronPricePassive3") == true &&

if (party.LeaderHero == null || party != PartyBase.MainParty || party.LeaderHero != Hero.MainHero) return explainedNumber;

SlyDevil · 2026-09-10T19:19:19Z


                    if (characterObject.HasAttribute(CharacterAttributes.IRONBREAKER))
                    {
                        explainedNumber.AddFactor(3f);


Change this to an Add instead of AddFactor.
.Add() will affect the base number that all factors are applied on and prevent this +300% increase from being additive with other factors. Consequently, all of the additions here for the IronPricePassive3 perk can be removed.

Suggested change

explainedNumber.Add(explainedNumber.BaseNumber * 3);

This won't be exactly identical to what's written below due to base.GetGoldCostForUpgrade factors that would have been folded into the BaseNumber and would therefore be multiplicative with the IronPricePassive3 reduction, but it will be relatively similar, simpler, and more consistent with how factors are applied in the game.

Few factors are multiplicative; making one so should have a conscious reason for why it deviates and description formulations considered in how we communicate to the player that a value is being treated differently from others.

SlyDevil · 2026-09-10T19:25:54Z


            if (characterObject.Culture.StringId == TORConstants.Cultures.DAWI)
            {
                if (party == PartyBase.MainParty)


This .MainParty check is superfluous if the guard clause is added higher up and can be removed.
party.LeaderHero... covers both the check for MainParty and has the incidental effect of checking if the player is a prisoner and would be able to attempt an upgrade in that state.

SlyDevil · 2026-09-10T19:49:21Z

-
-
-
+                    if (Hero.MainHero.HasCareerChoice("IronDrakesPassive2") && character.HasAttribute(CharacterAttributes.IRONBREAKER))


This can be removed. This is a bug I introduced in commit 799baa4 and I'll fix separately.

This would only fix the bug for the player when having this specific perk and troops instead of all contexts.

See commit b653ab9

SlyDevil · 2026-09-10T20:29:43Z

                    blow.VictimBodyPart = BoneBodyPartType.Chest;
                    blow.StrikeType = StrikeType.Thrust;
-                    if (hasShockWave)
+                    if (hasShockWave && !agent.HasAttribute("NestCleansing"))


Was there a case where using the AgentStat model was insufficient to have the desired behavior?

This seems unnecessary. GetKnockBackResistance and GetKnockDownResistance in the AgentStatCalculateModel are used as percentages of max health.
Ignoring knockdown resistance as it doesn't apply here, if agents with this attribute receive a value of 1 in those overrides, they can resist knockdown and knockback up to 100% of their hp. Anything beyond that wouldn't matter because they'd be dead.

The addition of BlowFlags here is only about the capacity for a blow to cause those effects, not about the agent's resistance to them.

This method shouldn't need to have any evaluations for specific contexts because those should already have been accounted for elsewhere. This method intends to apply the Blow for the engine's sake, but the factors adjusting the blow should already be determined before reaching this.

SlyDevil · 2026-09-10T20:43:25Z

            try
            {
                if (agent.IsFadingOut()) return;
+                damageAmount = (int)TORDamageHelper.ApplyIronbreakerFriendlyFireReduction(damager, agent, damageAmount);


The damage should already be known when entering this method, this evaluation doesn't go here.

If a value is entering and bypassing relevant damage reductions, the prior calculations should be accounting for those.
Note the routes that are failing to account for reductions so they can be fixed upstream at their source.

SlyDevil · 2026-09-10T21:01:12Z

                {
                    var baseDamage = explosionDamage * MBRandom.RandomFloatRanged(1 - damageVariance, 1 + damageVariance);
                    var damage = (explosionRadius - distance) / explosionRadius * baseDamage;
+                    damage = TORDamageHelper.ApplyIronbreakerExplosionDefenses(affector, agent, damage);


That's interesting to know that explosions have been bypassing the rest of the damage system.

I had a feeling they were, allied trollhammers made you feel way to squishy.

This is fine for the moment, but in the future we'll have to look at the triggered effects from explosions.

Some of them currently have these custom explosions activating that ignore the hit resolution system while others make use of the AffectsArea and AffectsAreaBig weapon flags which are resolved by passing through the system as a normal blow.
There's 2 behaviours occurring and there's no way for a player to predict how divergent they are in practice.

SlyDevil · 2026-09-10T21:08:28Z

+            return base.GetKnockBackResistance(agent);
+        }
+
        public override float GetKnockDownResistance(Agent agent, StrikeType strikeType)


Nest cleansing goes here if it prevents knockdown.

SlyDevil · 2026-09-10T21:16:39Z

                return;
            }

-            BookSpellDamage(spellDamage.CastId, spellDamage.Target, spellDamage.Damage, 0, spellDamage.DamageType);


Damage was already calculated upstream before it is queuing. This doesn't belong here.

Follow the references backwards and you should reach TORAbilityModel.CalculateAbilityDamage where the calculations are performed for resistances. These should be added with the others at that point otherwise you have created a new damage resistance that is multiplicative with others of the same damage type instead of additive like the rest.

SlyDevil · 2026-09-10T21:46:16Z

+            {
+                return false;
+            }
+


Do all explosions also carry the ShockWave attribute? Can that be added that to the guard before checking the string ids of irrelevant effects?

SlyDevil · 2026-09-10T21:59:05Z

+
+            // TOR's explosion templates use these identifiers, including cloned variants.
+            // HasShockWave alone also describes stomps and wind effects, not just explosions.
+            return template.StringID?.IndexOf("_explosion", StringComparison.OrdinalIgnoreCase) >= 0 ||


This criteria has a very large definition for "explosions" that will be affected by the NestCleansing keystone.

This is a design choice that requires more justification.

Due to the similarity in formulation between this keystone and the Gunpowder perk about reducing explosion damage, the definition in general needs to be explicited because their applications are very divergent in what they attempt to include.

Though, due to the code having been removed for the Bomb Suit perk at some point, there wasn't a way to have known that based on the available code.

SlyDevil · 2026-09-10T22:41:32Z

+            ApplyCareerPassives(attacker, victim, AttackTypeMask.Ranged, new float[(int)DamageType.All + 1], resistances);
+            ApplyNestCleansingExplosionResistance(victim, DamageType.Physical, resistances);
+
+            var physicalFactor = Math.Max(0f, 1f - resistances[(int)DamageType.Physical]);


Is DamageType.Physical an arbitrary choice made here, or is the intent to only affect Physical damage?
ie. Is there an incongruity with how you would apply the explosion damage reduction to triggered effects where it applies to all damage types.

SlyDevil · 2026-09-11T04:00:58Z

    protected override void InitializeKeyStones()
    {
-        _ironbreakerRoot.Initialize(CareerID, "Khazukan Kazakit-ha! For a brief period become Impenetrable. Gain +95% personal 'Ward Save', but move 25% slower. For every level of Athletics, gain 0.004s of Impenetrable. (Ability is charged by receiving and blocking damage.)", null, true,
+        _ironbreakerRoot.Initialize(CareerID, "Khazukan Kazakit-ha! Become Impenetrable for 10 seconds plus 0.05 seconds per Athletics point. Gain +90% personal 'Ward Save', but move 25% slower. (Ability is charged by receiving and blocking damage.)", null, true,


How long does this take to charge in practice?

The cooldown is entirely covered by the base duration here. With skill scaling at higher levels, the player is likely to have 30+ seconds of duration.
Is there any downtime or the player will have permanent uptime on at least 90% damage reduction + the rest of the buffs?

SlyDevil · 2026-09-11T04:29:30Z

            });

-        _gromrilArmorKeystone.Initialize(CareerID, "Impenetrable increases personal 'Physical Resistance' by 0.5% when a hit is taken, lasts 10s.", "GromrilArmor", false,
+        _gromrilArmorKeystone.Initialize(CareerID, "At 5s: +5% Physical Resistance per stored enemy melee hit for 10s. +25% reload speed; +0.02s Impenetrable per Gunpowder.", "GromrilArmor", false,


Why is 5 seconds the threshold?

SlyDevil · 2026-09-11T04:29:45Z

            });

-        _gromrilArmorKeystone.Initialize(CareerID, "Impenetrable increases personal 'Physical Resistance' by 0.5% when a hit is taken, lasts 10s.", "GromrilArmor", false,
+        _gromrilArmorKeystone.Initialize(CareerID, "At 5s: +5% Physical Resistance per stored enemy melee hit for 10s. +25% reload speed; +0.02s Impenetrable per Gunpowder.", "GromrilArmor", false,


"per stored melee hit" -> received hit?
Is this blocked? That deals damage to the player? Other?

SlyDevil · 2026-09-11T04:41:16Z

            });

-        _gromrilArmorKeystone.Initialize(CareerID, "Impenetrable increases personal 'Physical Resistance' by 0.5% when a hit is taken, lasts 10s.", "GromrilArmor", false,
+        _gromrilArmorKeystone.Initialize(CareerID, "At 5s: +5% Physical Resistance per stored enemy melee hit for 10s. +25% reload speed; +0.02s Impenetrable per Gunpowder.", "GromrilArmor", false,


Why is this scaling with Gunpowder and granting reload speed?

Neither of these seem related to armour made from high quality metal.
This looks like a previous copy-paste error is being propagated instead of removed.

With all 3 keystones granting reload rate increases, the player would have either +75% reload rate if they stack in effect, or a triple duration buff if they stack in duration, or identical buffs from 3 sources that don't stack.
None of these outcomes seems particularly intended from the code, description, and design notes are lacking.

SlyDevil · 2026-09-11T13:34:25Z

            });

-        _gromrilArmorKeystone.Initialize(CareerID, "Impenetrable increases personal 'Physical Resistance' by 0.5% when a hit is taken, lasts 10s.", "GromrilArmor", false,
+        _gromrilArmorKeystone.Initialize(CareerID, "At 5s: +5% Physical Resistance per stored enemy melee hit for 10s. +25% reload speed; +0.02s Impenetrable per Gunpowder.", "GromrilArmor", false,


Is the intent that this buff to resistance is applied while Impenetrable is active, or it's accumulated and applied after it wears off?
Overlaps with?

Given the uptime on Impenetrable with 90% damage reduction, is this meant to encourage the player to block attacks for the first couple seconds, to then have such high damage reduction that they can attack without care for defense for the next interval while it's active?

SlyDevil · 2026-09-11T13:38:14Z

            });

-        _runeWeaponsKeystone.Initialize(CareerID, "Impenetrable increases personal 'Physical' damage by 0.5% when a hit is taken, lasts 10s.", "RuneWeapons", false,
+        _runeWeaponsKeystone.Initialize(CareerID, "At 5s: +5% Physical damage per stored enemy melee hit for 10s. +25% reload speed; +0.02s Impenetrable per Gunpowder.", "RuneWeapons", false,


All of the comments for gromrilArmorKeystone apply here as well.
Why those values?
What's the usage intent of the buff? How would a player actively plan and execute on benefiting from it?
Why reload rate + gunpowder scaling?

SlyDevil · 2026-09-11T13:51:38Z

+        _ironDrakesPassive2.Initialize(CareerID, "-25% 'Oathgold' upgrade cost for Ironbreakers, Ironbeards, Irondrakes, and Trollhammer Irondrakes.", "IronDrakes", false, ChoiceType.Passive, null, new CareerChoiceObject.PassiveEffect(-25, PassiveEffectType.CustomResourceUpgradeCostModifier, true,
+            characterObject => characterObject.HasAttribute(CharacterAttributes.IRONBREAKER)));
+        _ironDrakesPassive3.Initialize(CareerID, "+12 ammunition for Drakefire canisters carried by you and companions in your party.", "IronDrakes", false, ChoiceType.Passive, null, new CareerChoiceObject.PassiveEffect());
+        _ironDrakesPassive4.Initialize(CareerID, "+1% ammunition for 'Ironbreaker' troops per Ironbeard unit.", "IronDrakes", false, ChoiceType.Passive, null, new CareerChoiceObject.PassiveEffect());


"per Ironbeard unit" seems like it'll be confusing. I usually read "unit" as a composition of troops which implies to me that I'd get only +1% ammo.

Perhaps, "+1% ammunition for Ironbreakers per Ironbeard troop."?

The presence of both "troops" and "unit" in the description implies that they are referring to different categorizations.

SlyDevil · 2026-09-11T13:55:05Z

        _nestCleansingPassive1.Initialize(CareerID, "+10 personal Hitpoints.", "NestCleansing", false, ChoiceType.Passive, null, new CareerChoiceObject.PassiveEffect(10, PassiveEffectType.Health));
        _nestCleansingPassive2.Initialize(CareerID, "+20% personal 'Fire Resistance'.", "NestCleansing", false, ChoiceType.Passive, null, new CareerChoiceObject.PassiveEffect(PassiveEffectType.Resistance, new DamageProportionTuple(DamageType.Fire, 20), AttackTypeMask.All));
-        _nestCleansingPassive3.Initialize(CareerID, "Explosive charges gain +2 ammunition.", "NestCleansing", false, ChoiceType.Passive, null, new CareerChoiceObject.PassiveEffect());
+        _nestCleansingPassive3.Initialize(CareerID, "Your personal explosive charges gain +2 ammunition.", "NestCleansing", false, ChoiceType.Passive, null, new CareerChoiceObject.PassiveEffect());


Could this not have been achieved with PassiveEffectType.Ammo, putting the value into the PassiveEffect, and adding a predicate for detecting explosive charges?

SlyDevil · 2026-09-11T13:59:54Z

-        _gromrilArmorPassive1.Initialize(CareerID, "+20% 'Physical Resistance' for 'Ironbreaker' troops.", "GromrilArmor", false, ChoiceType.Passive, null, new CareerChoiceObject.PassiveEffect(PassiveEffectType.TroopResistance, new DamageProportionTuple(DamageType.Physical, 20), AttackTypeMask.Melee,
-            (attacker, victim, mask) => attacker.Team == victim.Team && attacker.Character.StringId.Contains("ironbreaker")));
+        _gromrilArmorPassive1.Initialize(CareerID, "+20% 'Physical Resistance' for 'Ironbreaker' troops.", "GromrilArmor", false, ChoiceType.Passive, null, new CareerChoiceObject.PassiveEffect(PassiveEffectType.TroopResistance, new DamageProportionTuple(DamageType.Physical, 20), AttackTypeMask.All,
+            (attacker, victim, mask) => victim.BelongsToMainParty() && victim.Character.IsIronbreakerUnit()));


agent.IsPlayerTroop or .IsPlayerUnit are simpler checks than .BelongsToMainParty to determine the same thing.

Ya, wow, I see how common that BelongsToMainParty check is.

I've left a note directly on the agent.BelongsToMainParty (albeit on a different branch that isn't yet merged to next_update) about this.

Leave this as is for the moment; the actual check being performed within BelongsToMainParty is likely what will be changed.

SlyDevil · 2026-09-11T14:20:48Z

This would have been overall easier to go through if the changes for each perk were in separate commits to simplify following the changes and their impacts between files.

Even with your spreadsheet detailing the previous functionality and what you were aiming for with these changes, there's often a lack of sufficient context to follow how you're going about accomplishing the goals and why in the way you've chosen.
Sharing some of your thinking makes it easier to differentiate between "goal is correct, but method to reach it isn't" and "stated goal isn't the actual goal, the method is fine but seems wrong due to lack of information".

Dannyboi802 requested a review from SlyDevil September 7, 2026 20:46

RandyChihuahua reviewed Sep 7, 2026

View reviewed changes

SlyDevil reviewed Sep 8, 2026

View reviewed changes

SlyDevil reviewed Sep 10, 2026

View reviewed changes

Merge branch 'next_update' into Ironbreaker-fixes

070efa7

SlyDevil reviewed Sep 11, 2026

View reviewed changes

		@@ -1186,6 +1186,12 @@ public float CalculateWardSaveFactor(Agent attacker, Agent victim, float[] resis
		{
		result.LimitMin(0.11f);

	var applyIronbreakerDiscountLast = party.LeaderHero?.HasCareerChoice("IronPricePassive3") == true &&
	if (party.LeaderHero == null \|\| party != PartyBase.MainParty \|\| party.LeaderHero != Hero.MainHero) return explainedNumber;




		if (Hero.MainHero.HasCareerChoice("IronDrakesPassive2") && character.HasAttribute(CharacterAttributes.IRONBREAKER))

Conversation

Dannyboi802 commented Sep 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SlyDevil Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SlyDevil Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SlyDevil Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SlyDevil commented Sep 11, 2026

SlyDevil Sep 10, 2026 •

edited

Loading

SlyDevil Sep 10, 2026 •

edited

Loading

SlyDevil Sep 10, 2026 •

edited

Loading