Using tables to make a patch

[WanderingScholar]

I'm trying to write a mod that patches the prices of all items in the game. The purpose is to revert the price changes made by the Item Revisions mod. 

The first component is installed before IR. It gathers and stores all the items and their prices into a table. "pricelist.2da" I couldn't think of another way to do this.

The second component, which should be installed after IR, is meant to read pricelist.2da and patch all of the items in the game to match the corresponding prices listed in the table.

I'm struggling to write this second component. I'm assuming I need to use something like READ_2DA_ENTRIES_NOW to get the info out of the table, then store it in a variable to be used in some of loop. I guess I'd have to match the SOURCE_RES of the item with it's SOURCE_RES in the table, then tell it to patch the price value in that same row. 

Any ideas?

////////////////////////////////////////////////////////////////////
BEGIN ~Read in item prices for IR price reversions~
FORBID_COMPONENT "item_rev.tp2" 0 "This component needs to be installed before Item Revisions"
////////////////////////////////////////////////////////////////////

<<<<<<<<.../WStweaks-inline/pricelist.2da
>>>>>>>>
MKDIR ~weidu_external/data/WStweaks~
COPY ~.../WStweaks-inline/pricelist.2da~ ~weidu_external/data/WStweaks~

OUTER_SPRINT data ~~
COPY_EXISTING_REGEXP - ~.*\.itm~ nowhere
    READ_LONG 0x34 itemprice
    SPRINT data ~%data%%SOURCE_RES%%TAB%%itemprice%%WNL%~
    
APPEND_OUTER ~weidu_external/data/WStweaks/pricelist.2da~ ~%data%~

////////////////////////////////////////////////////////////////////
BEGIN ~Change item prices for IR to their original values~
REQUIRE_COMPONENT "item_rev.tp2" 0 "This component needs to be installed after Item Revisions"
////////////////////////////////////////////////////////////////////

?

[jmerry]

A neat trick: READ_2DA_ENTRIES_NOW directly stores the data as an array, which you can continue to use after leaving the 2DA behind. Here's one of my components that reads data from a table:

Spoiler

BEGIN @45200 DESIGNATED 452 // Make Hexxat a Shadowdancer
SUBCOMPONENT @45000
GROUP @3
REQUIRE_PREDICATE GAME_IS ~bg2ee eet~ @11
COPY ~jtweaks/resource/2da/j8hhexsd.2da~ ~override~ // Table of data for changes
	READ_2DA_ENTRIES_NOW ~j8hhexsd~ 8
BUT_ONLY // No need to keep a copy

OUTER_FOR (row = 0; row < j8hhexsd; ++row) BEGIN
	OUTER_SPRINT FileID $j8hhexsd(~%row%~ ~0~)
	COPY_EXISTING ~%FileID%.cre~ ~override~
		WRITE_BYTE 0x273 4 // Class - thief
		WRITE_LONG 0x244 0x40210000 // Kit - shadowdancer
		WRITE_BYTE 0x234 $j8hhexsd(~%row%~ ~1~) // Level 1
		WRITE_BYTE 0x45 $j8hhexsd(~%row%~ ~2~)  // Hide in Shadows
		WRITE_BYTE 0x64 $j8hhexsd(~%row%~ ~3~)  // Detect Illusion
		WRITE_BYTE 0x65 0 // Set Traps
		WRITE_BYTE 0x67 $j8hhexsd(~%row%~ ~4~)  // Open Locks
		WRITE_BYTE 0x68 $j8hhexsd(~%row%~ ~5~)  // Move Silently
		WRITE_BYTE 0x69 $j8hhexsd(~%row%~ ~6~)  // Find Traps
		WRITE_BYTE 0x6a $j8hhexsd(~%row%~ ~7~)  // Pick Pockets
	BUT_ONLY
END

 

Hopefully, that helps with your second component.

In my example, the table is pre-built; the first column is the resource ID for a given CRE, and the remaining columns are data for that CRE. I also have a header row, which is discarded by the READ_2DA_ENTRIES_NOW because it's shorter.

Edited December 22, 2023 by jmerry

[WanderingScholar]

8 hours ago, jmerry said:

Hopefully, that helps with your second component.

In my example, the table is pre-built; the first column is the resource ID for a given CRE, and the remaining columns are data for that CRE. I also have a header row, which is discarded by the READ_2DA_ENTRIES_NOW because it's shorter.

