Unity-Technologies / InputSystem

An efficient and versatile input system for Unity.
Other
1.43k stars 310 forks source link

DRAFT - NEW: Add setting for logging processed input events #1921

Closed jfreire-unity closed 5 months ago

jfreire-unity commented 5 months ago

Description

This PR adds a setting to improve debugging of received device events on the Input System.

It helps track down devices that might be sending a lot of events per frame. It can also help when looking for issues with the Input System.

The idea came while trying to fix https://unity3d.atlassian.net/servicedesk/customer/portal/2/IN-61472. The customer needed more information to track down a problem with InputSettings.maxEventBytesPerUpdate being exceeded in certain situations. Our engine support engineers had to go back and forth to help the customer add logging of events to their Input System package.

This PR pretends to improve that and fulfill the customer request to have more logging information about which device is causing the problem.

Changes made

Added a new setting to InputSettings.

When enabled, the Console output will be "polluted" with data like this (Player build logging example). image

Notes

📣 This is a very naive solution to avoid making customers have to add this kind of logs manually to InputManager. It is not "super elegant", but it works. I did not find a better location for this: I thought about InputMetrics or InputDiagnostics, but they all depend on the Input Debugger window which might not be ideal as it lacks saving history AFAIK.

With this PR I'm mostly looking for comments on this solution. The goal is to improve observability when we have problems in the Input System and allow for developers to have more information about input devices event processing.

Please write down any additional notes, remove the section if not applicable.

Checklist

Before review:

During merge:

stefanunity commented 5 months ago

So could the customer have solved their problem with access to this information? They could at least then just not use the offending device?

jfreire-unity commented 5 months ago

So could the customer have solved their problem with access to this information? They could at least then just not use the offending device?

Likely. It would help them understand their issue better since they are using manual updates.

jfreire-unity commented 5 months ago

After only a quick glance for me it is unclear: What is the EventSize and what is Bytes Processed? Is the event size included into the processed bytes? I am just thinking about the info the user should take from that and if there is possibly a better naming.

EventSize is the amount of data of a single event to be processed. Bytes processed is the sum of all the EventSize's processed during InputManager.onUpdate().

So maybe these names would have to be changed.

jfreire-unity commented 5 months ago

I'm closing this PR and moving into something that provides more data to the error message instead.