REPENTOGON icon indicating copy to clipboard operation
REPENTOGON copied to clipboard

Callbacks with Optional Param -1 run all callbacks

Open jsgnextortex opened this issue 1 year ago • 1 comments

Due to how vanilla callbacks logic works, callbacks that can feature -1 as a valid value in an optional parameter will run all callbacks on trigger. For example, in the case of items, tmtrainer items can be of id -1. The main culprit is this:

local defaultCallbackMeta = {
	__matchParams = function(a, b)
		return not a or not b or a == -1 or b == -1 or a == b
	end
}

Which I assume may still be an issue in vanilla isaac althought I dont think there are any callbacks there that can have a -1 value.

jsgnextortex avatar Feb 15 '24 23:02 jsgnextortex

This comment is very late, but a long while back I added a blacklist to main_ex.lua for callbacks that should treat -1 as a valid parameter, rather than run every single callback. This list currently includes:

ModCallbacks.MC_PRE_USE_ITEM ModCallbacks.MC_USE_ITEM ModCallbacks.MC_PRE_ADD_COLLECTIBLE ModCallbacks.MC_POST_ADD_COLLECTIBLE ModCallbacks.MC_POST_TRIGGER_COLLECTIBLE_REMOVED ModCallbacks.MC_PLAYER_GET_ACTIVE_MAX_CHARGE ModCallbacks.MC_PLAYER_GET_ACTIVE_MIN_USABLE_CHARGE

Basically, callbacks that use an item ID as their param, since -1 is the ID of the first tmtrainer item.

As some additional context for the issue, some vanilla callbacks that do not support optional params at all will explicitly execute with -1 as the param, presumably to ignore any attempts for mods to add callbacks with a param to say, MC_POST_UPDATE.

Anyway, with the blacklist we can circumvent the issue wherever it might matter. It's possible that more could be done here, but at the moment I don't think any more in-depth work is worth it. The functionality does serve a purpose for callbacks that explicitly don't support params, even if it may not be the ideal implementation for it.

ConnorForan avatar Sep 06 '24 22:09 ConnorForan