Splitscreen multiplayer + Netplay (Made with Claude) - #318
Splitscreen multiplayer + Netplay (Made with Claude)#318SomeoneIsWorking wants to merge 18 commits into
Conversation
|
I'm not sure it makes sense to have the core_controller, CoreController and multiplayer_controller all split up like that. Looks like a lot of this would simpler if we took the core-level turbo out, which endrift would rather we did, as well. |
I don't really understand what's going on. I'm not an emulator guy. I actually had a better working version for SDL client, not in this repo. |
|
Judiciously avoiding mentioning that this is vibe coded this time I see. |
|
I wasn't trying to avoid it. I'm just trying to figure out. |
|
I assumed it was, based on (the code itself, obviously, but also) the remark about not knowing if it was implemented correctly. I had also assumed that was why it was split into 3 separate headers and files, but it sounds like it's that way because the Qt frontend does it that way, too. That makes it much more acceptable, IMO, if it makes it easier to track upstream changes to those Qt frontend files. |
|
Generally speaking, tracking reimplementations of those files is suboptimal. One of the things I'm planning to do in 0.12 with the netplay support code is lower-level implementations of core coordination for multiplayer purpose, for cases like this. It should be possible to do with even single-threaded code, akin to how TGB-Dual swaps between cores every so often. But it may be a year before I even get there, so who knows i it's worth waiting or just yanking it out and replacing it later. |
I tried this a while ago because it makes perfect sense. Why do lockstep and mutex on the same thread, just run the cores one after another. But I could NOT get it working. |
|
What the hell I'm going to try anyway. By "I" you get the point. 1% I, 99% Autocomplete bots. |
54e6369 to
28bc5da
Compare
|
Hey I got lucky. Working singlethreaded lockstep. I faced timing and desync issues and solved it with "pumping" (and cycle skips if pumping occurred) |
28bc5da to
21530f3
Compare
21530f3 to
0c72697
Compare
|
Don't mind the last two force pushes, they are just cosmetics. |
|
Sounds good. I'll see if we can get some testers. |
|
I tested it on macOS when developing. |
|
One thing eating my brain is Network possibility because technically let's say. You have 2 people running the game. But I have no idea where someone would even begin with this. Edit: I got netplay working but uhhh that discussion should be for another PR... |
|
Okay I said I want to keep netplay related comments out of this PR but I had a problem regarding RA rollback netplay not exposing player index and I made a PR to RetroArch which got merged so now rollback netplay is playable but I still want to make it a separate PR because it has some parts that I want to talk about separately. 
|
|
Replaced "pumping" with individual running based on state |
|
@SomeoneIsWorking is this ready to go, from your perspective? or are you still actively working on it? @endrift do you have any thoughts on any of it? I/we don't want to step on your toes merging something you think would be a problem down the road. I haven't actually done any testing on it myself, yet, so it wouldn't be merged immediately anyway, just to be clear. |
|
@hizzlekizzle it is ready to go IMO. I'm going to submit the netplay PR after this which contains some further refactoring. |
|
FWIW, I just compiled this for Windows and played Advance Wars with myself with no issues. @kaysedwards what sort of platform/device/OS were you using? |
|
@hizzlekizzle syncArmed or without it? |
|
Oh, I didn't even think to test that. I was the non-syncArmed commit. |
|
Okay then I think @kaysedwards ran into the same problem you did Edit: I just did a fresh build with latest commit on a Fedora machine and it worked fine. By fresh build I mean. This is an entirely new setup with no RA installed. So I downloaded RetroArch.AppImage. |
|
I just re-compiled with the syncArmed commit and it worked perfectly on Windows but failed in Linux, where it just spins on the connection screen with this output repeated indefinitely: |
|
@hizzlekizzle I hate to reiterate this but please try a clean build, run a Also. Which linux? Maybe I can test it in a VM |
|
Ubuntu linux. I clean between every build. I can try a fresh pull in case the clean routine isn't actually cleaning everything. EDIT: yeah, same behavior with a fresh pull. Tested Kirby this time and it just sits there at the multiplayer connection screen with the connection data spinning forever. |
|
I removed syncArmed checks from mode switches. I tested both games again and I tested the scenarios that crashed for me without syncArmed. |
|
Just tested the latest commit with linux and windows and both worked great. |
|
I've been waiting until you got something that worked before taking too close a look at it. I'll try to find some time to look at this in the next few days |
|
Thank you @endrift (consider me a fan) |
|
hey @endrift did you ever get a chance to peek at this? it seemed solid/reliable in my testing, but I'm not that familiar with the mGBA codebase to know if it's implemented sensibly. OTOH, it does live all in the libretro directory, so any mess is confined there :) |
|
Sorry, I've been distracted and it fell off my radar. I'll try to look at this soon |
|
I have tried this out a little, and I have observed a few things. First, from what I've tested, it works perfectly online through Retroarch Kaillera, although I have not tried the standard netplay version. I found that in Sonic Advance 3, at the boss of the Toy Kingdom world, there is a relatively high chance the emulator will crash. I have a replay file that consistently causes this crash if anyone wants it. In Kirby Nightmare in Dream Land, it seems that most of the time you start up a game in multiplayer, as the star is crashing into the overworld area, the game feels laggy. If I stay in that area, I find that it takes a random amount of time before going back to full speed. Every time I go through a door, this lag returns before going away again. It seems to be running at about 45 fps when this happens (in game, since it's not my computer or Retroarch slowing down). I think observing how long it takes to jump and hit the ground is an especially easy way to check if it's lagging. Finally, there are a few features that I personally consider missing. These are the ability to choose which screen(s) are seen in the main window, which GBAs the audio is coming from, and targeted cheats. I'm unsure if Retroarch itself is able to choose which GBA a cheat is used on, but I currently have it set for myself so that cheats affect either players 2 to 4 or all 4 players. I also was able to get those audio and videos features I mentioned working, but this was just low quality vibe coded stuff that I don't think would be helpful to share. Edit: I also forgot to mention individual save files. Currently, it only loads a save file for the first GBA. |
|
Thank you. It has been a very long time I tried this. And the game issues you mentioned, also not sure if netplay branch resolves them. |
|
@SomeoneIsWorking I appreciate the work done so far and am looking forward to future updates. |
|
@SuperDavid2 Sorry I neglected this a lot as it doesn't look like it will get merged sonic_2p_linked_last20s.mp4I think I fixed your crash case with Sonic, does it look right to you? Edit: This is also "vibe-coded" so don't be reluctant to share if you could make any fixes please. And LLMs have evolved a good deal since I posted the PR. |
|
Also recorded videos for Kirby & The Amazing Mirror and Advance Wars kirby_link.mp4aw_link.mp4 |
mTiming's masterCycles is a free-running uint32 read back through an int32 API, so every cycle stamp in the lockstep coordinator goes negative once per 2^31 cycles - roughly 128 seconds of emulated time. Comparing two of them with a plain int32 subtraction overflows whenever the pair straddles that wrap, which is undefined behaviour, and at -O2/-O3 the compiler folds the comparison to a constant. The one that mattered is the event-due test in _lockstepEvent: past the wrap it decided no queued event was ever due, the event queue stopped draining, and the cooperative scheduler's watchdog aborted the core. That is the Sonic Advance 3 crash - reproducible at about 11 seconds of link play with an -O3 build, absent with DEBUG=1 (-O0), and inconsistent across platforms because each compiler folds it differently. Route every such comparison through _cyclesBetween(), which does the subtraction in uint32 and converts back, so the difference is well defined across the wrap. Also restores the indentation in GBASIOLockstepDriverSetMode. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three conflicts, resolved as follows. * Makefile.libretro and libretro-build/Makefile.common: upstream moved the libretro build to cmake (mgba-emu#330) and deleted both. Took the deletion; the macOS build fix this branch carried for Makefile.libretro is no longer needed, because the cmake build probes HAVE_LOCALE / HAVE_STRTOF_L through find_function instead of hardcoding them per platform. * retro_get_system_av_info: upstream dropped a duplicated aspect_ratio assignment. Took the deletion and kept mLibretroMultiplayerAdjustGeometry, which scales the aspect ratio itself for the split layouts. * retro_run audio: upstream replaced mAudioResampler with the new audioConverter, and the cmake libretro target now excludes util/audio-resampler.c outright, so the old path could not have linked. Took upstream's converter loop and kept this branch's two additions on top of it - feeding it from the local player's core in "Self" mode, and draining the other players' audio buffers so they do not back up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
6d309b5 to
da6f319
Compare
SomeoneIsWorking
changed the title
syncArmed held the lockstep machinery inert until the first MULTI transfer start, so two attached cores ran free until the games actually began talking. It was introduced to stop a hang, but the hang was the signed overflow in the cycle comparisons, fixed in the preceding commit: past the 2^31-cycle wrap the event-due test folded to a constant, the queue stopped draining, and the cooperative watchdog aborted. Keeping the coordinator inert until a transfer simply delayed reaching the wrap in the cases people happened to test. Measured on the case it was meant to help - Advance Wars in link mode with no transfer yet initiated, player 1 driven into the menus while player 2 sits on the name entry screen, saved as a state so both builds start identically. Counting core interleavings (runLoop calls) per frame, which is deterministic and so immune to machine noise, over 12000 frames: with syncArmed 35843047 runloops 2986.92 per frame without syncArmed 35986923 runloops 2998.91 per frame +0.40% Wall-clock throughput could not distinguish the two at all; repeated runs of the same binary varied by more than 2x on this machine. Neither build crashes or stalls, in that scenario or in 12000-40000 frame soaks of Sonic Advance 3, Kirby & The Amazing Mirror (2P and 4P) and Advance Wars. Both still complete a real link: Kirby through MULTI-PAK to a running 2-player game, Advance Wars through Multi-Pak to day 1 of a 2-player battle. Serialized flag bit 22 is now unused; bits 23-27 were already free, so nothing shifts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
_multiplayerDisplayLabel was declared but never defined or called, which clang reports as an unused function on every build of this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removed syncArmed entirely, it was apparently a bandaid for a wrap bug. Update: Multisave also should be working now. |
SomeoneIsWorking
changed the title
Only player 1 had a save: the libretro API exposes a single SAVE_RAM region and the frontend manages that one for the primary core, so the extra cores booted with empty cartridge memory every time. The extra players now open their own save next to player 1's, named "<rom>.p2.srm", "<rom>.p3.srm", "<rom>.p4.srm", and hand it to the core the same way the deferred setup does for player 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
libretro.c was reaching into the multiplayer struct directly to work out which display mode was in effect, which player is local and whose audio to play. That state now stays behind the module's own interface: * the module owns its instance, so libretro.c no longer holds one * session and display refresh each become a single call that says whether the frontend needs new AV info * picking the audible core and draining the others moves next to the rest of the per-player core handling Also drops three things nothing calls: the primary-core accessor, the netplay rollback-context getter and its contextKnown field. No behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5 participants
I wanted to play Kirby & The Amazing Mirror split-screen co-op.
This works but I need a second opinion.
I have no idea if it's implemented correctly.