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

Emulate Skylanders Portals within Dolphin #11331

Merged
merged 3 commits into from Jan 24, 2023

Conversation

deReeperJosh
Copy link
Contributor

@deReeperJosh deReeperJosh commented Dec 8, 2022

The Skylanders Wii games require external hardware in order to play, and while they are relatively easy to source not all the portals work with every game, and MacOS users have variable success due to the USB device being claimed by the kernel without providing root access to Dolphin.

Using the Trap Team version of the Portal that I own, I have been able to successfully track the requests and responses, and have been able to Emulate the physical portal within Dolphin, a feature requested here.

This requires a new method to Schedule responses to Control and Interrupt Messages, a new Class to extend as a device with descriptors pre-defined as a Skylander Trap Team wired portal, a new struct to define Fake Skylander toys, and a new tool within the UI to create, clear and load Fake Skylanders to the Emulated portal.

The structure of the UI including widgets and windows is basically just a copy+paste from RPCS3, and then modified to fit Dolphin's coding styling, as well as using Dolphin specific methods to handle File IO. As well as this, the structure of handling requests within Dolphin is a lot of copy+paste from RPCS3 as well, with some modifications to use Trap Team Portal specific responses and Dolphin utility methods. If this is an issue I am more than happy to modify the code and make it from scratch, but it was a lot easier without much C++ experience to get as much of a head start as possible, and I'm sure not much (besides the UI) will change anyway.

C++ certainly isn't my strongest language, and this will be my first ever contribution to any open source project, so there is bound to be many improvements to be made, any feedback will be much appreciated.

@deReeperJosh deReeperJosh changed the title Skylanders portal emulated and working, tidy commits and formatting Emulate Skylanders Portals within Dolphin Dec 8, 2022
@Tilka
Copy link
Member

Tilka commented Dec 8, 2022

Nice! Can you post some screenshots of the new UI?

@deReeperJosh
Copy link
Contributor Author

Nice! Can you post some screenshots of the new UI?

Sure thing!
I've added the tool to this window here:
Toolbar Location
This is the Window that pops up, to tell you the status of the Skylanders on the portal:
Skylander Window
This is the Create Skylander window, where you can search a list of Skylanders known, create your own from an Id and Variant number:
Create Dialog Window

@deReeperJosh
Copy link
Contributor Author

Added a method definition required for Android, which caused the build to fail, have re-tested locally with Android Studio and build succeeded 🤠

@deReeperJosh
Copy link
Contributor Author

Youtube video uploaded just to demonstrate the key functionality, playing a build on my Macbook Pro w/ M1:
Emulated Skylanders Portal on Dolphin

@mandar1jn
Copy link
Contributor

mandar1jn commented Dec 9, 2022

It seems like you are almost exactly copying the rpcs3 portal when it comes to handling incoming requests. Although there is nothing inherently wrong with their implementation, it gets some details wrong. For now this implementation is solid. If this gets merged, I’ll create a touch-up pr fixing some of them. They are mostly cosmetic (lights) related

@deReeperJosh
Copy link
Contributor Author

It seems like you are almost exactly copying the rpcs3 portal when it comes to handling incoming requests. Although there is nothing inherently wrong with their implementation, it gets some details wrong. For now this implementation is solid. If this gets merged, I’ll create a touch-up pr fixing some of them. They are mostly cosmetic (lights) related

Yeah a lot of this will replicate RPCS3, but the color requests were not responded to - what differences would you make?

@mandar1jn
Copy link
Contributor

The L and J command are also Color related. The J command is responded to in your code but too early and I don’t think it is that big of a deal. Both L and J can set seperate sides of the portal since that was a feature of the traptanium portal. This could later be used if anyone wants to spruce up the skylanders portal window. You’re also handling A slightly incorrect

@mandar1jn
Copy link
Contributor

Funnily enough, the correct handling of A is described in the comments

@deReeperJosh
Copy link
Contributor Author

