Allow ADD_STORE_ITEM to set item sale triggers in STO V1.1 resources

[Argent77]

The STO V1.1 format used by PST and PST:EE supports triggers to determine whether or when an item is available for sale. The trigger is set as a strref.

It should be possible to set item sale triggers with ADD_STORE_ITEM.

[FredrikLindgren]

The TP2 format does not handle optional arguments well, especially not since ADD_STORE_ITEM already has an optional argument. I'd rather write a function and deprecate the action.

[Argent77]

Is it possible to return the index or offset of the item that has been added by ADD_STORE_ITEM? It would allow modders to make further modifications to the item after the function call.

[FredrikLindgren]

It's possible but would also require an optional argument (in which to store the index/offset).

[Argent77]

Well, in this case a rewrite as a regular WeiDU function would indeed be the preferrable solution.

[Argent77]

I'd like to make an attempt to turn ADD_STORE_ITEM into a WeiDU function if you haven't started working on it yourself.

[FredrikLindgren]

I have, but I did not get very far before I started on returning arrays. That's almost done now, but feel free to go for it. Bear in mind the function should have the same features as the patch, in addition to the new ones.

[Argent77]

I have started working on it already. This is the current design:

/**
 * Adds an item to the current STO file. This is a PATCH function.
 * SET charge1          Number charges of the 1st ability or quantity for stackables.
 *                      (Default: 0)
 * SET charge2          Number charges of the 2nd ability. (Default: 0)
 * SET charge3          Number charges of the 3rd ability. (Default: 0)
 * SET stack            Number of item instances the store carries in the stack.
 *                      (Default: 1)
 * SET unlimited        Set to non-zero if the store should carry an inexhaustible stack 
 *                      of the new item. (Default: 0)
 * SET overwrite        Set to non-zero to overwrite any instance of an existing sale 
 *                      entry of matching item resref when found. (Default: 0)
 * SET expiration       The item's expiration time, when it will be replaced with the 
 *                      drained item. (Default: 0)
 * SPRINT item_name     The resource name (resref) of the item to add.
 * SPRINT position      The desired position of the item in the list of sale entries. 
 *                      The following syntax is supported:
 *                      AFTER resref    Will place the new item directly behind the item 
 *                                      given by "resref". You can list more than one 
 *                                      resref, separated by space. The new item will be 
 *                                      added after the entry of the first matching resref.
 *                      BEFORE resref   Will place the new item directly before the item 
 *                                      given by "resref". You can list more than one 
 *                                      resref, separated by space. The new item will be 
 *                                      added before the entry of the first matching resref.
 *                      LAST            Will place the new item after all existing items.
 *                      FIRST           Will place the new item before all existing items.
 *                      AT value        Will place the new item at the position given by 
 *                                      the number "value". Use negative values to place 
 *                                      the new item relative to the last item position in 
 *                                      reverse order.
 *                      (Default: FIRST)
 * SPRINT flags         Use numeric values or the following constants: none, identified, 
 *                      unstealable, stolen.
 *                      Constants can be combined by using ampersand (&) or space as 
 *                      separators (e.g. identified&unstealable). (Default: none)
 * SPRINT sale_trigger  Availability trigger (STO V1.1 only). The following syntax is 
 *                      supported:
 *                      Trigger string          Example: GlobalGT("MyCondition","GLOBAL",0)
 *                      Strref value            Example: #1234
 *                      Translation reference   Example: @1000
 *                      (Default: #-1)
 * RETURN index         Returns the sale entry index of the added item or the last matching 
 *                      sale entry index when overwriting items. Returns -1 if the item 
 *                      could not be added or updated.
 */
DEFINE_PATCH_FUNCTION ADD_ITEM_TO_STORE

It hopefully covers all documented and undocumented features of the original ADD_STORE_ITEM patch.

Question: Is it possible to let WeiDU do a syntax check for a given script trigger or sequence of script triggers? It might be useful for the sale_trigger parameter.

[FredrikLindgren]

Looks good. You requested it and can, of course, implement it however you want, but I was planning to return both index and offset. But if index alone is enough for you use case, never mind.

About script validation, I'll have to get back to you. WeiDU can do it (obviously), but I'd have to expose this in TP2, which may or may not be difficult.

[Argent77]

Sure, offset can be added as well.

