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

IOS: WD and NCD fixes #9300

Merged
merged 4 commits into from Jan 5, 2021
Merged

IOS: WD and NCD fixes #9300

merged 4 commits into from Jan 5, 2021

Conversation

leoetlino
Copy link
Member

@leoetlino leoetlino commented Nov 29, 2020

This PR fixes the implementation of three ioctlvs: Lock, Unlock (in NCD) and GetInfo (in WD), based on the implementation in IOS59.

Without a correct implementation for those three ioctlvs, Nintendo's MPDL code (no clue what it stands for) will fail at initialization time and return an error to the caller. If a title does not check for errors correctly (e.g. Driver San Francisco or Tales of Graces), this can result in game crashes.

DS communication is still unimplemented but now it actually has a chance of working if it is implemented in the future.

Fixes issue 11977 but now the game hangs when cancelling, so this is a partial fix. I am submitting this now to keep the diff small and avoid merge conflicts.

Also, the Mii Channel doesn't instantly abort DS communication anymore:

HACA01_2020-11-29_17-40-27

@leoetlino leoetlino marked this pull request as draft November 29, 2020 18:23
}
else
{
common_result = -3;
Copy link
Member

Choose a reason for hiding this comment

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

Do we know what -3 is?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it is an unknown error code. I don't remember it being used elsewhere so I'm not sure how to name it...

Copy link
Contributor

Choose a reason for hiding this comment

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

Are the error codes in this table applicable? If so, -3 would be EADDRINUSE.

Copy link
Member Author

@leoetlino leoetlino Nov 29, 2020

Choose a reason for hiding this comment

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

I don't think they're related, as this module also uses -4 which is the standard IPC_EINVAL and not a network error code. -3 is probably another standard IOS error code.

Copy link
Member Author

@leoetlino leoetlino Dec 10, 2020

Choose a reason for hiding this comment

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

Do you think it's worth adding it to the ReturnCode enum anyway (named something like IPC_ERROR_3), or are you fine with this as is?

For what it's worth, -3 is also used in two other places in our WFS implementation.

@leoetlino leoetlino marked this pull request as ready for review December 2, 2020 19:52
@leoetlino leoetlino added the QA done The PR has been checked to work properly. label Dec 11, 2020
@JMC47
Copy link
Contributor

JMC47 commented Dec 11, 2020

Pokemon Battle Revolution and My Pokemon Ranch do not freeze. So far it's just Tales of Graces.

@iwubcode
Copy link
Contributor

iwubcode commented Dec 11, 2020

Anyone able to test any of the other games?? Google tells me:

  • Animal Crossing City Folk
  • Batman: the Brave and Bold
  • Castlevania Judgment
  • FFCC: Echoes of Time
  • Geometry Wars Galaxies

all had some form of DS connectivity. Unfortunately I don't own any of those..

@AdmiralCurtiss
Copy link
Contributor

No crashes on Geometry Wars (RGLP7D), both the Game Share and the Connect to DS options. Can also repeatedly start and cancel to no apparent ill effect.

@Kolano
Copy link

Kolano commented Dec 11, 2020

For testers a more complete list of titles with DS connectivity can be found on the Wiki here...
https://wiki.dolphin-emu.org/index.php?title=Category:Nintendo_DS_(Input_supported)

@JMC47
Copy link
Contributor

JMC47 commented Dec 11, 2020

You can add Driver: San Francisco to the list. It has an invisible DS multiplayer thing it broadcasts at all times.

@leoetlino leoetlino force-pushed the ncd-wd-fixes branch 2 times, most recently from 8a99379 to 2725f0c Compare December 15, 2020 17:45
Copy link
Member

@BhaaLseN BhaaLseN left a comment

Choose a reason for hiding this comment

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

I have to say, it looks a lot nicer without the (yet) unknown parts of the state machine; and thats what delayed my review so far (since I wanted to take a closer look myself on what IOS does there).
I still didn't find the time to do so, but the code looks sound. Untested though.

NCD returns an error if it receives a request to lock the driver
when it is already locked.

Emulating this may seem pointless, but it turns out PPC-side code
expects NCD to return an error and will immediately fail and stop
initialising wireless stuff if NCD succeeds.
By making GetVector return nullptr for invalid indices, we don't have
to check the total number of vectors all the time before calling
GetVector.
This commit implements the following commands:

* open
* close
* GetMode
* SetLinkState (used to actually trigger scanning)
* GetLinkState (used to check if the driver is in the expected state)
* GetInfo
* RecvFrame and RecvNotification (stubbed)
* Disassociate (stubbed)

GetInfo was already implemented, but the structure wasn't initialized
correctly so the info was being rejected by official titles.
That has also been fixed in this commit.

Some of the checks may seem unimportant but official titles actually
require WD to return error codes... Failing to do so can cause hangs
and softlocks when DS communications are shut down.

This minimal implementation is enough to satisfy the Mii channel
and all other DS games, except Tales of Graces (https://dolp.in/i11977)
which still softlocks because it probably requires us to actually
feed it frame data.
Lets us find games to test more easily.
@delroth delroth merged commit 27013e8 into dolphin-emu:master Jan 5, 2021
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA done The PR has been checked to work properly.
10 participants