dolphin icon indicating copy to clipboard operation
dolphin copied to clipboard

Implement tapserver-based modem adapter

Open fuzziqersoftware opened this issue 7 months ago • 6 comments

This change depends on #12234, which is currently waiting on review. (When that PR is merged, I'll rebase this one to make it easier to review.)

This implements the GameCube modem adapter. This implementation appears to be stable but is not perfect; it drops frames if the receive FIFO length is exceeded. This is probably due to the unimplemented interrupt mentioned in the comment in OnReceiveBufferSizeChangedLocked. If the tapserver end of the connection is aware of this limitation, it's easily circumvented by lowering the MTU of the link, but ideally this wouldn't be necessary.

This has been tested with a couple of different versions of Phantasy Star Online, including Episodes 1 & 2 Trial Edition. The Trial Edition is the only version of the game that supports the Modem Adapter and not the Broadband Adapter, which is what made this PR necessary in the first place.

How state should be saved and loaded is also an open question, described a bit in the comments in DoState. Probably it shouldn't do nothing (as it currently does), but I don't know what exactly it should do. Comments are welcome, of course.

fuzziqersoftware avatar Dec 03 '23 22:12 fuzziqersoftware

I was able to get the Windows build to work on Windows 11, ARM64, by making a few changes to the initially submitted code. You can view what I did over here, but note that it is just... not pretty or proper. I was just trying to simply get it to work. (But it did build and work at least.) It looks like reinterpret_cast is pretty helpful for cross compatibility?

It looks like the other thing that the Windows build doesn't like is that there are two files named TAPServer.cpp, even if they're in folders.

MSB8027	
Two or more files with the name of TAPServer.cpp will produce outputs to the same location. This can lead to an incorrect build result.  The files involved are Core\HW\EXI\BBA\TAPServer.cpp, Core\HW\EXI\Modem\TAPServer.cpp.	

image

Two notes. I got this error on both MacOS and Windows when the game initially starts to connect (initializing?) or after disconnecting.

image

Second, when the client disconnects, and some time passes (a minute?), the log goes haywire with the following error:

Screenshot 2024-01-28 at 1 53 20 AM

...and keeps going on until the game goes back online or the instance is stopped:

image

ShiftaDeband avatar Jan 28 '24 09:01 ShiftaDeband

I've updated this PR with changes based on the review comments here and in #12234, and have rebased onto main. I have not tested the Windows build, but have incorporated ShiftaDeband's changes into this PR as well, so hopefully it will work on Windows too.

fuzziqersoftware avatar Jan 29 '24 01:01 fuzziqersoftware

As for the log issue, it looks like ATH commands may not be handled? At least as far as I could tell.

ATH0 is hook control, go on hook/disconnect (ATH1 is apparently to go off hook/connect, not sure if used.) When a client disconnects, the GameCube's modem sends this to the other end.

When handled inside the RunAllPendingATCommands, adding an else if statement seems to resolve the repeating log message issue mentioned earlier:

...
    if (command.substr(0, 3) == "ATZ")
    {
      // Reset
      m_network_interface->Deactivate();
      AddATReply("OK\r");
    }
    else if (command == "ATH0")
    {
      // Disconnect
      m_network_interface->Deactivate();
      AddATReply("OK\rNO CARRIER\r");
    }
...

