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

Super Mario Advance 4: SMB3 Link with e-Reader communication error #2925

Closed
DesertWolf29 opened this issue May 15, 2023 · 4 comments
Closed
Milestone

Comments

@DesertWolf29
Copy link

DesertWolf29 commented May 15, 2023

As of version 10.2, the link function between Super Mario Advance 4 and the e-Reader will fail. The last known working version is 10.0. 10.1 will have the e-Reader ROM speed up when in a multiplayer window, but I successfully scanned a card over to SMA4. With 10.2, and the latest development build, the link fails to establish between the two ROMS, resulting in a communication error. The same speed-up issue persists as well.

Edit: Also of note, when using the installed version of 10.0, the communication fails. When using the portable version, it works.

SMA4 Bug Report.zip

@Testsr
Copy link

Testsr commented Jun 14, 2023

Can reproduce. Communication after getting the save is unaffected.

@Testsr
Copy link

Testsr commented Jun 16, 2023

I have bisected the connection issue to commit 395fa2d on 0.10 or f046596 on master. The speed up issue is bisected to 8e58dca on 0.10.

@Testsr
Copy link

Testsr commented Jun 20, 2023

@endrift Can we add this to a milestone,so that it gets fixed.

@endrift endrift added this to the mGBA 0.10.3 milestone Jun 20, 2023
@Testsr
Copy link

Testsr commented Jun 28, 2023

I have traced down the regression to lines 517 - 534 of lockstep.c

...
      if (node->id < 3 && attached > node->id + 1) {
	      value = GBASIONormalSetSi(value, GBASIONormalGetIdleSo(node->p->players[node->id + 1]->d.p->siocnt));
      } else {
	      value = GBASIONormalFillSi(value);
      }
      
      enum mLockstepPhase transferActive;
      ATOMIC_LOAD(transferActive, node->p->d.transferActive);
      if (node->id > 0 && transferActive == TRANSFER_IDLE) {
	      int try;
	      for (try = 0; try < 3; ++try) {
		      GBASIONormal parentSiocnt;
		      ATOMIC_LOAD(parentSiocnt, node->p->players[node->id - 1]->d.p->siocnt);
		      if (ATOMIC_CMPXCHG(node->p->players[node->id - 1]->d.p->siocnt, parentSiocnt, GBASIONormalSetSi(parentSiocnt, GBASIONormalGetIdleSo(value)))) {
			      break;
		      }
	      }
...

(removing the lines fixes the bug), and know the regression is caused by the GBAs having different transfer modes. I don't know what causes the transfer mode desync, however. My best guess is a race condition in setting the SIOCNT.

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

No branches or pull requests

3 participants