GrognardsFromHell / TemplePlus

ToEE hooks, extensions and fixes
https://github.com/GrognardsFromHell/DllDocumentation/wiki
MIT License
86 stars 22 forks source link

AC enhancement bonuses aren't implemented correctly #750

Closed dolio closed 8 months ago

dolio commented 9 months ago

While messing around a few days back, I realized that various AC enhancement bonuses aren't actually implemented properly. Here's the way it works right now:

This might seem like it would give the correct result, but it actually doesn't. You can observe the results this way:

Tabletop rules say this should give you +7 AC from armor. The enhancement modifies the chain's base 4 AC to 7, which is greater than the 6 from the bracers. However, in ToEE you get +9 AC, because you get the +6 base armor bonus from the bracers (which is grater than the +4 from the shirt) and the +3 enhancement bonus from the shirt still applies.

I suspect this also happens with shields. So if you wear a +3 buckler, casting shield probably gives you +7 shield AC instead of +4. It would also happen for natural armor, but I don't think there are ways to get multiple sources of natural armor; all the spells and items give enhancements, and the base just comes from the object fields.

I'm not sure how to fix this best. I'll think it over.

dolio commented 9 months ago

I think this is actually pretty easy to fix.

There's already a d20 query Q_Armor_Get_AC_Bonus for calculating the AC of a specific piece of worn armor. You can call it from the console with:

critter.d20_query_with_object(Q_Armor_Get_AC_Bonus, armor)

This will give you 7 for the chain shirt example. It works for shields, too.

So, instead of just having both the Armor Bonus and Armor Enhancement Bonus respond directly to the GetAC event, I think only Armor Bonus should, and it should figure out what value to add by running that d20 query to incorporate the enhancement bonus. That way you get just armor bonuses from each source, and they won't stack. Same with shields.

doug1234 commented 9 months ago

That seems like it would work well. Have you given it a try?

dolio commented 9 months ago

Not yet. Maybe I'll just do it, though, because I have to mess around with the shield bonus stuff anyway for shield bashing, and once I get that working it's not much more work to do for armor as well.

dolio commented 9 months ago

Okay, so, there's actually a problem with my strategy above. The d20 query isn't correct, either. If you get multiple enhancement bonuses on an item they stack, because a d20 query only has a return value, not a bonus list. You can actually see this in game, because the query is used for the AC value reported in the UI. If you cast e.g. Magic Vestment on an already enchanted piece of armor, the UI will say it provides more AC than it actually does.

I could fix the query by doing some packing, and fix up the result where it's called. Like, put the base AC bonus in the low byte, and the enhancement bonus in the second byte. The queries would manually take the max of the relevant byte with their value. Then you add the two bytes up at the end.

I only see two places where it's called. One is the monk AC bonus, which just checks if it's non-zero, and doesn't require a change. The other is the UI function, which is already partially replaced for the armor UI, so it's not a big deal to tweak that, either. Then it would fix the AC reported in the UI as well.

Does that sound all right?