two players can play on one keyboard by pro2p · Pull Request #52 · CStudioCS/AutomathonMVC

pro2p · 2026-05-06T20:46:41Z

A few other bugs were fixed

thomasbringer-madbox

MVC is respected which is #1 priority.

Besides that, some relatively small things that can be easily refactored (use AI).

My main concern of the PR is actually not code, it's design: even for debugging, does it make sense to play a top down shooter where you can't aim? This should be tested in editor, but i have doubts about it. But i let you and @Fypur be the judges for that (just raising awareness) my main focus is code review

thomasbringer-madbox · 2026-05-12T23:09:23Z

+                Vector3 worldPos = Camera.main.ScreenToWorldPoint(new Vector3(aimingDir.x, aimingDir.y, WorldConstants.CAMERA_DISTANCE / 1000));
+                return new Vector2Int((int)(worldPos.x * WorldConstants.SPACE_SCALE), (int)(worldPos.y * WorldConstants.SPACE_SCALE));


Both of those calculations look like they could be outsourced (not sure to where exactly, method, class, extension method?) I know it's View -> Controller, so it should go in view specific probably, but it just feels a little wrong to have this heavy calculation in the middle of the Input Provider script.

thomasbringer-madbox · 2026-05-12T23:22:27Z

+                    bool isLeftPlayer = Keyboard.current.escapeKey.wasPressedThisFrame;
+                    PlayerInput playerInputToRemove = null;
+
+                    foreach (PlayerInput playerInput in inputProviders.Keys)
                    {
-                        playerInputToRemove = playerInput;
-                        break;
+                        if ((isLeftPlayer && playerInput.currentControlScheme == "Keyboard_left") || (!isLeftPlayer && playerInput.currentControlScheme == "Keyboard_right"))
+                        {
+                            playerInputToRemove = playerInput;
+                            break;
+                        }
                    }


This is still very painful to read, especially that if statement. I'm sure there are more elegant ways to write it. Consider using a Left/Right enum.

This is a slight improvement ChatGPT suggested (there could be better ways though, for instance with a left/right enum)

bool isLeftPlayer = Keyboard.current.escapeKey.wasPressedThisFrame; string targetScheme = isLeftPlayer ? "Keyboard_left" : "Keyboard_right"; PlayerInput playerInputToRemove = null; foreach (PlayerInput playerInput in inputProviders.Keys) { if (playerInput.currentControlScheme != targetScheme) continue; playerInputToRemove = playerInput; break; }

thomasbringer-madbox · 2026-05-12T23:24:22Z

        }

-        private void JoinKeyboardPlayer()
+        private void JoinKeyboardPlayer(bool isLeft)


Using booleans for keyboard players (left/right) is hard to read and maintain. Consider using a Left/Right enum.

