pcsx2 icon indicating copy to clipboard operation
pcsx2 copied to clipboard

Sio Phase 2.5 - The Paddening

Open RedPanda4552 opened this issue 1 year ago • 12 comments

Description of Changes

  • Completely replaces all pad code.
  • Reimplements pad protocol as discrete functions, uses an abstract PadBase with each actual controller type as a subclass, each with an independent implementation of the protocol matching that type of pad.
  • Added guitars.
  • GT4 should now properly engage DS2 mode again, so axis triggers can be used for throttle/brake controls again.
  • Other games which may have failed to engage DS2 mode should now be able to.

Rationale behind Changes

Old pad code was soup, had issues with DS2 mode.

Suggested Testing Steps

  • Verify inputs all function as expected.
  • Check stick inversion working.
  • Check using pressure (triggers on modern controllers are axes not buttons, bind these to PS2 face buttons).
  • Check guitar games working.
  • Check macros working.
  • Check settings retention between game restarts and full emulator restarts.

To-Do

  • ~~Un-scuff the guitar mapping image.~~
  • TAS is undoubtably broken.
  • Probably more things.

Casualties of War

  • PS1 mode is inoperable, SIO0 is disconnected from the pads for now.

RedPanda4552 avatar Mar 25 '23 19:03 RedPanda4552

Sky Odyssey left Analog doesn't work on the PR

Mrlinkwii avatar Mar 25 '23 20:03 Mrlinkwii

How about true crimes Los Angeles. Does it still require second controller to be in?

seta-san avatar Mar 26 '23 01:03 seta-san

How about true crimes Los Angeles. Does it still require second controller to be in?

Yes. Best I can tell that is a SIF problem made happy by the additional pad data saturating the bus. Without the extra data it seems to trip over itself.

RedPanda4552 avatar Mar 26 '23 01:03 RedPanda4552

Sky Odyssey is fixed Thanks :)

Mrlinkwii avatar Mar 26 '23 12:03 Mrlinkwii

Not sure why it causes crashes, but PadBase needs a virtual destructor and it's the reason the macOS unit tests are failing.

Clearly clang thinks you have a 100% chance of hitting UB here. If anyone knows what UB we're hitting, that would be interesting to know.

Process 9888 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
    frame #0: 0x0000000100564ac8 core_test`PadBase::~PadBase(this=0x0000600001709600) at PadBase.cpp:11:19
   8   		this->unifiedSlot = unifiedSlot;
   9   	}
   10  	
-> 11  	PadBase::~PadBase() = default;
   12  	
   13  	void PadBase::SoftReset()
   14  	{
Target 0: (core_test) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
  * frame #0: 0x0000000100564ac8 core_test`PadBase::~PadBase(this=0x0000600001709600) at PadBase.cpp:11:19
    frame #1: 0x000000010056b65b core_test`std::__1::default_delete<PadBase>::operator(this=0x00000001030bde00, __ptr=0x0000600001709600)[abi:v15006](PadBase*) const at unique_ptr.h:48:5
    frame #2: 0x000000010056b5dc core_test`std::__1::unique_ptr<PadBase, std::__1::default_delete<PadBase> >::reset[abi:v15006](this=0x00000001030bde00, __p=0x0000000000000000) at unique_ptr.h:305:7
    frame #3: 0x000000010056b579 core_test`std::__1::unique_ptr<PadBase, std::__1::default_delete<PadBase> >::~unique_ptr[abi:v15006](this=0x00000001030bde00) at unique_ptr.h:259:19
    frame #4: 0x000000010056b555 core_test`std::__1::unique_ptr<PadBase, std::__1::default_delete<PadBase> >::~unique_ptr[abi:v15006](this=0x00000001030bde00) at unique_ptr.h:259:17
    frame #5: 0x000000010056b51d core_test`std::__1::array<std::__1::unique_ptr<PadBase, std::__1::default_delete<PadBase> >, 8ul>::~array(this=0x00000001030bddc8) at array:152:29
    frame #6: 0x000000010056ad35 core_test`std::__1::array<std::__1::unique_ptr<PadBase, std::__1::default_delete<PadBase> >, 8ul>::~array(this=0x00000001030bddc8) at array:152:29
    frame #7: 0x000000010056ad55 core_test`PadManager::~PadManager(this=0x00000001030bddc8) at PadManager.cpp:44:25
    frame #8: 0x000000010056aa95 core_test`PadManager::~PadManager(this=0x00000001030bddc8) at PadManager.cpp:44:25
    frame #9: 0x00007ff80dd70c1f libsystem_c.dylib`__cxa_finalize_ranges + 409
    frame #10: 0x00007ff80dd70a39 libsystem_c.dylib`exit + 35
    frame #11: 0x00007ff80de911d3 libdyld.dylib`dyld4::LibSystemHelpers::exit(int) const + 11
    frame #12: 0x00007ff80db4733a dyld`start + 2474
(lldb) disas
core_test`PadBase::~PadBase:
    0x100564ac0 <+0>: push   rbp
    0x100564ac1 <+1>: mov    rbp, rsp
    0x100564ac4 <+4>: mov    qword ptr [rbp - 0x8], rdi
->  0x100564ac8 <+8>: ud2    
(lldb) 

TellowKrinkle avatar Apr 13 '23 02:04 TellowKrinkle

Virtual destructor added fixes Mac build, and fixed a couple regressions that popped up from rebase.

RedPanda4552 avatar Apr 22 '23 16:04 RedPanda4552

Requesting review for input recording - no clue if what I did will have broken it or not. If I understand it right, PadData.cpp is really the only thing affected and it seems okay? But I can't be 100% sure.

RedPanda4552 avatar Apr 29 '23 03:04 RedPanda4552

Requesting review for input recording - no clue if what I did will have broken it or not. If I understand it right, PadData.cpp is really the only thing affected and it seems okay? But I can't be 100% sure.

Regarding input recording, it seems they don't errate if starting from savestate right after loading any game (except for games where clock seed is very sensitive). Every tests I've done with power-on always desync at the very first moment of actual gameplay. Though even with the 'from savestate' workaround there are moments where a desync will occur.

Note that this issue is happening on the main dev builds.

kage2051 avatar May 03 '23 08:05 kage2051

Requesting review for input recording - no clue if what I did will have broken it or not. If I understand it right, PadData.cpp is really the only thing affected and it seems okay? But I can't be 100% sure.

Regarding input recording, it seems they don't errate if starting from savestate right after loading any game (except for games where clock seed is very sensitive). Every tests I've done with power-on always desync at the very first moment of actual gameplay. Though even with the 'from savestate' workaround there are moments where a desync will occur.

Note that this issue is happening on the main dev builds.

Mildly interesting, but if the behavior is consistent with the actual nightly releases, that means my changes didn't break anything... I think... It sounds more like input recording just isn't quite working right to begin with at this time?

RedPanda4552 avatar May 03 '23 13:05 RedPanda4552

Mildly interesting, but if the behavior is consistent with the actual nightly releases, that means my changes didn't break anything... I think... It sounds more like input recording just isn't quite working right to begin with at this time?

I think it has to do with CDVD timing discrepancy between playbacks rather than something wrong in how inputs are polled. This also affects recordings from savestate but they are much less frequent.

kage2051 avatar May 03 '23 19:05 kage2051

Git has made it very clear that it wants this branch to be ruined, that is twice now that it has succeeded in destroying the commits. I will try to salvage at a later time.

Also to-do: Fix duplicate includes on a bunch of files, because, surprise, Git can't handle a rebase without adding a hundred garbage merge conflicts.

RedPanda4552 avatar Jun 01 '23 04:06 RedPanda4552

Fixes triggers and the directional analog in Le Mans 24 Hours - triggers are now pressure sensitive and the analog actually works.

CookiePLMonster avatar Jun 11 '23 16:06 CookiePLMonster

Just seen this commit in Crowdin and the "Analog light is now {} for port {} / slot {}" string at pcsx2/SIO/Pad/PadDualshock2.cpp:644 needs two custom "On"/"Off" strings so non-English languages can match the gender/number of "Analog light".

IlDucci avatar Jul 26 '23 09:07 IlDucci