WeiDUorg / weidu

WeiDU is a program used to develop, distribute and install modifications for games based on the Infinity Engine.
http://www.weidu.org
GNU General Public License v2.0
90 stars 20 forks source link

fj_are_structure may sometimes remove more than a single container item #85

Closed Argent77 closed 8 years ago

Argent77 commented 8 years ago

The given code

// Attempts to determine the index of an ARE container item structure.
DEFINE_PATCH_FUNCTION are_index_of_container_item
STR_VAR
  resource = ~~
RET
  index
BEGIN
  SET index = "-1"
  PATCH_IF (NOT ~%resource%~ STRING_EQUAL ~~) BEGIN
    READ_SHORT 0x76 numItems
    READ_LONG 0x78 ofsItems
    FOR (idx = 0; idx < numItems; ++idx) BEGIN
      SET curOfs = ofsItems + (idx * 0x14)
      READ_ASCII curOfs curRes ( 8 ) NULL
      PATCH_IF (~%curRes%~ STRING_EQUAL_CASE ~%resource%~) BEGIN
        SET index = idx
        SET idx = numItems
      END
    END
  END
END

// Removing single item
COPY_EXISTING ~ar3017.are~ ~override~
  LPF are_index_of_container_item STR_VAR resource = "tome02a" RET index END
  PATCH_IF (index >= 0) BEGIN
    LPF fj_are_structure
    INT_VAR
      fj_delete         = 1
      fj_delete_mode    = index
    STR_VAR
      fj_structure_type = "itm"
    END
  END
BUT_ONLY

is supposed to find the item index of TOME02A.ITM in AR3017.ARE (Watcher's Keep, Level 4) and delete it via fj_are_structure. However, it not only removes the desired item, but also removes the following item which is referenced by another container and happens to be the quest item "Crystal Mallet" (PLOT04D.ITM) in this instance.

I'm getting the same results in BG2:ToB, BG2:EE and EET. Using the code for different items on different maps appears to work correctly though.

Ardanis commented 8 years ago

This appears to happen when the item being deleted is in the last position within container. In that case the first item in the next container will also be deleted.

Ardanis commented 8 years ago

It checks "fj_delete_mode != item_count + item_index" and deletes if true. Problem is, the values essentially swap when the function proceeds to the next container, so it returns true twice instead of once.

FredrikLindgren commented 8 years ago

Fixed.