The implementation is pretty much complete by now. I'll create a pull request when you have more details about script validation.

[FredrikLindgren]

Can we rename the function into ADD_STORE_ITEM? I think it becomes clearer that they are related and that one supersedes the other if they have the same name.

About script validation: I have two values which each take a string of whitespace-separated actions or triggers and return 1 if the string was parsed without exceptions, or 0 if there were exceptions. Recoverable errors (was expecting an int, too many arguments, etc) do not fail the check (that logic is internal to the parser and not something I can affect from outside). Is that something like what you have in mind?

[Argent77]

Can we rename the function into ADD_STORE_ITEM? I think it becomes clearer that they are related and that one supersedes the other if they have the same name.

I would like to name it ADD_STORE_ITEM, but WeiDU is overly sensitive when I do that. Names for both function definition and function call have to be wrapped in quotes, or WeiDU would fail with a compiler error. I tend to use function names without quotes and many other modders seem to do it too, so it would just cause unnecessary confusion (or frustration).

About script validation: I have two values which each take a string of whitespace-separated actions or triggers and return 1 if the string was parsed without exceptions, or 0 if there were exceptions. Recoverable errors (was expecting an int, too many arguments, etc) do not fail the check (that logic is internal to the parser and not something I can affect from outside). Is that something like what you have in mind?

Yes, I think that'll work. The games' own compilers seem to be quite resilient as well, so recoverable errors should be no problem.

[FredrikLindgren]

I would like to name it ADD_STORE_ITEM, but WeiDU is overly sensitive when I do that.

Right, I should know that. Well, either prefix it or never mind.

I'll try to push the new values later this week. I'll have to fix them up so they can take str- and trarefs, as well.

[FredrikLindgren]

Actually, array support is probably more broadly useful (and it's either one or the other, I'm afraid). It is, of course, possible to put the string of the str- or traref in a regular variable and use that. Pushed.

[Argent77]

The trigger check seems to work fine (using simple string argument). What action would be more sensible in your opinion when the check returned false? Merely display a warning message and proceed, default trigger to empty string (plus warning), or cancel the whole "add store item" operation?

Btw, I'll probably call the function ADD_STORE_ITEM_EX. I don't think prefixed function names are appropriate to be shipped with WeiDU.

[FredrikLindgren]

Empty string and WARN, I think.

[Argent77]

While I'm at it, I will also implement the functions ADD_STORE_DRINK, ADD_STORE_CURE and ADD_STORE_CATEGORY, and possibly their removal counterparts. They will all follow the general design of ADD_STORE_ITEM_EX.

[Argent77]

This is the item removal function signature:

/**
 * Removes one or more items from the current STO file. This is a patch function.
 * SPRINT item_name     The resource name (resref) of the item to remove. Set "item_name" 
 *                      directly to remove a single item. Set "item_name_index", where 
 *                      "index" is a numeric value starting at 0, to remove multiple items 
 *                      at once.
 *                      Example:
 *                        item_name_0 = "RING01"
 *                        item_name_1 = "AMUL03"
 *                        item_name_2 = "PLAT07"
 *                      Note: The function prefers indexed item names over a single item 
 *                            name. To remove a single item name, make sure that item_name_0 
 *                            is either undefined or empty.
 * RETURN result        Number of removed sale entries. Returns -1 on error.
 */
DEFINE_PATCH_FUNCTION REMOVE_STORE_ITEM_EX

The drink, cure and purchase removal functions will follow the same design.

[FredrikLindgren]

I don't know, with the awkward interface, the benefit of the removal function over the patch seems iffy. Maybe it's better to not creep the scope? You could also change it to take a single item and leave the lists to other language features, like *_FOR_EACH.

[Argent77]

You are right for REMOVE_STORE_ITEM_EX, where the patch variant is much easier to use. However, for drinks, cures and purchases there are no such functions available. I will add the item removal function just for the sake of completeness.

To make the removal functions more appealing I could add more potential return values. It might be useful to get index and offset of the removed entry (e.g. if you plan to replace it in a subsequent operation).

Leaving out the array feature is probably a good idea. It's not only tedious to define array arguments, it can also be a source for subtle bugs, e.g. if there are (leftover) indexed array variables defined that are not intended to be part of the array.

[FredrikLindgren]

Current status: I need add the documentation to the readme.