Conversation
- also auto-selects one upgrade, and can now confirm with spacebar on keyboard too
|
@sentry review |
| bool shouldShowArrow = Application.platform == RuntimePlatform.Android || | ||
| Application.platform == RuntimePlatform.IPhonePlayer || | ||
| Gamepad.current != null; | ||
|
|
||
| gameObject.SetActive(shouldShowArrow); |
There was a problem hiding this comment.
Bug: The arrow's visibility is set once in Awake() based on gamepad presence and never updates, causing the UI to be missing if a controller is connected mid-game.
Severity: MEDIUM
Suggested Fix
To ensure the arrow's visibility reflects the current input device, subscribe to the InputSystem.onDeviceChange event. In the event handler, check for gamepad presence and update the arrow's active state accordingly. Remember to unsubscribe from the event in OnDestroy() to prevent memory leaks. Alternatively, move the check into the Update() method for continuous polling.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: Assets/Scripts/Characters/Arrow.cs#L16-L20
Potential issue: The arrow's visibility is determined by checking `Gamepad.current !=
null` only once within the `Awake()` method and is never updated again. If a player
connects a gamepad after the scene has loaded, the arrow UI indicator will remain
hidden, even though other game systems correctly switch to gamepad input. This creates
an inconsistent user experience where the aiming system recognizes the controller, but
the associated visual aid does not appear.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
true, also thought about this while adding the one-time-check; but since we're gonna run the demo on handheld consoles, if the controller disconnects that must mean the console itself was destroyed.. IMO this is not something we need to fix right away, but can be opened as a new issue to show we're aware of this.
Adds support for controllers:
Also fixes the following:
TODO