Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Atari 5200 and Nintendo 64 Game Controller Improvements #2367

Merged

Conversation

pabloarista
Copy link
Contributor

@pabloarista pabloarista commented Dec 29, 2024

User description

What does this PR do

Atari 5200:

  1. Game controller fix so the left thumbtack can be used by the user. Also added mapping for 1-8, * and # buttons
  2. Virtual controller fix to add functionality to the virtual thumbtack because currently it doesn't do anything

Nintendo 64

  1. Swapped Z and Start button to make them similar to the OG Nintendo 64 controller so the Z is on the left finger and the start is on the right finger.
  2. Also added a fallback on the 2 controller logic so if the user isn't using a dual sense controller and if the R3 button exists, so the start button works

Where should the reviewer start

All of this can only be tested via the game controllers (virtual and hardware), see next section.

How should this be manually tested

Atari 5200:

  1. To test the 1-8 buttons, I used star raiders, that one uses all of those buttons for the speed (at least for the first part of the game). To test the * and # I used Gremlins, that is to set 1 or 2 players and which night to use respectively.
  2. To test the virtual stick, just use any game.

Nintendo 64

  1. You can test any game that has the Z button.

Any background context you want to provide

I just found that these were lacking while working with some games.

What are the relevant tickets

There are no relevant tickets. Actually not sure if I need to create one and link it here. I can do that if needed.

Screenshots (important for UI changes)

There are no UI changes, just fixing functionality.

Questions

No question, but if I'm missing anything just let me know.


PR Type

Enhancement


Description

Atari 5200 Controller Improvements:

  • Added left thumbstick support as an alternative to D-pad for movement
  • Implemented mapping for buttons 1-8, * and # for expanded game compatibility
  • Fixed virtual controller joystick functionality
  • Simplified fire button controls

Nintendo 64 Controller Improvements:

  • Swapped Z and Start button positions to match original N64 controller layout (Z on left finger, Start on right)
  • Added fallback support for Start button on non-DualSense controllers with R3 button

Changes walkthrough 📝

Relevant files
Enhancement
PVAtari5200ControllerViewController.swift
Refactor Atari 5200 virtual controller joystick handling 

