nsyshid: Emulate Skylander Portal

[deReeperJosh]

Carrying on from some of the work I have done on Dolphin and RPCS3, and thanks to #950 being merged, I am opening this PR up with the ability to emulate the Skylander Portal on Cemu.

Currently have tested Trap Team and Swap Force as working on my Mac, but hoping to get feedback and some testers on this PR too. Hoping to add Disney Infinity in the near future, but I don't currently have access to my Wii U so can't dump the necessary erreula to test my changes have actually worked there.

Would partly close #588

[Exzap]

Is this still being worked on or is it ready for review? Asking because some suggestions by @ssievert42 are still open or without comment.

[deReeperJosh]

Yes sorry, just been busy with work, will fix those comments this week hopefully

[deReeperJosh]

Just pushed up some changes, but still haven't implemented the find device across backends method, so will aim to do that before the end of the week.

[deReeperJosh]

All those requested changes have now been made, let me know if that's what you had anticipated it looking like @ssievert42

Otherwise, I am not convinced with the performance on the Vicarious Visions games (Swap Force and Superchargers), I've had varying degrees of success because at some point the game decides to stop sending commands to the portal, presumably because the emulated portal has replied too fast or too slow. Will keep investigating the perfect timing.

[deReeperJosh]

Had a playthrough of Swap Force and 10s might be the sweet spot for now, could be something that I can add as a custom option so that users can increase/decrease themselves to try to avoid the same issues that occur with a real portal, but for now the PR is good to go

[ssievert42]

I like it!
Just some minor nitpicks :)
About the timing issues: These kinds of problems tend to make me really nervous 😅. Do you think subtracting the time it takes to run the other stuff in Read and SetReport from the time that should be slept before we can return might help?
Otherwise a dirty workaround might be to add a button to detach and reattach the emulated portal ¯\_(ツ)_/¯

[deReeperJosh]

@ssievert42 more review work added - unfortunately without a proper core timing module there isn't much we can do besides sleep the thread for a bit, I have the done the same work on Dolphin and that framework (schedule the transfer to be completed at x amount of thread ticks rather than time based), but would need a major rework to Cemu's core. Even detaching/reattaching doesn't fix the issue, the game just decides to stop sending commands to the portal, go figure

[ssievert42]

Welp, that's certainly a nice thing to do of the game.
Putting aside the fact that I can't actually test this, because I don't own any Skylanders games, I've run out of things to complain about :)

LGTM 👍

[ElBread3]

Hello, I just did some testing with Skylanders Giants. Cemu consistently crashes, either when the Activision logo is about to stop spinning, at various random points at the middle and end of the Vicarious Visions logo, or (my most successful attempt) a crash on "Press A". Just for kicks I tested with vanilla Cemu and it booted all the way through.
I wonder if that's the timing issue with the emulated portal, but from previous talk it sounded like it didn't crash Cemu, as by then you wouldn't be able to "detach and reattach the emulated portal". Hope my testing helps, looking forward to seeing this feature in Cemu

[deReeperJosh]

@ElBread3 thanks for helping test, I don't have giants for the Wii U so can't confirm that but will see if I can get someone else to recreate the bug so I can try to fix it locally. I would have thought trap team would have the same issue but I guess not!

[ElBread3]

Tested with a debug executable today and did not crash, though loading takes too long to daily drive, of course. Planning on testing with Disney Infinity on your infinitybase branch later once I buy it

[deReeperJosh]

I ended up grabbing giants and I had no crashes - might be Windows only so I will try to build on a Windows machine to see if I can replicate. Have you tried any other Skylander games @ElBread3?

[deReeperJosh]

I have found that the crash happens when fmt::print is called, something that I still don't really understand but the error message is "Stack cookie instrumentation code detected a stack-trace buffer overrun." I have commented this out for now, but @ssievert42 if you could confirm if this is actually doing something valuable besides just being a debug statement let me know!

[deReeperJosh]

Just made the layout of the window look a bit nicer and even

[ElBread3]

