Widespread 'filter' lua functions change api contract, causing incompatibilities

[Guest testerman 9876]

When writing 'filter' functions like this, it is necessary to make sure the interface of what is filtered remains the same as the original to make mod compatibility as robust as possible.

For example, https://github.com/Gibberlings3/TalentsOfFaerun/blob/341f6aed2988cb2671cef46c950c20dfd04f3428/dw_talents/sfo/lua/ui_detect_class_kit.tph#L718 , this line returns multiple values because gsub returns multiple values. This breaks mods that also try to similarly 'filter' the same line because it doesn't expect multiple return values. For example, a mod can 'filter' your 'filter' using tonumber, which behaves differently if there is a 2nd parameter.

In this specific case, to improve compatibility, it can be written:

local result = string.gsub(str,"{[^}]*}","")
return result

 

[DavidW]

Theoretically I agree. In practice I don't see any realistic scenario in which this would occur (the UI.menu modding setup is just not robust in general against multiple mods trying to do this kind of editing).

In this particular example, ToF adds a hidden ID to every kit name in order to allow it to be uniquely identified. The dwFilterKitDesc function is used when the UI wants to display a string, and it filters the {} out of the string. If some other mod is trying to do anything similar, they're just going to conflict.

[Guest testerman9876]

30 minutes ago, DavidW said:

Theoretically I agree. In practice I don't see any realistic scenario in which this would occur (the UI.menu modding setup is just not robust in general against multiple mods trying to do this kind of editing).

https://www.gibberlings3.net/forums/topic/37666-bgeebg2eeiwdee-gameplay-rebalance/ is a practical example where it occurs.

Quote

In this particular example, ToF adds a hidden ID to every kit name in order to allow it to be uniquely identified. The dwFilterKitDesc function is used when the UI wants to display a string, and it filters the {} out of the string. If some other mod is trying to do anything similar, they're just going to conflict.

That's perfectly fine. The issue is that Infinity_FindString (which returns 1 value) is wrapped by dwFilterKitDesc (which returns 2 values). That changes the contract. Instead of returning a single string, a string and a number are returned.

What I'm suggesting isn't to remove the filters, rather it is for the filters to have the same contract of what they wrap e.g. for dwFilterKitDesc to return 1 value like in the example code I provided in the first post. That would fix the conflict with the above linked mod and any others that would behave unexpectedly by changing the contract of Infinity_FindString, which dwFilterKitDesc filters for the purpose you described.

[Guest testerman9876]

1 hour ago, DavidW said:

(the UI.menu modding setup is just not robust in general against multiple mods trying to do this kind of editing)

As an aside, the decorator pattern can help with this issue you raise here. This will allow you to modify the result of Infinity_FetchString in a way more robust to other mods messing with ui.menu. Other mods can use this same pattern, resulting in multiple 'filters', if you will, without wrapping calls to Infinity_FetchString in ui.menu.
 

local dw_original_Infinity_FindString = Infinity_FetchString
Infinity_FetchString = function(a)

    -- Do stuff before
    print(a)

    local result = dw_original_Infinity_FindString(a)

    -- Do stuff after e.g. filtering
    print(result)

    return result
end

-- If you need to decorate variadic functions, or if you want to be extra robust to changes e.g. a future EE version adding more parameters or more return values:
local dw_original_Infinity_FindString = Infinity_FetchString
Infinity_FetchString = function(...)
    -- Do stuff before
    print(...)
    print(select(1, ...))

    local results = { dw_original_Infinity_FindString(...) }

    -- Do stuff after e.g. filtering
    print(table.unpack(results))
    print(results[1])

    return table.unpack(results)
end

 

[DavidW]

That's pretty interesting (if a bit above my current LUA pay grade). Will study when I get a chance!

[DavidW]

58 minutes ago, Guest testerman9876 said:

https://www.gibberlings3.net/forums/topic/37666-bgeebg2eeiwdee-gameplay-rebalance/ is a practical example where it occurs.

That's perfectly fine. The issue is that Infinity_FindString (which returns 1 value) is wrapped by dwFilterKitDesc (which returns 2 values). That changes the contract. Instead of returning a single string, a string and a number are returned.

What I'm suggesting isn't to remove the filters, rather it is for the filters to have the same contract of what they wrap e.g. for dwFilterKitDesc to return 1 value like in the example code I provided in the first post. That would fix the conflict with the above linked mod and any others that would behave unexpectedly by changing the contract of Infinity_FindString, which dwFilterKitDesc filters for the purpose you described.

I take the point; at any rate this is clearly good practice. Will address.