[1.1.76] Error when passing nil as event handler filter for custom inputs

[BiterUnion]

When passing `nil` as an event handler filter to `script.on_event` for custom inputs, an error "Filters can only be used when registering single events." is raised, e.g.:

Code: Select all

script.on_event('test-custom-input', function(event) end, nil)  -- raises error

Not passing `nil` as a filter, does not raise the error:

Code: Select all

script.on_event('test-custom-input', function(event) end)  -- doesn't raise error

I know that custom inputs don't support filters and you may have a reason for handling it this way, but I think passing `nil` should be handled the same as passing no filter. This would be beneficial for general event handling logic, e.g. when iterating over a dict mapping event names to filters and calling `script.on_event` for these pairs.

[curiosity]

This is all over the API. The number of arguments matters.

edit: for the described use it is easily circumvented as such:

Code: Select all

function register_event_handler(event, handler, event_filters)
    if event_filters then
        script.on_event(event, handler, event_filters)
    else
        script.on_event(event, handler)
    end
end

[BiterUnion]

edit: for the described use it is easily circumvented as such:
...

This would not work in case I want to set the filter to nil for events that do support filters.

There absolutely are work-arounds and this bug is nothing serious, I just think it would be cleaner to handle nil and no argument the same. At the very least the error message could be clearer.

[curiosity]

Actually, in this specific case it will work, because the nil is not meaningful. Passing nil for filters is the same as passing nothing.

But generally, IIRC, there are some meaningful nils out there, so your proposal can only be applied selectively. At which point I'd say that consistency is more valuable: better all API functions behave in the same, if unusual, way than only some of them, with no obvious pattern.

[BiterUnion]

Actually, in this specific case it will work, because the nil is not meaningful. Passing nil for filters is the same as passing nothing.

Sorry, you are right. I somehow thought omitting the nil would keep the previously set filter.

In the interest of consistency I agree that it should be kept this way. But the error message may be more clear?

[curiosity]

Yeah, if you are already passing it a single (if custom) event, the error message is just wrong.

[Rseding91]

Thanks for the report. I've fixed the error message for the next release.