thomasbringer-madbox · 2026-05-12T23:24:52Z

            foreach (PlayerInput playerInputTemp in inputProviders.Keys)
            {
-                if (playerInputTemp.currentControlScheme == "Keyboard&Mouse")
+                if ((isLeft && playerInputTemp.currentControlScheme == "Keyboard_left") || (!isLeft && playerInputTemp.currentControlScheme == "Keyboard_right"))


This still looks bad and looks almost duplicate to the logic from before, consider refactoring it into a single method

thomasbringer-madbox · 2026-05-12T23:27:21Z

            Vector2Int directionInput = InputProvider.GetMilliAimingDir();
+            if (InputProvider is PlayerInputProvider playerInputProvider && playerInputProvider.PlayerControls == PlayerInputProvider.playerControlsType.Right)
+            {
+                directionInput = new Vector2Int(directionInput.X - Position.X, directionInput.Y - Position.Y);


From a design point of view, I sure wouldn't want to play a top-down shooter... where you can't aim.

thomasbringer-madbox · 2026-05-12T23:29:26Z

    public static class WorldConstants
    {
        public const int SPACE_SCALE = 1000;
+        public const int CAMERA_DISTANCE = 10000;


I think this should be removed, see this comment

Copilot

Pull request overview

Adds support for two players sharing one keyboard by splitting bindings into left/right keyboard control schemes, and updates gameplay input handling to support mouse-based aiming for the right-side player.

Changes:

Split the former Keyboard&Mouse scheme into Keyboard_left and Keyboard_right with updated bindings (movement/aim/shoot/dash/shield/grenade).
Updated lobby join/leave logic to allow joining either keyboard side based on key presses.
Adjusted aiming/rotation logic and introduced a world camera-distance constant to convert mouse screen position to world position.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
Assets/Game/InputActions.inputactions	Defines new `Keyboard_left` / `Keyboard_right` control schemes and rebinding for split-keyboard play.
Assets/Game/InputActions.cs	Regenerated wrapper for the updated input actions asset and new control scheme accessors.
Assets/Game/Features/World/WorldConstants.cs	Adds a camera-distance constant used for mouse-to-world conversion.
Assets/Game/Features/Tank/Tank.cs	Updates aiming direction handling and tank rotation logic.
Assets/Game/Features/Shield/ShieldAbility.cs	Aligns shield rotation with the tank’s last aim direction rather than tank rotation.
Assets/Game/Features/Lobby/TankSpawnerView.cs	Updates join/leave detection to support left/right keyboard players and fixes repeated subscription by moving it to Awake.
Assets/Game/Features/Input/PlayerInputProvider.cs	Adds left/right keyboard control scheme identification and mouse-position aiming conversion for the right scheme.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

thomasbringer-madbox · 2026-05-12T23:42:18Z

            rigidbody.Velocity = movementInput * SPEED / 1000;

            Vector2Int directionInput = InputProvider.GetMilliAimingDir();
+            if (InputProvider is PlayerInputProvider playerInputProvider && playerInputProvider.PlayerControls == PlayerInputProvider.playerControlsType.Right)


AH. Your code doesn't compile but you made the PR anyway? 😭

thomasbringer-madbox · 2026-05-12T23:43:30Z

+            else if (playerInput.currentControlScheme == "Keyboard_right")
+            {
+                PlayerControls = PlayerControlsType.Right;
+            }


I think this, we probably don't care, unless i'm missing something

thomasbringer-madbox · 2026-05-12T23:44:38Z

+            Vector2 aimingDir = aimAction.ReadValue<Vector2>();
+            if (PlayerControls == PlayerControlsType.Right)
            {
-                Vector2 aimingDir = aimAction.ReadValue<Vector2>();
-                return new Vector2Int((int)(aimingDir.x * 1000), (int)(aimingDir.y * 1000));
+                Vector3 worldPos = Camera.main.ScreenToWorldPoint(new Vector3(aimingDir.x, aimingDir.y, WorldConstants.CAMERA_DISTANCE / 1000));
+                return new Vector2Int((int)(worldPos.x * WorldConstants.SPACE_SCALE), (int)(worldPos.y * WorldConstants.SPACE_SCALE));
            }


Yes, this is this issue - i believe it's out of scope for this PR, i suggest we fix in upcoming PR.

thomasbringer-madbox · 2026-05-12T23:45:10Z

+        {
+            playerInputManager.onPlayerLeft += OnPlayerLeft;
+        }
+


Yes, correct I believe

thomasbringer-madbox · 2026-05-12T23:46:27Z

-                if (playerInputToRemove != null)
-                    OnPlayerLeft(playerInputToRemove);
+                    if (playerInputToRemove != null)
+                        OnPlayerLeft(playerInputToRemove);


A potential issue yes, kinda linked to this comment

… acces

two players can play on one keyboard

ffc73f0

A few other bugs were fixed

pro2p requested review from Fypur and thomasbringer-madbox May 6, 2026 20:46

pro2p added 4 commits May 6, 2026 23:53

fix

2769b3d

fix

931f39b

fix

9fe62a6

fiiiix

80c3a5c

thomasbringer-madbox requested changes May 12, 2026

View reviewed changes

thomasbringer-madbox mentioned this pull request May 12, 2026

Camera.main is a crime against humanity #54

Open

thomasbringer-madbox requested a review from Copilot May 12, 2026 23:34

Copilot started reviewing on behalf of thomasbringer-madbox May 12, 2026 23:35 View session

Copilot AI reviewed May 12, 2026

View reviewed changes

outsource view-related calculations and replace if-else by dictionary…

1bcd70b

… acces

		Vector3 worldPos = Camera.main.ScreenToWorldPoint(new Vector3(aimingDir.x, aimingDir.y, WorldConstants.CAMERA_DISTANCE / 1000));
		return new Vector2Int((int)(worldPos.x * WorldConstants.SPACE_SCALE), (int)(worldPos.y * WorldConstants.SPACE_SCALE));

Conversation

pro2p commented May 6, 2026

Uh oh!

thomasbringer-madbox left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants