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
Add initial WiiConnect24 support #11072
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few thoughts that jumped at me while looking thru. Both are pretty much ignorable, since I can't really offer any alternatives that make it more readable/understandable...but you never know if someone else has a smart idea.
| std::string content(m_data.entries[entryIndex].filename); | ||
|
|
||
| // Determine if we need to append the subtask to the name. | ||
| if ((Common::swap32(m_data.entries[entryIndex].subtask_bitmask) & (1 << (1 - 1))) == 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the symmetry with GetDownloadURL, but this 1 - 1 is really messing with me, and I don't know what to do about it.
I keep thinking something along the lines of BitField but I'm not sure if that works, straight outta that PoD.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a good case for ExtractBit from BitUtils.h.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this should still be simplified here. I suggest (with #include "Common/BitUtils.h"):
if (subtask_id &&
Common::ExtractBit(Common::swap32(m_data.entries[entry_index].subtask_bitmask), 0))
Likewise for GetDownloadURL() you can use
if (subtask_id &&
Common::ExtractBit(Common::swap32(m_data.entries[entry_index].subtask_bitmask), 1))
and for isEncrypted() you can use
return !!Common::ExtractBit(Common::swap32(m_data.entries[entry_index].flags), 3);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would have been better to have a dedicated commit for the FatFs' Externals and another one for the modifications added to FatFs to support vff (and who authored it?).
Otherwise, I found some coding style issues.
| mbedtls_aes_context aes_ctx; | ||
| size_t iv_offset = 0; | ||
|
|
||
| ASSERT(!mbedtls_aes_setkey_enc(&aes_ctx, key, 128)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this make Dolphin halt? Is such behaviour wanted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I agree that this should be unwanted behaviour, I saw above the ContextGeneric initializer does the same thing so I thought I should make the logic similar. Not sure if that is for another reason though.
| return -1; | ||
| } | ||
|
|
||
| if (Version() != 1) | ||
| { | ||
| ERROR_LOG_FMT(IOS_WC24, "DL list version mismatch"); | ||
| return -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do IOS returns the same error value for these two (different) errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears that IOS (similar to behavior within channels) oddly never validates the file's magic or version - only the System Menu sets these upon file creation.
However, if the max entries/subtasks values differ from constants, and the file is effectively not 0xF800 in length (header + 120 entries + 120 records), it returns WC24_ERR_BROKEN. Perhaps it'd be worth implementing these as well, and utilizing its error for our own?
|
I have a feeling the custom FatFS modifications are going to cause us headaches in the future if we, say, ever want to update FatFs... As far as I can tell the changes are pretty minor, so there's probably a better way to handle this -- maybe make I also suspect you need to unify your new |
I don't quite follow, could you please show an example? |
|
As of 403594b, News Channel is now supported. |
|
I have done some refactoring on the FatFs code so that this can work without having to actually touch the external code. Please check if the code at #11107 still works correctly (which is the refactoring + all current code in this PR); if yes, I would move forward with merging #11108 (which is just the FatFs refactoring) and then rebasing this PR on top of that. As a side note, could you provide some instructions for how to actually use this with RiiConnect24 and what the expected result is? |
Works perfectly, thank you for that!
The idea is that the user would use the RiiConnect24 Patcher and install the patched WAD's (I have not tested the DNS method, maybe it works?). Once installed, everything should work as it would on a normal Wii, minus Forecast, News (DL Tasks are not implemented yet and DownloadNowEx is not called on every launch of the channel) and mail. Region Select may have an issue with Dolphin created NAND's as during my testing it throws the WiiConnect24 account not registered error. |
|
Looking forward to seeing this used for Mario Kart Wii competitions - right now playing competitions on Dolphin requires you to run the competition patcher each time to download the newest competitions, with WC24 support this would happen automatically like on a Wii. |
In addition to what Sketch said, we have a tutorial to install RiiConnect24 specifically for Dolphin (that will be updated once this is merged) at https://wii.guide/riiconnect24-dolphin, so if we're looking to link something inside of Dolphin or on the wiki that's the easiest link. |
0772bb4
to
1012e6c
Compare
|
You, uh, need some help here? |
|
Yes please |
|
Anything specific you want to ask? Or do you just want me to rebase onto master? |
|
Could you rebase onto master? |
331a9eb
to
19a8a3e
Compare
|
Done. Note that you'll need to manually update your local branch now; don't try to pull the remote changes, git would try to merge them. Something like |
|
Alrighty, thank you for the help. |
|
Just a couple small things, otherwise this looks good to me now. Untested though. |
|
Oh yeah, and you didn't apply the ExtractBit stuff either (#11072 (comment)), any particular reason, does that not actually work like I thought? |
I did not see this comment, but I tested and it does work. |
|
In case you didn't see my furious editing, https://godbolt.org/z/aKachGEE1 here's a reference for how to write the weird bitshifts in |
|
Also please squash the fixup commits when you're done. Yell if you need help with that. |
|
Ok all changes have been applied, how would I squash the commits? |
|
First start with:
The commit hash here is the first hash in this PR, for reference. You'll get a text window with the commits here. Mark all the ones you want to squash as 'f' (instead of the default 'pick'), then save and close the file. You can then verify this worked correctly by inspecting the commits, with git log or whatever. Once you're happy, push the commits to GitHub with |
c5b0797
to
75650b4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, this looks good from my end now. Untested.
75650b4
to
e413d7f
Compare
|
News and Forecast channels do not work, and some channels still give weird errors (eg. the Nintendo Channel claims that the server is undergoing maintenance). But the basic functionality is there, and definitely better than what we currently have in master, so |
|
The Nintendo Channel going undermaintence is a server side issue with RiiConnect24. What system did you use to test? Because from what I have tested macOS works fine but Windows attempts to read non-existent sectors. |

Big work in progress, but the support exists.
VFF support is handled by FatFS, which was modified a bit to handle the VFF header and read properly.
HTTP request is handled by Common::HttpRequest which I am not sure if I should be using or not. Currently no headers are being sent, support should be added later down the line for this (There are many depending on the scenario.
DownloadNowEx is in a separate function which is launched asynchronously. Support for subtasks and decryption is present.
DLList parsing thanks to @spotlightishere. Currently parsing the entire file exists, however writing the file incase an entry does not exist doesn't.
In my testing, every WiiConnect24 channel except for the News Channel is able to download, decrypt content if needed and pack into their VFF correctly. (The only exception is TV no Tomo which crashes after the in-channel region setup for unrelated reasons.)