search

jpd002 / Play-

Play! - PlayStation2 Emulator
http://purei.org
Other
2.03k stars 249 forks source link

McServe fixes for Champions Return To Arms #1330

Closed bigianb closed 5 months ago

bigianb commented 5 months ago

fixes issue #1329

jpd002 commented 5 months ago

Thanks for this!

I'm gonna to some investigation on my side, but I think the first part that sets the contents of the paramAddress makes sense, it seems to be updated regardless of the result of the Read operation: https://github.com/ps2dev/ps2sdk/blob/7efb3b409b9c12067f9455de97fa5190b6bd1037/iop/memorycard/mcserv/src/mcserv.c#L515

jpd002 commented 5 months ago

Also, did you try returning -5 when the game tries to read a invalid fd? That's what seems to happen here: https://github.com/ps2dev/ps2sdk/blob/master/iop/memorycard/mcman/src/main.c#L749.

bigianb commented 5 months ago

Yes - tried that first. The RTA code treats -4 and -5 equivalently. It’s troubling because -4 or -5 is really the correct response but RTA does not like it. It could be the chdir fix … it may work without that. The SDK docs say if a chdir fails then the current directory is set to the root. The way to test this would be to write a little ps2 program to log the return values and run it on pcsx2 with a real bios and then verify play does the same.

bigianb commented 5 months ago

Bit more info. When the RPC returns for the Read command on the EE side, this function is always called: https://github.com/ps2dev/ps2sdk/blob/7efb3b409b9c12067f9455de97fa5190b6bd1037/ee/rpc/memorycard/src/libmc.c#L197C9-L197C9 So, if we don't initialise the first 2 elements to zero then it will have garbage in it and that function will overwrite random memory.

jpd002 commented 5 months ago

I tried on PCSX2 and the commands sent to McServ are the same as Play!. The last open command seems to work properly even though there was a failed ChDir command sent, so, I'm not sure a ChDir failure resets the current directory.

I tried only with the proper clearing up of the paramAddress area and it seems to work fine on my end. So, maybe that's all we need? Though you mentioned in the issue that it was still causing issues on your end (or maybe you had the ChDir change while you were testing that).

bigianb commented 5 months ago

I tried on PCSX2 and the commands sent to McServ are the same as Play!. The last open command seems to work properly even though there was a failed ChDir command sent, so, I'm not sure a ChDir failure resets the current directory.

I tried only with the proper clearing up of the paramAddress area and it seems to work fine on my end. So, maybe that's all we need? Though you mentioned in the issue that it was still causing issues on your end (or maybe you had the ChDir change while you were testing that).

I think I agree - smaller change is better and I think the docs are wrong with the failed chdir. I'm writing a little bit of home-brew to test that assumption ... which is a bit painful as I can't get the ps2 sdk to compile on my m1 Macbook pro so I'm using my x86 iMac.Feel free to edit this PR as you like or push your own change and I will rebase on that.

jpd002 commented 5 months ago

I've pushed the change to move the paramAddress area clearing.

For writing homebrew, I think using ps2dev's docker image makes the process a lot easier. It runs a bit slower on Apple Silicon Macs, but it's still pretty good if you wanna write simple things.

bigianb commented 5 months ago

I've pushed the change to move the paramAddress area clearing.

For writing homebrew, I think using ps2dev's docker image makes the process a lot easier. It runs a bit slower on Apple Silicon Macs, but it's still pretty good if you wanna write simple things.

thanks for the hint. It's more fun to get it running natively on the M1 ... but life is too short. I'll have to learn about docker :)