OK I tested the print fix and Giants booted and is playable. To answer if I have other Skylanders games I also own Imaginators and Superchargers; The only other game that crashed when I did my first tests was Superchargers, crashing on the logo appearing, but it booted after a handful of attempts and still boots consistently. And Imaginators works fine.

[ssievert42]

@deReeperJosh That call to fmt::print can go 👍
(And shouldn't be there in the first place; If you look at the history of that _debugPrintHex function you'll notice that I managed to forget that call in there.)
But I don't understand what could cause that error message either. At least in the context of _debugPrintHex we shouldn't be overwriting parts of the stack ¯\_(ツ)_/¯
Looking at recent PRs, #977 fixes a crash on Windows caused by the same fmt::print call, so I guess it's just not a good idea to use fmt to print stuff in Cemu, when there is a perfectly good function called cemuLog_logDebug to log something instead.

[ETE-Design]

@deReeperJosh I have an Wii U + Disney Infinity - Anything I can help with to get Disney Infinity working on CEMU?

[deReeperJosh]

@ETE-Design I have a branch on my repo where I have my infinity base progress, there isn't much there yet (no loading/removing figures or anything) but it would be good if you could test getting in game, you need to dump the erreula from your console iirc to get past the menu

[Squall-Leonhart]

The portal audio is underflowing, leading to the issue reported in #1146

[deReeperJosh]

@Squall-Leonhart that is mostly an issue with the Windows HID transfers, afaik libusb doesn't have the same problem, would be ideal if windows users also had the option to use libusb in the future

[Squall-Leonhart]

Quite possibly, i can't recall what the barrier for using it on windows is, but building with -DENABLE_NSYSHID_WINDOWS_HID=OFF hasn't demonstrated any odd behaviors in lego dimensions to date, alas i don't have skylanders to verify if the portal sounds work better with it.

[Exzap]

Changing the flag is not enough. Last time I tried, libusb via vcpkg had issues linking on Windows. It's probably solvable but then more work needs to be done so we can support windows HID API + libusb devices in parallel. At any rate this topic is outside the scope of this PR so lets not further clutter it.

[deReeperJosh]

@Exzap can't seem to directly request a review from you without being a member of the Cemu team, but just tagging you here to add to your backlog

[deReeperJosh]

Latest push adds in a Create Skylander Dialog to the window, which was missing previously while I was still figuring wxWidgets out.

[Exzap]

This looks mostly good to me. My only complaints are of cosmetic nature in regards to variable naming and code structure. See my other annotations below.

Additionally, I think extra validation in many of the functions that work with raw data wouldn't hurt, especially in the SkylanderUSB class, but I couldn't spot any potential out-of-bounds access so it's fine.

A lot of the manual data juggling and hardcoded array indices could be avoided by defining the Skylanders data as a struct instead of std::array<uint8, 0x40 * 0x10> or raw C uint8* pointers. Similar to how we are already doing it for Amiibos.

At any rate, I think this PR has been open long enough and we don't need to delay it any further for purely surface level code changes. I will merge it as-is. If you want you can address my feedback in a future PR.

As mentioned already, portal emulation will make a lot of people happy 😄 Thanks for your contribution!

[Exzap]

Suggested change

	for (auto i = 0; i < 16; i++)
	for (auto i = 0; i < m_skySlots.size(); i++)

[Exzap]

The low level Skylander file creation code in this function belongs into a separate utility function in Skylander.cpp/.h imho. Separating UI and logic lets us recycle more code if we ever add other UI backends

[Exzap]

listSkylanders should be renamed to s_listSkylanders to match our naming convention. Even better would be to avoid using a loose global variable altogether. Instead, accessing it via a static getter method in SkylanderUSB would be preferable. What I mean is accessing it like this:

nsyshid::SkylanderUSB::GetSkylanders();

Which imho looks better than nsyshid::skylanders

[Exzap]

Suggested change

	for (auto i = 0; i < 16; i++)
	for (auto i = 0; i < m_skylanders.size(); i++)