Games: Add Player Viewer (aka "Player Manager Light") #23548

garbear · 2023-07-24T22:59:42Z

Description

This PR a new window that I call the "Player Viewer". I basically stripped the in-progress Player Manager of, well, management.

However, I ended up with a still relatively useful and not-terrible new feature.

Even though you can't fine-tune control over player assignments yet, you can still change the emulator's ports, which will intelligently re-assign players after #23482.

And you can finally change controller settings like appearance and deadzones from within the game, instead of having to hunt down the heavily buried Input settings.

Plus, all controllers have the rad highlighting added in #23521. Because you can see active controllers, this enables a low-tech kind of player management called "just hand the controller to the other player".

Review should be similar to the port dialog (#20505). To manage a hefty PR I made every effort to have a safe diff with only lines added and very few logical lines changed.

Motivation and context

I was hesitant to PR the window because it's not fully complete. Like, controller settings are per-driver, not per-controller, which leaves something to be desired.

My monthly allocation of Kodi time is drawing to a close though, so it's either PR part of my vision, or let it linger for yet more years. And despite my hesitation, I've gotten a lot of positive feedback for even the incomplete version, so I'll share it in the current state and ship a cool new feature in v21.

As they say, perfect is the enemy of good.

How has this been tested?

Included in test builds for the last few weeks: https://github.com/garbear/xbmc/releases

Screenshots (if appropriate):

Added a "Players" item to the main in-game OSD:

Player Viewer showing a button being pressed on a controller:

Existing Port Dialog when you select the "ports" button:

Existing Peripheral Settings Dialog, opens when you select a controller

What is the effect on users?

Added new in-game Player Manager

Types of change

Bug fix (non-breaking change which fixes an issue)
Clean up (non-breaking change which removes non-working, unmaintained functionality)
Improvement (non-breaking change which improves existing functionality)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that will cause existing functionality to change)
Cosmetic change (non-breaking change that doesn't touch code)
Student submission (PR was done for educational purposes and will be treated as such)
None of the above (please explain below)

lrusak

Nice feature. I don't have much to comment. The code is nice and almost 100% new.

lrusak · 2023-07-28T02:37:51Z

xbmc/games/agents/GameAgentManager.cpp

+  {
+    if (agent->GetPeripheralLocation() == peripheralLocation)
+    {
+      activation = agent->GetActivation();


just return instead of assigning the temporary?

I like to avoid early returns so that there's a single path out of the function. If we returned early here, we would also have a return at the end of the function.

That said, I like the for-if-break pattern less, so I'll add the early return.

lrusak · 2023-07-28T02:43:28Z

xbmc/games/agents/windows/GUIAgentDefines.h

+#define CONTROL_AGENT_LIST 5
+
+// GUI button IDs
+#define CONTROL_AGENT_CLOSE_BUTTON 18


Can these defines be constexpr instead?

lrusak · 2023-07-28T02:49:29Z

xbmc/games/agents/windows/GUIAgentList.cpp

+      CGUIMessage msg(GUI_MSG_REFRESH_LIST, m_guiWindow.GetID(), CONTROL_AGENT_LIST);
+      CServiceBroker::GetAppMessenger()->SendGUIMessage(msg, m_guiWindow.GetID());
+    }
+    break;


break inside braces?

lrusak · 2023-07-28T02:55:02Z

xbmc/games/controllers/listproviders/GUIGameControllerProvider.cpp

+void CGUIGameControllerProvider::InitializeItems()
+{
+  m_items.resize(ITEM_COUNT);
+  for (unsigned int i = 0; i < ITEM_COUNT; ++i)


Suggested change

for (unsigned int i = 0; i < ITEM_COUNT; ++i)

for (auto item& : m_items)

fuzzard · 2023-07-28T03:18:56Z

xbmc/games/controllers/guicontrols/GUIGameControllerList.h

+#include <vector>
+
+class CScroller;
+class TiXmlElement;


Superfluous? im not seeing a need to forward declare this.

garbear · 2023-07-30T08:27:11Z

After build 5, CI errors still seem to be unrelated.

Ch1llb0 · 2024-04-28T15:14:19Z

Just implementing this into my skin and wondering whether I'm missing something, but differently than other color control tags (like the colordiffuse tag), controllerdiffuse doesn't seem to interpret variables here... When I input a color name or a color value, it works, but using a variable nothing happens, if I press a button of one of the listed input devices. Any ideas? 🤔

garbear · 2024-04-28T23:37:48Z

It looks like I didn't add support for skin variables or info labels in the controllerdiffuse field, only raw color values. Is there another diffuse property in the skin that allows for variable names that I can copy for controllerdiffuse?

garbear · 2024-04-28T23:44:25Z

I checked the code and controllerdiffuse uses the same code as colordiffuse, so it should support $var[] and $info[]. Are both of these broken for you?

Ch1llb0 · 2024-04-29T04:08:28Z

Will check with $INFO[] as well. It's working for the normal colordiffuse tags though...

garbear · 2024-05-01T00:23:20Z

Can you give an example of how you're using $VAR[]? (I haven't used vars before in skinning). I'll look into this when I get a chance.

Ch1llb0 · 2024-05-01T05:55:22Z

Sure... Here you can find a line where a VAR is used with a colordiffuse: https://github.com/osmc/skin.osmc/blob/bd340735b7b1762a98d5b0ccf9593ecbd3437b61/xml/AddonBrowser.xml#L48

This then points to my colour variables file that I created to store some colour sets toggled by skin settings: https://github.com/osmc/skin.osmc/blob/bd340735b7b1762a98d5b0ccf9593ecbd3437b61/xml/Variables_Colours.xml#L95

This works everywhere without issue, but in this new diffuse we have here 🙂

Ch1llb0 · 2024-05-05T09:00:10Z

I've pushed my initial Omega skin support now, so you can see the diffuse that doesn't work here now: https://github.com/osmc/skin.osmc/blob/f6641f8fec35294acbc63936466deadef2ce3d3b/xml/Coordinates_DialogGameControllers.xml#L835

All other diffuses in that very same file do work though 😊

garbear · 2024-05-06T04:12:27Z

I downloaded the skin and was able to reproduce. I've been short on Kodi time the last two months but when I get a chance I'll debug the problem.

Do you have a repo for the skin? Any skin with game support I'll include a repo for in my test builds, and if I personally have easy access to the skin I can test new RetroPlayer features in it before I release.

Ch1llb0 · 2024-05-06T04:17:41Z

Unfortunately, we don't have a repo for it - it's only released with OSMC updates and thus a repo wouldn't help us, I'm afraid 😕

garbear requested review from lrusak, VelocityRa, gusandrianos and NikosSiak July 24, 2023 22:59

garbear added Type: Feature non-breaking change which adds functionality Component: Input Component: Games v21 Omega labels Jul 24, 2023

garbear added this to the Omega 21.0 Alpha 3 milestone Jul 24, 2023

garbear force-pushed the player-viewer branch from 23433f4 to 16aca8d Compare July 24, 2023 23:50

xbmc deleted a comment from jenkins4kodi Jul 24, 2023

lrusak reviewed Jul 28, 2023

View reviewed changes

lrusak approved these changes Jul 28, 2023

View reviewed changes

fuzzard reviewed Jul 28, 2023

View reviewed changes

garbear closed this Jul 28, 2023

garbear reopened this Jul 28, 2023

Games: Add Agent Dialog

3c4f91e

garbear force-pushed the player-viewer branch from ef0139a to 3c4f91e Compare July 29, 2023 20:08

garbear merged commit 3f52a87 into xbmc:master Jul 30, 2023
1 of 2 checks passed

garbear deleted the player-viewer branch July 30, 2023 08:27

garbear mentioned this pull request Aug 11, 2023

Player Viewer: Two fixes #23610

Merged

8 tasks

garbear added the Component: Skin label Oct 22, 2023

scott967 mentioned this pull request Nov 1, 2023

Add window InGameSaves #24034

Closed

14 tasks

garbear mentioned this pull request Nov 8, 2023

[RetroPlayer] GameAgents dialog does not show players #24072

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Games: Add Player Viewer (aka "Player Manager Light") #23548

Games: Add Player Viewer (aka "Player Manager Light") #23548

garbear commented Jul 24, 2023 •

edited

lrusak left a comment

lrusak Jul 28, 2023

garbear Jul 28, 2023

lrusak Jul 28, 2023

lrusak Jul 28, 2023

lrusak Jul 28, 2023

garbear Jul 28, 2023

fuzzard Jul 28, 2023

garbear Jul 28, 2023

garbear commented Jul 30, 2023

Ch1llb0 commented Apr 28, 2024 •

edited

garbear commented Apr 28, 2024

garbear commented Apr 28, 2024

Ch1llb0 commented Apr 29, 2024 •

edited

garbear commented May 1, 2024

Ch1llb0 commented May 1, 2024

Ch1llb0 commented May 5, 2024

garbear commented May 6, 2024

Ch1llb0 commented May 6, 2024

	for (unsigned int i = 0; i < ITEM_COUNT; ++i)
	for (auto item& : m_items)

Games: Add Player Viewer (aka "Player Manager Light") #23548

Games: Add Player Viewer (aka "Player Manager Light") #23548

Conversation

garbear commented Jul 24, 2023 • edited

Description

Motivation and context

How has this been tested?

Screenshots (if appropriate):

What is the effect on users?

Types of change

lrusak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garbear commented Jul 30, 2023

Ch1llb0 commented Apr 28, 2024 • edited

garbear commented Apr 28, 2024

garbear commented Apr 28, 2024

Ch1llb0 commented Apr 29, 2024 • edited

garbear commented May 1, 2024

Ch1llb0 commented May 1, 2024

Ch1llb0 commented May 5, 2024

garbear commented May 6, 2024

Ch1llb0 commented May 6, 2024

garbear commented Jul 24, 2023 •

edited

Ch1llb0 commented Apr 28, 2024 •

edited

Ch1llb0 commented Apr 29, 2024 •

edited