Restore packed item state when regenerating an item using heroitem data

[StephenCWills]

Mirrors the logic in UnPackItem() to restore and validate the values of fields that would have been restored during unpacking if the item didn't need to be regenerated in LoadHeroItems().

devilutionX/Source/pack.cpp

Lines 405 to 411 in 49c5aea

	item._iIdentified = (packedItem.bId & 1) != 0;
	item._iMaxDur = packedItem.bMDur;
	item._iDurability = ClampDurability(item, packedItem.bDur);
	item._iMaxCharges = std::clamp<int>(packedItem.bMCh, 0, item._iMaxCharges);
	item._iCharges = std::clamp<int>(packedItem.bCh, 0, item._iMaxCharges);
	RemoveInvalidItem(item);

This resolves #6561
Introduced in 1.5.1 (#6258)
Unrelated to #5580

[StephenCWills]

Hang on, there are some potential issues I need to check into. Converting to draft.

[StephenCWills]

In Diablo, if a packed item is regenerated for the wrong game, it's going to be a Hellfire item regenerated as a Diablo item. In this case, the regenerated item should be considered valid for Diablo because the erroneous data wouldn't include Hellfire-only base items or item powers. This could result in false positives (invalid items look valid), but cannot result in false negatives (valid items look invalid).

In Hellfire, if a packed item is regenerated for the wrong game, it doesn't really matter. All items are considered valid in Hellfire so we still don't need to worry about false negatives. Since we don't need to worry about false negatives in either case, we don't need to worry about accidentally deleting valid items in UnPackItem().

To handle the false positive case, it would stand to reason that we should invoke RemoveInvalidItem() in LoadMatchingItems(). However, we don't really need to because RemoveInvalidItem() was already called on the heroitem data when we called LoadItemData(). If the item was invalid, we'd never make it past the check for heroItem.isEmpty(), except maybe in very specific edge cases where the save data was tampered with to make it look valid.

I removed the code to adjust CF_HELLFIRE in UnPackItem() because I determined it's unnecessary. All branches in RecreateItem() lead to either InitializeItem() or GetItemAttrs(), either of which will set the flag if necessary based on the value of isHellfire that gets passed in.

[StephenCWills]

In Diablo, if a packed item is regenerated for the wrong game, it's going to be a Hellfire item regenerated as a Diablo item.

Actually, this assertion seems to be wrong. If you take a Diablo item to Hellfire, the idx gets adjusted and the item is repacked as a Hellfire item. If that item is ever returned to Diablo, it could end up being unpacked as a Hellfire item with Hellfire affixes. That could result in false negatives. This is easy to reproduce with Demonspike Coat.

[StephenCWills]

I decided the best way to deal with this was to eliminate the possibility of both false positives and false negatives. RemoveInvalidItem() is called after loading all the heroitem data and matching it up with unpacked item data. Other items, like ground items for example, don't need the call to RemoveInvalidItem() to be deferred because heroitems only contains data from the player's inventory. So I kept them the same by calling LoadAndValidateItemData().

Demonspike Coat can now make the round-trip through Hellfire and back. This PR should be in good shape now.

[AJenbo]

This code (not the PR) is nightmare fule, lets hope it all goes well.