(Side note, I'm sure you could do an || statement there instead of a separate statement, the NO CARRIER was just a loose test.)

Of course, reconnecting and everything still works fine. Although when using this, TAPServerConnection.cpp still logs one error. Doesn't seem to be a huge issue but wanted to at least report this.

Screenshot 2024-02-07 at 8 31 14 PM

Anyway, I've tested this with several revisions of PSO from different regions and haven't had any issues. This is a great addition, again, thank you.

EDIT: With the most recent changes, everything looks good as far as I can see! No log errors, disconnecting/reconnecting works perfectly.

ShiftaDeband avatar Feb 08 '24 03:02 ShiftaDeband

Thanks for investigating; I've added a case to handle ATH0 as you recommended. I believe the remaining error is because we close the socket before terminating the read thread, so in some cases it will try to read from a closed socket. I've switched the order, which should prevent this in the future, even though it is harmless.

fuzziqersoftware avatar Feb 08 '24 04:02 fuzziqersoftware

I've been playing around with my own PSO newserv instance on the tapserver modem and BBA.

The only issue I've had with the tapserver modem is related to newserv not sending enough information fast enough, although this does mean I haven't been able to do any extensive testing with it, but otherwise it is quite stable.

However, there does seem to be an issue with the tapserver BBA where Dolphin will seemingly cut the connection to the PSO server instance randomly. As the PSO client didn't send a disconnect packet or receive any sort of disconnection instruction from the server, the game will be softlocked until the player quits the game themselves (or the client responds to having received no packets from the server after 15 minutes). This is the same behaviour that occurs if a player loads a save state while online.

The newserv packet logs don't indicate anything amiss, as it happens during innocent scenarios, and the client just times out when the newserv instance doesn't receive any responses.

After this, trying to go online again will immediately crash Dolphin.

I managed to get a backtrace under gdb with Dolphin debug of this crash, although it's not quite under the same circumstances:

tapserver-bba-crash.txt (This is on Debian 12)

This seems 100% reproducible by running Dolphin Debug under gdb, loading game GPOE8P (Phantasy Star Online Episode I & II Plus US), going online, quitting game, then going online again. If you want to test on a public newserv instance rather than a local one, 217.160.146.96:9501 will accept tapserver BBA, and 217.160.146.96:9502 will accept tapserver modem.

Matt-Swift avatar Feb 26 '24 15:02 Matt-Swift

With the new build, Dolphin no longer crashes when trying to connect online after the tapserver BBA bug kicks in (it does still fail to reconnect, however). In regular usage, this bug seems to occur after a few hours randomly, and sometimes it will also occur after a regular disconnection after some play time too.

On the debug build under gdb (and not otherwise, whether using debug only or gdb only), it happens every single time you connect online, then disconnect, meaning this is quickly reproducible.

I'm not sure how to get any sort of useful trace since the program isn't crashing, but I did decide to look at what was happening on the network with wireshark from the first connection, followed by the second (broken) connection:

normal-connection.txt broken-connect.txt

Matt-Swift avatar Feb 28 '24 12:02 Matt-Swift

@fuzziqersoftware I'd like to help test this with PSO, however the modem adapter option doesn't appear to be exposed in the UI with a build made after this PR was merged. Currently shown under SP1: Nothing, Dummy and the three BBA options. Sorry to ask if it's obvious but, where exactly can this be found? This could be the only working method for PSO as the game doesn't work with the BBA HLE, even after testing #12560 (as previously stated, freezes at unknown intervals causing loss of game data).

Shoegzer avatar Mar 23 '24 15:03 Shoegzer

It's in the same location, if you've built from dolphin-emu:master, it should just be visible under SP1. If for some reason it doesn't appear (I have not tested the master branch), build from fuzziqersoftware's fork and checkout the tapserver-modem-adapter branch and build from there.

If you're not on Linux, if you download one of the Dolphin versions from here, this will expose the tapserver BBA in SP1 for sure, as other users have already used this on my public newserv instance.

Note that this BBA option only works on servers which support the tapserver interface, which at time of writing, is any newserv instance, and the only known public one would be at ragol.org. To test it out there, put ragol.org:9501 in the configuration box for the tapserver BBA (or ragol.org:9502 for the modem) and it will just connect.

Matt-Swift avatar Mar 23 '24 15:03 Matt-Swift

Thanks, I'll try that. I'm on Linux Mint 21.3 and I built from current master but the adapter doesn't show. I'll try the fork when I get a chance though, and if the interface shows there I'll test against ragol.

Incidentally, this section of the ragol website directs users to Dolphin 5.0-16838 as "Future versions have constant FSOD issues." While I haven't tested that build myself, notaloop reported here that it too exhibits the freezing issue. If the modem adapter fixes that problem though, whomever maintains ragol (hopefully they're reading this) should update it to attract more players.

It does leave me wondering though, whether this issue is triggered by a specific event from another logged-in user and not necessarily random (therefore a server with more players would be more likely to trigger the issue, and it may not manifest as often on a less active server). I was thinking schtserv transitioned to newserv but I suppose it hasn't. That instance seems to attract the most players, partially due to the fact that the HLE BBA defaults to it. My theory is just a wild guess and probably wrong, but if not, schthack would probably make the best testing ground.

Shoegzer avatar Mar 23 '24 18:03 Shoegzer

PSO itself can cause the issue, but it is probably true that 16838 also has the issue, just to a lesser degree.

Anyway, a closed git forge PR is probably not the best place to discuss these things, but here's the details to the above:

  • I am the owner of the ragol.org PSO newserv instance, so the page has been updated. As I'm sure you can appreciate, this only got merged yesterday, although there is a news post about it.
  • SCHTServ will most likely never "transition to newserv" as it uses its own closed-source codebase written in Delphi. They can choose to host their own tapserver instance to accept tapserver BBA requests if they so choose. tapserver can be found here.
  • The issue is not triggered by a specific event from a logged-in user, nor by the server specifically. This issue happens on SCHTServ, Sylverant, and newserv regardless of how many players are present - it can even happen alone. We have played many PSO games at Ragol, and FSODs are extremely common on players on the latest Dolphin, but have never occurred to players using the tapserver BBA.

Matt-Swift avatar Mar 23 '24 20:03 Matt-Swift

Thanks, that's quite interesting.

  • Indeed, given the fast pace of the new developments it's understandable that the site wasn't updated. At the time I didn't know if the changes would be noticed anytime soon, but I'm glad to know you're the owner!
  • I had no idea the schtserv backend was closed-source but that's good to know. And I agree it doesn't sound likely they would transition away from the codebase they've developed and maintained for so many years now.
  • I didn't realize the FSOD happened when a single user was logged in but that's good to know.

I'm not sure why the new tapserver options weren't showing earlier, but they do now on a build I just made from master (so you can remove the fork reference for linux users on your page, and replace it with the requirement for Dolphin 5.0-21253 or higher). I configured the BBA tapserver for ragol.org:9501 and it worked without any other changes, with one issue: on first attempt to load a mission it got to 21% and then I got the FSOD with buzzing noise. On second and third attempts it didn't do that though, and I've been on testing for a while with no issues. I'm guessing it was a one-time issue then, since it happened during a load event, and I'm hoping not anything necessarily wrong with the tapserver implementation.

You're right though, a closed PR isn't exactly the best place to discuss issues that aren't related to the tapserver itself, though I can provide more detail on the above if anyone needs it.

Shoegzer avatar Mar 24 '24 01:03 Shoegzer