The L and J command are also Color related. The J command is responded to in your code but too early and I don’t think it is that big of a deal. Both L and J can set seperate sides of the portal since that was a feature of the traptanium portal. This could later be used if anyone wants to spruce up the skylanders portal window. You’re also handling A slightly incorrect

I think it's probably best to get it all correct in one PR - how about we work together to get it working properly?

@mandar1jn
Copy link
Contributor

Sounds good. Do you have discord or something? Also, some proof I’m really knowledgeable: https://github.com/mandar1jn/SkylandersEditor

@deReeperJosh
Copy link
Contributor Author

Sounds good. Do you have discord or something? Also, some proof I’m really knowledgeable: https://github.com/mandar1jn/SkylandersEditor

No need for proof! I've definitely used your tool before :)

If it's anything to go by I based all of the responses off of the Trap Team portal, which responded to J/L commands the same way in Dolphin that I've emulated but I probably missed something. My discord is Grim_de#3727

@mandar1jn
Copy link
Contributor

Your responses are not wrong. Just the way you handle the command data

@mandar1jn
Copy link
Contributor

I have sent you a friend request

@EGNL
Copy link

EGNL commented Dec 9, 2022

Hello, I am the creator of the Feature Request thing. I have totally no experience with coding but I am glad to see someone (or multiple people) work on this, good luck!

@Florin9doi
Copy link

Florin9doi commented Dec 9, 2022

@RipleyTom
Copy link

RipleyTom commented Dec 9, 2022

I just stumbled upon this, a lot of the code is a straight copy/paste from rpcs3(the UI and the portal emulator internals), an acknowledgment in the description would have been nice, that is all.

@deReeperJosh
Copy link
Contributor Author

I just stumbled upon this, a lot of the code is a straight copy/paste from rpcs3(the UI and the portal emulator internals), an acknowledgment in the description would have been nice, that is all.

Yes of course! There is still lots I need to tidy up but I will definitely be adding acknowledgments for RPCS3 in the code, I briefly mentioned it in the first comment of the PR but can definitely do more

@deReeperJosh
Copy link
Contributor Author

Oh awesome - the number of skylanders allowed on the portal is a final variable so will just need to be changed from 4 -> 8

@mandar1jn
Copy link
Contributor

@deReeperJosh something that's inaccurate but I won't fix for fear of breaking things: from what I can see, you constantly send status responses even when the portal is disabled. Your code seems to rely on this behaviour so I won't fix it but you should

@mandar1jn
Copy link
Contributor

