devilutionX icon indicating copy to clipboard operation
devilutionX copied to clipboard

armor pierce conversion between vanilla and hellfire doesn't work at all

Open qndel opened this issue 3 years ago • 2 comments

Important information Compiled master - https://github.com/diasurgical/devilutionX/commit/90fbf1c535106826489e052ed976e2836c3bccbc

Describe armor pierce values aren't being converted between vanilla and hellfire at all

To Reproduce Generate any item with armor pierce in vanilla or hellfire Convert your character to hellfire/vanilla The values don't change. This is pretty serious imo as you can get an item with _iPLEnAc = 24 in vanilla (bashing affix), then it stays 24 after being converted to hellfire

	int CalculateArmorPierce(int monsterArmor, bool isMelee) const
	{
		int tmac = monsterArmor;
		if (_pIEnAc > 0) {
			if (gbIsHellfire) {
				int pIEnAc = _pIEnAc - 1;
				if (pIEnAc > 0)
					tmac >>= pIEnAc;
				else

so it basically sets the armor of all enemies to 0

Expected behavior Values should convert between vanilla and hellfire

Additional context Easy to test with replacing showing armor pierce with

	case IPL_TARGAC:
		return _("penetrates target's armor") + fmt::format(" ({:d})", item._iPLEnAc);

to see the actual value

qndel avatar Feb 09 '22 13:02 qndel

We could add a value to the item struct, and have it use that for the Diablo _iPLEnAc value. So when the affix is created, it saves two version of it, one for Hellfire (set at the param1 value) and one for Diablo (set at a random number between param1 and param2). When getting the item stats, it could then use whichever version is appropriate for the version of the game you are in. Then when saving and loading a character, check which version is being loaded, and load the appropriate version of the affix.

~~Or do nothing since the difference between _pIEnAC =4 and _pIEnAC = 24 isn't really that much (in a relative sense). Hellfire so OP.~~ I guess to be fair, it's likely that any of these affix are going to be super overpowered if created in Diablo and not Hellfire. Just thought of a maybe better idea: We should know the affix name, so just check the affix name and determine the correct value to use that way. Probably much easier than doing what I originally said.

Edit again - maybe not, the suffix name isn't saved (except as a language dependent string), just the type of power it is. Maybe adding another variable is the way to go 🤷🏽‍♂️

DakkJaniels avatar Feb 12 '22 00:02 DakkJaniels

I believe this should already be fixed for MP in 1.5.1, due to #6258. We may be able to use a similar approach to fix this in SP.

StephenCWills avatar Nov 15 '23 17:11 StephenCWills