[Bug] "Disable Portrait Icons Added by Equipped Items: v16" broken when installed with another tweak

[marco_costa]

Steps to repro on game directory (under Linux, latest weidu - I'm not assuming it breaks the same way on Windows, did not have the chance to try and reproduce it there):

1. weinstall cdtweaks, pick "Disable Portrait Icons Added by Equipped Items: v16" and "Separate Resist Fire/Cold Icon into Separate Icons [Angel]: v16".
2. wine Baldur.exe
3. use CLUAConsole:CreateItem("HELM04",1,0,0), equip helm, see that Resist Fire and Resist Cold portraits still appear on character portrait.

Steps that fix:

1. weinstall cdtweaks --force-install 2010, then weinstall cdtweaks --force-install 90
2. repeat steps 2 and 3 above, but this time no Resist Fire and Resist Cold appear on the character portrait.

[jmerry]

The issue is not platform dependent, and I'd put the blame entirely on the "Separate Resist Fire/Cold Icon into Separate Icons" component.

The way the component for splitting the resist fire/cold icon works ... actually, here's the entire code.

INCLUDE ~cdtweaks/lib/bg2fp_effect_batches.tpa~

COPY_EXISTING_REGEXP GLOB ~^.+\.cre$~ ~override~
                          ~^.+\.itm$~ ~override~
                          ~^.+\.spl$~ ~override~
  LPF cd_apply_batch STR_VAR array_name = cd_cold_icons END
  LPF cd_apply_batch STR_VAR array_name = cd_fire_icons END
  BUT_ONLY

The real action there is in a separate library of standardized stuff originally developed for the BG2 fixpack. Here, the cd_cold_icons batch removes the resist fire/cold icon and adds the cold resistance icon, if a cold resistance effect is present. The cd_fire_icons batch removes the resist fire/cold icon and adds the fire resistance icon, if a fire resistance icon is present.

So here's your issue. If you install the other component first and remove portrait icons from equipped icons, this component will add fire and/or cold resistance icons back in whenever the item grants fire and/or cold resistance.

This doesn't seem like a good way of doing things. Wouldn't it make more sense to key everything off the portrait icons? If the resist fire/cold icon is present, replace it with the fire resistance and cold resistance icons. I mean, the following code should work in place of the above:

COPY_EXISTING_REGEXP GLOB ~^.+\.cre$~ ~override~
                          ~^.+\.itm$~ ~override~
                          ~^.+\.spl$~ ~override~
  LPF CLONE_EFFECT INT_VAR match_opcode = 142 match_parameter2 = 26 parameter2 = 16
  LPF ALTER_EFFECT INT_VAR match_opcode = 142 match_parameter2 = 26 parameter2 = 25
BUT_ONLY

It might be possible to optimize that better for speed, but it does seem entirely functional to me. And it would fix your issue; if the resist fire/cold icon has already been removed by something else, this won't be adding fire resistance and cold resistance icons.

[marco_costa]

I'm probably not qualified to opine on optimization and weidu best practices since I'm new at this, but your solution worked.

I just added a silent = 1 to both LPF calls to avoid the deluge of "WARNING: no effects added to X" .

Thank you!

Edited June 21, 2023 by marco_costa

[marco_costa]

On the 'optimize for speed' front, my instinct would be to refactor the COPY_EXISTING_REGEXP call to use a single regex with cre/itm/spl or just cre/spl depending on whether setup-cdtweaks.tp2 #90 has been installed.

That'd introduce coupling between the tweaks, but eliminate the need to silent = 1 the LPF calls.

EDIT: heh, that's not exactly right. my instincts are as naive as I'd expect.

Edited June 21, 2023 by marco_costa
correcting for wrongness

[jmerry]

Yeah, definitely silent there. I just posted quick and dirty, not thinking about that aspect.

[marco_costa]

INCLUDE ~cdtweaks/lib/bg2fp_effect_batches.tpa~

DEFINE_ACTION_FUNCTION replace_fire_cold_resist_icons
BEGIN
  // If "Remove icons added by equipping items" was installed, there
  // are no Resist Fire/Cold portrait icons in ITM files to remove
  ACTION_IF MOD_IS_INSTALLED ~%TP2_FILE_NAME%~ ~90~ THEN BEGIN
    OUTER_SPRINT regex ~^.+\.\(cre\|spl\)$~
  END ELSE BEGIN
    OUTER_SPRINT regex ~^.+\.\(cre\|spl\|itm\)$~
  END
  COPY_EXISTING_REGEXP GLOB ~%regex%~ ~override~
    LPF cd_apply_batch STR_VAR array_name = cd_cold_icons END
    LPF cd_apply_batch STR_VAR array_name = cd_fire_icons END
    BUT_ONLY
END

LAF replace_fire_cold_resist_icons END

Attempt #2 at the efficiency angle. Seems to work so long as the assumption I lay out in the comment isn't wrong.

[Axatax]

On a similar note, the "disable portrait icons" causes the game to freeze when the Girdle of Masculinity/Femininity is identified by a shopkeeper, and then the girdle is subsequently picked up in the inventory screen.

 

[CamDawg]

On 8/26/2023 at 9:27 AM, Axatax said:

On a similar note, the "disable portrait icons" causes the game to freeze when the Girdle of Masculinity/Femininity is identified by a shopkeeper, and then the girdle is subsequently picked up in the inventory screen.

I'm unable to repro this.

Otherwise, Separate Resist Fire/Cold Icon into Separate Icons [Angel] will no longer add portrait icons for equipped items if the Disable Portrait Icons Added by Equipped Items component is installed.