I have submitted a pr to your fork that fixes the way Activate and all the color commands are handled (deReeperJosh#2)

@deReeperJosh
Copy link
Contributor Author

I have submitted a pr to your fork that fixes the way Activate and all the color commands are handled (deReeperJosh#2)

Awesome thank you! I'll approve and merge that now, was just about to do the same thing

@mandar1jn
Copy link
Contributor

Also, it would be great if you added support for normal 1kb binary files. That way people can used real dumped figures

@deReeperJosh
Copy link
Contributor Author

Also, it would be great if you added support for normal 1kb binary files. That way people can used real dumped figures

Normal 1kb files will work but the file extension will just need to be .sky, I've used dumps myself with this build and they've worked well! I'm not sure if there is a standard file extension that people will have, but it would just be a matter of including that file extension in the Qt code

@mandar1jn
Copy link
Contributor

Also, it would be great if you added support for normal 1kb binary files. That way people can used real dumped figures

Normal 1kb files will work but the file extension will just need to be .sky, I've used dumps myself with this build and they've worked well! I'm not sure if there is a standard file extension that people will have, but it would just be a matter of including that file extension in the Qt code

I tried a regular 1kb dump I made but I got a file too small error

@deReeperJosh
Copy link
Contributor Author

Also, it would be great if you added support for normal 1kb binary files. That way people can used real dumped figures

Normal 1kb files will work but the file extension will just need to be .sky, I've used dumps myself with this build and they've worked well! I'm not sure if there is a standard file extension that people will have, but it would just be a matter of including that file extension in the Qt code

I tried a regular 1kb dump I made but I got a file too small error

Strange, the file opening is based on existing File IO code within Dolphin, shown here:
std::array<u8, 0x40 * 0x10> file_data; if (!sky_file.ReadBytes(file_data.data(), file_data.size()))
So either the read failed or the file isn't quite the right size

@deReeperJosh
Copy link
Contributor Author

Changes made - anyone aware of what the messaging in the failed windows build means? https://dolphin.ci/#/builders/21/builds/9955

Copy link
Member

@leoetlino leoetlino left a comment

Choose a reason for hiding this comment

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

LGTM

@leoetlino
Copy link
Member

Changes made - anyone aware of what the messaging in the failed windows build means? https://dolphin.ci/#/builders/21/builds/9955

That build failure looks unrelated. Might be related to the recent merge of #11438?

@deReeperJosh deReeperJosh requested review from AdmiralCurtiss and JosJuice and removed request for AdmiralCurtiss and JosJuice January 22, 2023 04:50
@deReeperJosh
Copy link
Contributor Author

Oops sorry for the review spam, too used to ADO and adding multiple reviewers at once there, just realising now that it removes and adds a new reviewer each time. Just looking to get any last feedback on this one 😁

Copy link
Contributor

@AdmiralCurtiss AdmiralCurtiss left a comment

Choose a reason for hiding this comment

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

Some of the logic is still a bit confusingly written, but tbqh this seems totally fine for now. I would merge this if not for this that strikes me as wrong:

Source/Core/Core/IOS/USB/Emulated/Skylander.cpp Outdated Show resolved Hide resolved
Capitalize Skylander in tr strings

Lint and validation method fixes

Proper Attach and Change Interface method

Re-jig code to exit early and read easier
@AdmiralCurtiss
Copy link
Contributor

Yeah looks good. @JMC47 Please give this another round of testing, and if it's fine you can merge it.

@JMC47
Copy link
Contributor

JMC47 commented Jan 24, 2023

Everything is still working in this build, was hotswapping Skylanders and whatnot in all four games that I have. I tested the creation mechanism too to create blank ones with no stats too.

@AdmiralCurtiss AdmiralCurtiss merged commit d4d6f3d into dolphin-emu:master Jan 24, 2023
@mandar1jn
Copy link
Contributor

@deReeperJosh I believe congratulations are in order. We are now both part of the dolphin commit tree.

@deReeperJosh
Copy link
Contributor Author

@mandar1jn feels good to see contributor next to our names 😁

@TheRealThomasGamer4000
Copy link

You should probably remove the senseis and creation crystals. there's no Imaginators port on wii.

also, there's an unused skylander variant named "Gear Head VVind Up" that isn't in the list. ID 3011, Variant 0424.

@deReeperJosh
Copy link
Contributor Author

You should probably remove the senseis and creation crystals. there's no Imaginators port on wii.

also, there's an unused skylander variant named "Gear Head VVind Up" that isn't in the list. ID 3011, Variant 0424.

Thanks for the heads up, I'll remove these in a future release!

@KermitPrplYT
Copy link

KermitPrplYT commented Mar 3, 2023

Does anybody think there is a way to emulate the portal audio for Trap Team so I can hear Kaos' dialogue in level 18 and also the Portal Audio?

@mandar1jn
Copy link
Contributor

Does anybody think there is a way to emulate the portal audio for Trap Team so I can hear Kaos' dialogue in level 18 and also the Portal Audio?

There is. I just need to dive into how dolphin handles audio

@Pokeyboy2017
Copy link

This is a dumb question, but is the portal emulator avaliable for download?

@JosJuice
Copy link
Member

It's part of the Dolphin download.

@deReeperJosh
Copy link
Contributor Author

deReeperJosh commented Mar 14, 2023

@KermitPrplYT this is now possible in the latest dev edition of Dolphin, see #11644

@KermitPrplYT
Copy link

@deReeperJosh Thanks!

@potatoes911
Copy link

Thank you so much. This allowed me to re-live my childhood. You are a legend. Thank you.

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