Unity-Technologies / InputSystem

An efficient and versatile input system for Unity.
Other
1.42k stars 306 forks source link

FIX: Prefabs and missing default control scheme in the inspector view of playerinput (ISXB-818) #1932

Closed bmalrat closed 3 months ago

bmalrat commented 3 months ago

Description

Fixed Prefabs and missing default control scheme used by PlayerInput component are now correctly shown in the inspector ISXB-818. Before the change previously defined PlayerInput's default control scheme that doesn't exist exist any more are not shown and produce some error when entering in playmode. Disabling/enabling component silently remove the lost control scheme. Also Default Scheme, Auto-Switch,Default Map fields was not well displayed(no blue override color and no context actions) for prefabs.

Changes made

Changed the PlayerInput's inspector the show the missing default control scheme with a red background and a suffix. Capture d'écran 2024-05-21 093655 The missing one is always in second position after and others schemes are now sorted in the popup.

Changed the ways that Default Scheme, Auto-Switch,Default Map are displayed to support prefabs. Capture d'écran 2024-05-24 160723

Notes

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

Checklist

Before review:

During merge:

Pauliusd01 commented 3 months ago

This seems to mess with scheme order even when there are no missing schemes in the list (In the video I compare your PR with the current develop) https://github.com/Unity-Technologies/InputSystem/assets/54306142/2b904247-bee1-44cc-a3ae-3e43cbd17c9e

bmalrat commented 3 months ago

This seems to mess with scheme order even when there are no missing schemes in the list (In the video I compare your PR with the current develop) https://github.com/Unity-Technologies/InputSystem/assets/54306142/2b904247-bee1-44cc-a3ae-3e43cbd17c9e

I sorted them alphabetically like mentioned in that comment ////TODO: sort alphabetically I can remove the sort and/or the sort the scheme editor windows

Pauliusd01 commented 3 months ago

This seems to mess with scheme order even when there are no missing schemes in the list (In the video I compare your PR with the current develop) https://github.com/Unity-Technologies/InputSystem/assets/54306142/2b904247-bee1-44cc-a3ae-3e43cbd17c9e

I sorted them alphabetically like mentioned in that comment ////TODO: sort alphabetically I can remove the sort and/or the sort the scheme editor windows

In my opinion It should be the same list both between the player component and the actual actions window. So probably best to not do any extra sorting in the component at all, if a missing scheme is selected it will already be quite visible by the red highlight. But maybe Hakan thinks otherwise

bmalrat commented 3 months ago

This seems to mess with scheme order even when there are no missing schemes in the list (In the video I compare your PR with the current develop) https://github.com/Unity-Technologies/InputSystem/assets/54306142/2b904247-bee1-44cc-a3ae-3e43cbd17c9e

I sorted them alphabetically like mentioned in that comment ////TODO: sort alphabetically I can remove the sort and/or the sort the scheme editor windows

In my opinion It should be the same list both between the player component and the actual actions window. So probably best to not do any extra sorting in the component at all, if a missing scheme is selected it will already be quite visible by the red highlight. But maybe Hakan thinks otherwise

I removed the sort. If @ekcoh prefer with it I will revert the last change

ekcoh commented 3 months ago

I think we can skip sort. Consistency between different places representing the same thing makes sense.

Pauliusd01 commented 3 months ago

Sorry, had to change the status. I took a look at the original bug report and noticed that action map specific fields do not dirty the prefab is this not what the user had trouble with? Are we truly fixing the issue properly here?

Pauliusd01 commented 3 months ago

Notice how none of the action specific fields get the blue highlights next to them when they're changed, so if the user wants to override their prefab they can't. The only way is to go into prefab mode and change it there.

https://github.com/Unity-Technologies/InputSystem/assets/54306142/6b87efc1-b433-48b5-924c-20011d6d62bd

bmalrat commented 3 months ago

Sorry, had to change the status. I took a look at the original bug report and noticed that action map specific fields do not dirty the prefab is this not what the user had trouble with? Are we truly fixing the issue properly here?

thanks for the highlight