Thanks a lot for the help @jmerry. I feel like I'm getting close, but I just can't make it over the finish line . It's the first time I'm using most of these commands. WeiDU doesn't like what is happening at PATCH_IF ("%row_0%" STRING_EQUAL_CASE "item_name") THEN BEGIN. I get a GLR Parse Error. This is the hardest thing to figure out. I need to match all the item names in the game with those in every row of first column in pricelist.2da

Spoiler

////////////////////////////////////////////////////////////////////
BEGIN ~Change item prices for IR to their original values~
//REQUIRE_COMPONENT "item_rev.tp2" 0 "This component needs to be installed after Item Revisions"
////////////////////////////////////////////////////////////////////

COPY ~weidu_external/data/WStweaks/pricelist.2da~ ~override~
    READ_2DA_ENTRIES_NOW pricelist 2  // Read the entries in "pricelist". "pricelist.2da" is only 2 columns 
BUT_ONLY

COPY_EXISTING_REGEXP GLOB ~.*\.itm~ ~override~
  PATCH_IF (SOURCE_SIZE > 0x71) THEN BEGIN // only patch if the item file is large enough to have a price field
    READ_LONG 0x34 "price" // read the current price at offset 0x34
    END
    OUTER_SET "new_price" = "-1" // Set the "new_price" to "-1" to check later if it changes
    OUTER_SPRINT "item_name" "%SOURCE_RES%" // Get all the item source names
    ACTION_FOR_EACH row IN pricelist BEGIN 
      PATCH_IF ("%row_0%" STRING_EQUAL_CASE "item_name") THEN BEGIN 
        SET "new_price" = "%row_1%"
      END
    END
    PATCH_IF ("%new_price%" > "-1") THEN BEGIN // check if a new price is specified
      WRITE_LONG 0x34 ("%new_price%") // write the new price at offset 0x34
    END
  END
BUT_ONLY

 

 

[jmerry]

Oh, there's lots of broken things about that code. Copied, with tweaks to indenting and new comments.

Spoiler

////////////////////////////////////////////////////////////////////
BEGIN ~Change item prices for IR to their original values~
//REQUIRE_COMPONENT "item_rev.tp2" 0 "This component needs to be installed after Item Revisions"
////////////////////////////////////////////////////////////////////

COPY ~weidu_external/data/WStweaks/pricelist.2da~ ~override~
    READ_2DA_ENTRIES_NOW pricelist 2  // Read the entries in "pricelist". "pricelist.2da" is only 2 columns 
BUT_ONLY

COPY_EXISTING_REGEXP GLOB ~.*\.itm~ ~override~
  PATCH_IF (SOURCE_SIZE > 0x71) THEN BEGIN
    READ_LONG 0x34 "price"
  END // Ends IF block
  OUTER_SET "new_price" = "-1" // This is an ACTION. Close the item. Or actually, wait until you've gone through all the items and only do this after the last one. Even if zzz.ITM was a stub with zero file size.
  OUTER_SPRINT "item_name" "%SOURCE_RES%" // Another ACTION
  ACTION_FOR_EACH row IN pricelist BEGIN // Another ACTION
    PATCH_IF ("%row_0%" STRING_EQUAL_CASE "item_name") THEN BEGIN // PATCH but nothing's loaded. Syntax error.
      SET "new_price" = "%row_1%" // Another PATCH.
    END // Close IF block
  END // Close array loop. Also, see explanation below for why your structure is inefficient.
  PATCH_IF ("%new_price%" > "-1") THEN BEGIN // PATCH
    WRITE_LONG 0x34 ("%new_price%") // PATCH
  END // Close IF block
  END // Close ... nothing. Syntax error.
BUT_ONLY // If you still had an item loaded, this would be useful.

 

So the main thing there is the distinction between ACTION and PATCH. When you try to use an ACTION with a file loaded, you close that file. When you try to use a PATCH without a file loaded, you get a syntax error. Lots of commands, like OUTER_SET and SET for setting integer variables, have both ACTION and PATCH versions; make sure you're using the right versions for your current environment.

Then there are two more issues.

First, and more minor, your sanity check is basically nullified by ENDing it before you do anything else. If you want to condition your changes on the file being valid, you have to put your changes inside the IF block. Which is probably about the extra END; in retrospect, the END immediately after the IF looks like the one that shouldn't be there.

Second, your fundamental structure here is O(n^2) in the number of items, when it could be O(n). There are about 3000 items in unmodified BG2EE. Making your code run a thousand times faster is absolutely worth it. What you're currently doing - or at least, trying to do - is to iterate over every item, then compare each item to the entire array of items and do stuff when they match. What you could do instead is iterate over the array and load the item file the current row corresponds to:

Spoiler

ACTION_FOR_EACH row IN pricelist BEGIN
  COPY_EXISTING ~%row_0%.itm~ ~override~ // First entry in row is item name
    // Do stuff with the item and the row's data
  BUT_ONLY
END // End array loop

 

That way, you're only making a single pass through the items, in that array loop. 

[WanderingScholar]

Holy smokes. So I was aware of the distinction between ACTION and PATCH, but for some reason I spaced out when writing this. This is the 3rd iteration of this code I've attempted. 

13 minutes ago, jmerry said:

First, and more minor, your sanity check is basically nullified by ENDing it before you do anything else. If you want to condition your changes on the file being valid, you have to put your changes inside the IF block. Which is probably about the extra END; in retrospect, the END immediately after the IF looks like the one that shouldn't be there.

 Yeah, the reason I added "END // Ends IF block" is because I was getting frustrated and blindly responding to WeiDU errors. Looks like many things were broken anyway. 

16 minutes ago, jmerry said:

Second, your fundamental structure here is O(n^2) in the number of items, when it could be O(n). There are about 3000 items in unmodified BG2EE. Making your code run a thousand times faster is absolutely worth it. What you're currently doing - or at least, trying to do - is to iterate over every item, then compare each item to the entire array of items and do stuff when they match. What you could do instead is iterate over the array and load the item file the current row corresponds to:

Good to know. I wasn't thinking about this. I was just trying to get it to run. 

[WanderingScholar]

Hopefully this will be the last one.

This throws a ERROR locating resource for 'COPY' for ~%row_0%.itm~. Resource [%row_0%.itm] not found in KEY file:

Spoiler

ACTION_FOR_EACH row IN pricelist BEGIN
  OUTER_SET "new_price" = "-1"
  COPY_EXISTING ~%row_0%.itm~ ~override~
    PATCH_IF (SOURCE_SIZE > 0x71) THEN BEGIN
    SPRINT item_name "%SOURCE_RES%"
    END
    PATCH_IF ("%row_0%" STRING_EQUAL_CASE "%item_name%") THEN BEGIN
    SET new_price = "%row_1%"
    END
    PATCH_IF ("%new_price%" > "-1") THEN BEGIN 
    WRITE_LONG 0x34 ("%new_price%") 
    END
  BUT_ONLY
END

I'm guessing I'm missing something that defines what "row_0" should be before it goes to copy since it doesn't "see" anything.

@jmerry I tried incorporating and adapting the code from your first post in various ways but it didn't get me anywhere, and the WeiDU docs ain't helping. 

I'm just sort of guessing at this point. 

[jmerry]

Honestly, I don't know that much about two-dimensional arrays - I just know the one setup that I used in the snippet from my mod, with the array and two arguments. Copying a row at a time and then pulling elements of that row - not something I'm sure about. And so, I'm not surprised when the version I posted didn't work.

Oh, wait. I'm pretty sure that should be PHP_EACH rather than FOR_EACH. It's an array we're working with here, not a simple list...

Alternate form, using the exact structure I'm familiar with:

Spoiler

OUTER_FOR (row = 0; row < pricelist; ++row) BEGIN
    OUTER_SPRINT FileID $pricelist(~%row%~ ~0~) // First entry of the row
    COPY_EXISTING ~%FileID%.itm~ ~override~
        PATCH_IF (SOURCE_SIZE > 0x71) BEGIN // Sanity check
            SET newprice = $pricelist(~%row%~ ~1~)
            PATCH_IF (newprice > -1) BEGIN
                WRITE_LONG 0x34 newprice
            END // Another sanity check? I don't think you really need all of them.
        END
    BUT_ONLY IF_EXISTS // OK, I added one more sanity check here. No copying files that don't exist.
END

 

 

[WanderingScholar]

26 minutes ago, jmerry said:

Honestly, I don't know that much about two-dimensional arrays - I just know the one setup that I used in the snippet from my mod, with the array and two arguments. Copying a row at a time and then pulling elements of that row - not something I'm sure about. And so, I'm not surprised when the version I posted didn't work.

Oh, wait. I'm pretty sure that should be PHP_EACH rather than FOR_EACH. It's an array we're working with here, not a simple list...

Alternate form, using the exact structure I'm familiar with:

That did the trick finally. It just needed some quotes around the -1 in "PATCH_IF (newprice > -1)" Not sure what I was missing the first time I tried it.

ACTION_PHP_EACH was a no go with the other one.