Skip to content

Controller backend improvements #4552

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Controller backend improvements #4552

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions src/BizHawk.Emulation.Common/Base Implementations/ControllerDefinition.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class ControllerDefinition

private IReadOnlyList<IReadOnlyList<(string, AxisSpec?)>>? _orderedControls;

/// <summary>starts with console buttons, then each player's buttons individually</summary>
/// <summary>Starts with console buttons, then each player's buttons individually</summary>
public IReadOnlyList<IReadOnlyList<(string Name, AxisSpec? AxisSpec)>> ControlsOrdered
{
get
Expand All @@ -35,6 +35,11 @@ public class ControllerDefinition
public readonly string Name;

private Dictionary<string, char>? _mnemonicsCache;

/// <summary>
/// A mapping between buttons names and their Bk2 mnemonics.
/// (it's only relevant for buttons, not axes)
/// </summary>
public IReadOnlyDictionary<string, char>? MnemonicsCache => _mnemonicsCache;

/// <remarks>
Expand Down Expand Up @@ -113,7 +118,7 @@ private void AssertMutable()
if (!_mutable) throw new InvalidOperationException(ERR_MSG);
}

/// <remarks>implementors should include empty lists for empty players, including "player 0", to match this base implementation</remarks>
/// <remarks>Implementors should include empty lists for empty players, including "player 0" (console buttons), to match this base implementation</remarks>
protected virtual IReadOnlyList<IReadOnlyList<(string Name, AxisSpec? AxisSpec)>> GenOrderedControls()
{
var ret = new List<(string, AxisSpec?)>[PlayerCount + 1];
Expand All @@ -123,7 +128,7 @@ private void AssertMutable()
return ret;
}

/// <summary>permanently disables the ability to mutate this instance; returns this reference</summary>
/// <summary>Permanently disables the ability to mutate this instance; returns this reference</summary>
public ControllerDefinition MakeImmutable()
{
BoolButtons = BoolButtons.ToImmutableList();
Expand All @@ -134,6 +139,12 @@ public ControllerDefinition MakeImmutable()
return this;
}

/// <summary>
/// Get the player number associated with a control (button, analog axis, etc.).
/// Returns 0 for general console buttons not associated with a particular player's control port.
/// (for example, returns 0 for the Power button on NES)
/// For some consoles like (non-linked) Game Boy, this always returns 0.
/// </summary>
public static int PlayerNumber(string buttonName)
{
var match = PlayerRegex.Match(buttonName);
Expand All @@ -144,6 +155,11 @@ public static int PlayerNumber(string buttonName)

private static readonly Regex PlayerRegex = new Regex("^P(\\d+) ");

/// <summary>
/// Returns the number of players.
/// Currently only returns 0 for consoles where all control ports are empty,
/// so returns 1 for (non-linked) Game Boy and similar cases.
/// </summary>
public int PlayerCount
Copy link
Member

@YoshiRulz YoshiRulz Nov 22, 2025

Choose a reason for hiding this comment

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

Where is this used? Would it not be better to change the semantics such that player 0 counts as a player (that is, PlayerCount is usually 1 more than the number of plugged-in gamepads)?

Copy link
Contributor Author

@RetroEdit RetroEdit Nov 22, 2025

Choose a reason for hiding this comment

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

I'll look and consider it. I'm not sure I entirely like the semantics of treating the console itself as a "player" though. At that point, I would also question why one couldn't just get the length of ControlsOrdered directly.

Copy link
Collaborator

@Morilli Morilli Nov 23, 2025

Choose a reason for hiding this comment

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

Well ControlsOrdered relies on PlayerCount to know how many control groups there are. I do think documenting the special behavior is not useful when it should be changed instead.

{
get
Expand Down