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

feat: distributed authority sample/expanded controls [MTT-9277] #256

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

fernando-cortez
Copy link
Collaborator

@fernando-cortez fernando-cortez commented Nov 15, 2024

Description

This PR adds the skeleton mobile controls UI to the project. Mobile controls can be debugged inside the editor by toggling the following field inside the Bootstrap scene:
Screenshot 2024-11-15 at 16 48 12

A result of this bootstrapping mobile work is that there's a wrapper between the UI and the project's inputs. So, at a high level, UI captures the value of a joystick or a button and routes them to the associated input system action. The result of this is some wrapper Input classes were removed or deprecated, as the game should fetch the input data from the new GameInput class, instead of referencing actions independently. The reason for this is that InputSystemManager now handles what platform the game is on and subsequently what control scheme to be on. Thus, reading from GameInput will track the currently enabled actions.

Note: having a gamepad connected while debugging mobile controls will override both the virtual joystick inputs, so make sure to test gamepad when outside mobile debug mode.

Issue Number(s)

MTT-9277

Contribution checklist

  • [ N/A ] Tests have been added for the project and/or any internal package
  • Release notes have been added to the project changelog file
  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • JIRA ticket ID is in the PR title or at least one commit message
  • Include the ticket ID number within the body message of the PR to create a hyperlink

@fernando-cortez fernando-cortez changed the title feat: distributed authority sample/expanded controls [MTT-9278] feat: distributed authority sample/expanded controls [MTT-9277] Nov 15, 2024
@fernando-cortez fernando-cortez marked this pull request as ready for review November 18, 2024 20:47
@fernando-cortez fernando-cortez requested a review from a team as a code owner November 18, 2024 20:47
{ nameof(MobileGamepadState.ButtonJump), MapRuntimeControl(m_JumpAction) },
{ nameof(MobileGamepadState.ButtonInteract), MapRuntimeControl(m_InteractAction) },
{ nameof(MobileGamepadState.ButtonSprint), MapRuntimeControl(m_SprintAction) },
// will re-visit to evaluate if this button is necessary
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you want to revisit it in a later PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we'll revisit if we need this button in the UI icons PR.

public event Action<string, Vector2> JoystickStateChanged;

/// <summary>
/// This method let the UI updates when a property bound with <see cref="BindingMode.ToTarget"/> is calling it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"lets the UI update"

}

/// <summary>
/// This method lets the Input system updates when a property value is changed.
Copy link
Contributor

@Elfi0Kuhndorf Elfi0Kuhndorf Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here and below

/// The UI is bound to <see cref="RightJoystickTop"/> and <see cref="RightJoystickLeft"/>
/// which converts the Vector2 position into a percent <see cref="StyleLength"/>.
/// <para>The <see cref="TouchScreenBehaviour"/> is reading the UI pointer
/// to directly write the delta in this property, which in returns updates the VisualElement position.</para>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo returns

@Elfi0Kuhndorf
Copy link
Contributor

The controller controls work well!

With the mobile ones I have some issues:

  • The jumping is very hard to execute when walking
  • The buttons are too small it is hard to execute them/ not comfortable
  • The interact button was for me in the right upper corner, which is not a good position to execute it, it should be close to the jump button

@fernando-cortez
Copy link
Collaborator Author

The controller controls work well!

With the mobile ones I have some issues:

  • The jumping is very hard to execute when walking
  • The buttons are too small it is hard to execute them/ not comfortable
  • The interact button was for me in the right upper corner, which is not a good position to execute it, it should be close to the jump button

What was challenging about the jumping? Is it something a follow-up PR could address?
If every bullet here could be addressed in a follow-up PR with images/positioning, then I think this one can go through as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants