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

Gundam: Senjou No Kizuna Portable Adhoc network bugs #14172

Open
shanzenos opened this issue Feb 17, 2021 · 30 comments
Open

Gundam: Senjou No Kizuna Portable Adhoc network bugs #14172

shanzenos opened this issue Feb 17, 2021 · 30 comments

Comments

@shanzenos
Copy link

This issue has been occurring for years now, although at the moment I am fully updated to PPSSPP 1.11.2
When entering the multiplayer lobby, matches work for only 2 players- and works fine. If even one more player enters the lobby while 2 are already present, they will "join" and be able to view the lobby, but will not show up in the player list. If the match is started all players will get a connection error message and the room will close it's self. Multiplayer for just 2 players works perfectly fine however. The game supports a maximum of 8 players over adhoc.

@anr2me
Copy link
Collaborator

anr2me commented Feb 18, 2021

@shanzenos could you test with multiple instances of PPSSPP on the same PC with more than 2 players? To see if it works or not

Btw are you using VPN when playing with more than 2 players?
because there were player joining issue related to VPN on PSP2i games in the past, and the fix was by increasing socket buffer size... may be it's not big enough for this game...

@anr2me
Copy link
Collaborator

anr2me commented Feb 19, 2021

@shanzenos could you guide me how to do multiplayer? how to host & join?
I can't read japanese text LOL
image

Edit:
Looks like it's AdhocMatching issue, when the 3rd player joined it didn't shows up in the list on Player 1 & 2 side, but when Player 4 joined it's shown in the list (4 players only shows 3 players), and then the 2nd player gets a blue screen:
image
image
image

While other players got Communication error:
image

Anyway, AdhocMatching issue will probably takes some times to be fixed.

@shanzenos
Copy link
Author

I've heard up to 8 players used to work in older versions of the emulator but I was never given a version number. I'm thankful though for you doing some debugging for the problem, hopefully the attention eventually gets the bug fixed I know quite a large number of people that would be thrilled to hear about it being fixed.

@sum2012
Copy link
Collaborator

sum2012 commented Feb 20, 2021

@anr2me
For the blue screen Try
in Core\PSPloaders.cpp
Force Memory::g_MemorySize = Memory::RAM_DOUBLE_SIZE;

@anr2me
Copy link
Collaborator

anr2me commented Feb 21, 2021

@anr2me
For the blue screen Try
in Core\PSPloaders.cpp
Force Memory::g_MemorySize = Memory::RAM_DOUBLE_SIZE;

do i need to change the source code like that or can i do it from compat.ini ?

@sum2012
Copy link
Collaborator

sum2012 commented Feb 21, 2021

The change is wrong,Just a test whether is memory issue

@anr2me
For the blue screen Try
in Core\PSPloaders.cpp
Force Memory::g_MemorySize = Memory::RAM_DOUBLE_SIZE;

do i need to change the source code like that or can i do it from compat.ini ?

@ghost
Copy link

ghost commented Sep 20, 2021

Not related to the issue but if you try to save a replay that shows Android VS PC you might see funny animations on the Android side lol.
PC to PC will show the replays correctly though.
And it will show correctly after you save the replays it seems.
The issue with 3 players and up is still there if someone is asking even on the latest orphis builds.

@ghost
Copy link

ghost commented Sep 20, 2021

Seems like 3 players works fine now if you wait a bit for the 3rd player to show up at least locally.
Having frameskip on might help it work better.
The more ping you got the more it takes for the 3rd player to show up (by using clumsy).

@PAUV0S
Copy link

PAUV0S commented Sep 20, 2021

i think 3 players on vpn needs more testing to see if it truly doesn't work.

@ghost
Copy link

ghost commented Sep 20, 2021

Yeah for some reason it only works with a VPN.
Trying a public server will just end in misery for all players involved.

@anr2me
Copy link
Collaborator

anr2me commented Sep 21, 2021

Hmm.. when i tried to test this again using the fix i did for Super Pocket Tennis on my test build, it seems all 4 players are visible on each PPSSPP now (previously only 3 players shown in the list), but if i just leave it there one of the PPSSPP will get frozen, which will eventually causes the other players to shows disconnection dialog
image
i can't even tell whether this is a regression or an improvement LOL

PS: I'm not sure why it became frozen either, as the log didn't show anything, and showing the Disassembly window didn't seems to hit any breakpoint or crash, pressing ESC to go back to PPSSPP settings didn't work either :( may be a race condition...

PPS: Looks like if i use the same save data for all players, all of them will be in the same group (left side group)

Edit: I can confirm that there is race condition happening on AdhocMatching :(
"Emu" thread is trying to lock peerlock which already locked by "MatchingInput" thread, with this callstacks:

sceNetAdhocMatchingSendData -> sendBulkDataPacket -> sceNetAdhocPdpSend -> resolveMAC

Meanwhile "MatchingInput" thread is trying to lock context->socketlock which already locked by "Emu" thread, occurred within broadcastPingMessage

Ugh.. all of these complicated locks can be avoided & removed by changing AdhocMatching real threads into PSPThreads, thus removing the possibility of race condition.

@anr2me
Copy link
Collaborator

anr2me commented Sep 21, 2021

The issue that caused Failed to allocate memory seems to be due to piling up AdhocMatching Events (which came from incoming data from players), which may lead to invalid memory access when null address from the failing allocation being passed to the callback handler or the callback handler it self tried to allocate memory (and failed)
image

AdhocMatching - Remaining Events: 932 <-- this is way too much, normally about 1-2 pending events in the queue for each additional player

Each events have an optional data of 768 bytes which is allocated on PSP memory (rounded to 1024 bytes i guess), and with more players more data will be received, thus more events will be generated in the queue.

Something similar was also happened on Lord of Arcana in the past, and the fix i did for it wasn't perfect.
I think the only way to properly fix it is probably by implementing ACK and blocking current thread on sceNetAdhocMatchingSendData while waiting for the ACK event, this might prevent the game from flooding too much data due to faked ACK.

@ghost
Copy link

ghost commented Sep 21, 2021

I think this is related to games that generate a random SSID for each session/player (noticed this on xlink).
If so then Ace Combat: Joint Assault gonna have a similar issue as well (and ofc Lord Of Arcana does it as well).

@anr2me
Copy link
Collaborator

anr2me commented Sep 21, 2021

Ugh.. i also found crash issue at return hleLogSuccessVerboseX(SCENET, ERROR_NET_ADHOC_WOULD_BLOCK, "would block"); of sceNetAdhocPdpRecv :(
image
I guess hleLog isn't thread-safe and shouldn't be called from another real thread.. in this case the MatchingInput thread

@hrydgard
Copy link
Owner

Yeah, hleLog should only happen on the main CPU emulation thread I think... It looks odd to have another thread call a HLE function directly (sceNetAdhocPdpRecv)? Maybe some code should be extracted from that function into a smaller one, that doesn't call hleLog?

@anr2me
Copy link
Collaborator

anr2me commented Sep 21, 2021

Well the MatchingInput (which is listening on incoming PDP packets and generate the Events) and MatchingEvent threads was supposed to be running on PSP side, but because the code was probably ported directly from pro-online client it ended running on the emulator side.

I'm planning to change those threads into PSPThreads later, probably after 1.12 is out, as it's too risky to do it near the end of the milestone.

@ghost
Copy link

ghost commented Sep 22, 2021

Ugh.. i also found crash issue at return hleLogSuccessVerboseX(SCENET, ERROR_NET_ADHOC_WOULD_BLOCK, "would block"); of sceNetAdhocPdpRecv :(
image
I guess hleLog isn't thread-safe and shouldn't be called from another real thread.. in this case the MatchingInput thread

When that crash happens? I didn't notice any crashes...
The only crashes I remember were related to GTA and Shin Budokai 2 .

@anr2me
Copy link
Collaborator

anr2me commented Sep 22, 2021

It only crashed with the fix i made for Super Pocket Tennis (which seems to fix the issue on this game too), if you're staying in lobby for too long, since the logs will be flooded with these PdpRecv's logs, thus have a higher chance to bump into this crash issue.

@hrydgard hrydgard added this to the Future-Prio milestone Sep 22, 2021
@anr2me
Copy link
Collaborator

anr2me commented Sep 23, 2021

Hmm.. it seems the crash issue on the Log was because i'm printing the syscall's argument.

If i use DEBUG_LOG(SCENET, "%08x=sceNetAdhocPdpRecv(%i): would block", ERROR_NET_ADHOC_WOULD_BLOCK, id); it will also crashed, probably because id is part of the syscall's arg

But if i use DEBUG_LOG(SCENET, "%08x=sceNetAdhocPdpRecv: would block", ERROR_NET_ADHOC_WOULD_BLOCK); it won't crash.
This is probably why the AdhocServer and UPnPService (which runs on another threads) never crashed even when they use Logging too.

@anr2me
Copy link
Collaborator

anr2me commented Sep 23, 2021

Btw @mojojojodojo if a player joined a room and then left the room, is that player's name still appeared on the other player's list?

Like this 4th player that are no longer in the room (which i feels odd)
image
The 4th player's name will only disappeared when the 4th player is exiting multiplayer mode.
Is this also happening on PSP?

@anr2me
Copy link
Collaborator

anr2me commented Sep 23, 2021

Ugh.. i bumped into another crash issue i'm not sure what's happening
image
image
image
@hrydgard / @unknownbrackets Is this happening because i tried to allocate userMemory from a different thread? (in this case the MatchingEvent thread)
The chance for this issue to occur seems to be pretty low, where i need to stay in the game room for quite a long time and with 4 players.

@ghost
Copy link

ghost commented Sep 23, 2021

Btw @mojojojodojo if a player joined a room and then left the room, is that player's name still appeared on the other player's list?

Like this 4th player that are no longer in the room (which i feels odd)
image
The 4th player's name will only disappeared when the 4th player is exiting multiplayer mode.
Is this also happening on PSP?

When I tried it on Xlink I dont think it happened.
On xlink you cant get it to work it seems like when starting the game the game will be stuck in some sort of a loop (is this what they call infinite timeout?).
Then again i only tried with 2 players only.

@unknownbrackets
Copy link
Collaborator

Yes, PSP memory allocation is not safe to do from another thread.

In C++, everything is unsafe to do from another thread unless you've specifically done a lot of work to make it safe (and paid a performance penalty for this.) STL containers like std::vector? Unsafe. BlockAllocator? Unsafe. CoreTiming ticks? Unsafe.

The places we use threads all have to have the data communicated to and from separated so they're never cross-talking with other threads.

-[Unknown]

@ghost
Copy link

ghost commented Oct 7, 2021

After you finish 1 round on coop at least then all the names show up (even the 3rd player) so the inital lobby is some sort of bug on PPSSPP.

@anr2me
Copy link
Collaborator

anr2me commented Oct 7, 2021

After you finish 1 round on coop at least then all the names show up (even the 3rd player) so the inital lobby is some sort of bug on PPSSPP.

how many players it should be? 2 players?

i do think the AdhocMatching ping is a bit strange, it's pinging all players even when itself is not a member of AdhocMatching, causing leaving players still have their last_recv updated and other players unable to detect that player leaving the room.

I believed it should only be pinging AdhocMatching members and only done it when itself is a member too, but i'll test it for 1.13 later, since there is a risk of some games unable to see multiplayer room/lobby if my assumption was mistaken.

@ghost
Copy link

ghost commented Oct 9, 2021

The coop is 4 players.

Seeing as the game was originally on arcade maybe it made more sense there and they just ported how it works verbatim.
Could be that the arcade cabinets were always online even when playing single player or something etc.

@anr2me
Copy link
Collaborator

anr2me commented Jan 19, 2022

@shanzenos please try again after this PR got merged #15331
Or you can use the artifact from that PR to test it.

@ghost
Copy link

ghost commented Jan 19, 2022

Using localhost only 4 players will work now.
5 players and above still fail to show properly (but might work randomly ?)

@anr2me
Copy link
Collaborator

anr2me commented Jan 20, 2022

This game is using AdhocMatching intensively (similar to Lord of Arcana), thus for each additional player there will be more events being queue at a rapid pace :(

I guess i'll need to find a way to process those matchingEvents faster, or at least prevent them from interfering a syscall that waited for a specific event.

PS: i'm not sure whether my laptop can handle more than 4 instances of PPSSPP to test 8 players LOL

@ghost
Copy link

ghost commented Jan 20, 2022

Actually if you are super fast at inputting , if you access the lobby in split second time with 7 instances you can get the lobby to be almost full.
You need to make the controller to work at the same time across all the instances though.
Rush got a similar outcome here if you remember.
The game is not that demanding at the lobby so I think its feasible
Intense stuff indeed lol.

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

No branches or pull requests

6 participants