PVUI/Sources/PVUIBase/Controller/Systems/PVAtari5200ControllerViewController.swift

  • Refactored joystick handling logic from analog to digital input
  • Replaced didMoveJoystick calls with didPush/didRelease for directional
    controls
  • +12/-15 
    PVAtari800Bridge.m
    Add extended button mapping for Atari 5200 controller       

    Cores/Atari800/Sources/PVAtari800Bridge/PVAtari800Bridge.m

  • Added left thumbstick support as alternative to D-pad
  • Mapped additional buttons (1-8, *, #) to various controller inputs
  • Simplified fire button mappings
  • +38/-7   
    PVMupenBridge+Controls.m
    Improve Nintendo 64 controller button layout                         

    Cores/Mupen64Plus/Sources/PVMupenBridge/PVMupenBridge+Controls.m

  • Swapped Z and Start button mappings for more natural N64 layout
  • Added fallback support for Start button on non-DualSense controllers
  • +7/-4     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    … alternate to the d-pad, also added mapping for 1-8,* and # buttons
    
    Fixed Atari 5200 virtual game controller missing functionality of the joystick
    Nintendo 64 swapped the start and z buttons so it's more natural as the OG nintendo 64 controller (z left fingher, start right finger)
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Input Validation

    The joystick value bounds are no longer validated after removing min/max checks. This could potentially lead to invalid input values being passed to the emulator.

    var up:CGFloat = value.y < 0.5 ? CGFloat(1 - (value.y * 2)) : 0.0
    var down:CGFloat = value.y > 0.5 ? CGFloat((value.y - 0.5) * 2) : 0.0
    var left:CGFloat = value.x < 0.5 ? CGFloat(1 - (value.x * 2)) : 0.0
    var right:CGFloat = value.x > 0.5 ? CGFloat((value.x - 0.5) * 2) : 0.0
    Button Conflicts

    Multiple buttons mapped to the same input could cause conflicts if pressed simultaneously. Consider adding logic to handle button priority.

    } else if (gamepad.rightThumbstick.left.isPressed) {
        //1 button
        INPUT_key_code = AKEY_5200_1;
    } else if (gamepad.rightThumbstick.up.isPressed) {
        //2 button
        INPUT_key_code = AKEY_5200_2;
    } else if (gamepad.rightThumbstick.right.isPressed) {
        //3 button
        INPUT_key_code = AKEY_5200_3;
    } else if (gamepad.rightThumbstick.down.isPressed) {
        //4 button
        INPUT_key_code = AKEY_5200_4;
    } else if (gamepad.leftThumbstickButton.isPressed) {
        //5 button
        INPUT_key_code = AKEY_5200_5;
    } else if (gamepad.rightThumbstickButton.isPressed) {
        //6 button
        INPUT_key_code = AKEY_5200_6;
    } else if (gamepad.buttonX.isPressed) {
        //7 button
        INPUT_key_code = AKEY_5200_7;
    } else if (gamepad.buttonY.isPressed) {
        //8 button
        INPUT_key_code = AKEY_5200_8;

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add value clamping to prevent potential overflow issues with joystick input calculations

    Add bounds checking to ensure joystick values are clamped between 0.0 and 1.0 before
    using them, similar to the removed code. Without this, extreme joystick positions
    could cause undefined behavior.

    PVUI/Sources/PVUIBase/Controller/Systems/PVAtari5200ControllerViewController.swift [41-44]

    -var up:CGFloat = value.y < 0.5 ? CGFloat(1 - (value.y * 2)) : 0.0
    -var down:CGFloat = value.y > 0.5 ? CGFloat((value.y - 0.5) * 2) : 0.0
    -var left:CGFloat = value.x < 0.5 ? CGFloat(1 - (value.x * 2)) : 0.0
    -var right:CGFloat = value.x > 0.5 ? CGFloat((value.x - 0.5) * 2) : 0.0
    +var up:CGFloat = value.y < 0.5 ? min(CGFloat(1 - (value.y * 2)), 1.0) : 0.0
    +var down:CGFloat = value.y > 0.5 ? min(CGFloat((value.y - 0.5) * 2), 1.0) : 0.0
    +var left:CGFloat = value.x < 0.5 ? min(CGFloat(1 - (value.x * 2)), 1.0) : 0.0
    +var right:CGFloat = value.x > 0.5 ? min(CGFloat((value.x - 0.5) * 2), 1.0) : 0.0
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion addresses a critical issue where removing the min/max bounds checking could lead to invalid joystick values and potential gameplay issues. This is particularly important since the code handles core controller functionality.

    8
    Add runtime safety check to prevent crashes when accessing potentially unsupported controller features

    Add a null check before accessing gamepad.rightThumbstickButton.isPressed to prevent
    potential crashes on controllers that don't support this feature.

    Cores/Mupen64Plus/Sources/PVMupenBridge/PVMupenBridge+Controls.m [238-240]

    -} else if (gamepad.rightThumbstickButton != nil) {
    +} else if (gamepad.rightThumbstickButton != nil && [gamepad respondsToSelector:@selector(rightThumbstickButton)]) {
         //fallback for non-dual sense only if the R3 button exists on the controller
         padData[playerIndex][PVN64ButtonStart] = gamepad.rightThumbstickButton.isPressed;
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion adds an important runtime safety check that could prevent app crashes when dealing with different controller types, though the existing nil check already provides some protection.

    7

    @JoeMatt JoeMatt merged commit 57fe03b into Provenance-Emu:develop Dec 30, 2024
    2 of 8 checks passed
    @pabloarista pabloarista deleted the feature/game-controller-fixes branch January 1, 2025 21:51
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants