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

Original game bug: Gamepads with more than 15 buttons can't bind buttons #104

Closed
CookiePLMonster opened this issue Sep 12, 2023 · 18 comments

Comments

@CookiePLMonster
Copy link

CookiePLMonster commented Sep 12, 2023

Disclaimer: I don't know if this is fixed in the project currently, I was only pointed at it when I tweeted about the original game bug with gamepad bindings. It goes as follows:

  1. The game polls gamepad button like so - notice the presence of a for loop with a signed int16_t acting as the "number of buttons" (sub_404360 in my executable):
    if ( *(_WORD *)((char *)&word_512F34 + v17 + 2) )
    {
      v3 = PollJoystickButtons(v22, a1);
      if ( v3 )
      {
        v4 = v20;
        v5 = 1;
        for ( i = 0; i < *(int *)((char *)&word_512F34 + v17) >> 16 && !v1; ++i )
        {
          if ( (v5 & v3) != 0 )
            v1 = (v4 & 0xFFFFFF00) + 2;
          else
            v5 *= 2;
          v4 += 256;
        }
        while ( PollJoystickButtons(v22, v5) )
          sub_483EB0();
      }
  1. This int16 is populated with... a button bitmask populated by calling IDirectInputDevice::GetObjectInfo in a loop, like so (sub_421BD0 in my executable):
      v9 = 48;
      do
      {
        if ( v8 >= 32 )
          break;
        v10 = *(int *)&v25[1] >> 24;
        if ( !pDirectInputJoystick[*(int *)&v25[1] >> 24]->lpVtbl->GetObjectInfo(
                pDirectInputJoystick[*(int *)&v25[1] >> 24],
                &v18,
                v9,
                1) )
        {
          v11 = (1 << v8) | dword_4E7388[23 * v10];
          dword_4E7388[23 * v10] = v11;
          if ( a2 )
            *a2 = v11;
          ++*(_DWORD *)v25;
        }
        ++v9;
        ++v8;
      }
      while ( *(_DWORD *)v25 < v22 );

The problem is - since this int16 representing the button count gets populated with a button bitmask, gamepads reporting 16 or more buttons stash a value FFFF in that count, that then gets interpreted as -1 by the for loop. The result - gamepad buttons cannot be bound at all.

I see two ways to fix it:

  1. If you can modify the original game code - modify the code snippet from 2. so dword_4E7388 becomes a button count and not a button bitmask. This is the approach I'm going to take in my standalone fix.
  2. If you absolutely cannot modify the original game code - spoof DirectInput capabilities and adjust gamepad's GetObjectInfo wrappers to never return more than 15 buttons. This will ensure that the button bitmask is at most 7FFF and therefore avoid underflow. I kind of recommend against this approach, because the game will still potentially do an equivalent of for ( i = 0; i < 32767; i++ ) every time it checks button presses.
@zaps166
Copy link
Owner

zaps166 commented Sep 16, 2023

I remember that I've made a workaround and you can set it in config file.

If you know what's going on - could you make a PR here ?

@CookiePLMonster
Copy link
Author

Sure - what are the "rules" for making code changes there? Does the binary size have to stay constant or can it change? My patch is going to make the affected function 2 or 3 bytes shorter, unless I pad it with nops.

@zaps166
Copy link
Owner

zaps166 commented Sep 16, 2023

A few bytes / kilobytes doesn't matter. Do you change ASM only or C code, too?

@CookiePLMonster
Copy link
Author

I can change both if that helps, it's a simple change (turning a bitwise OR into an increment).

@zaps166
Copy link
Owner

zaps166 commented Sep 16, 2023

I thought about C wrapper, but C++ for ARM can be patched too - it's auto-generated, but if it's simple, you can do it manually 🙂

After your patch, the config file might be changed, because game will read all buttons - am I correct?

@CookiePLMonster
Copy link
Author

After your patch, the config file might be changed, because game will read all buttons - am I correct?

Yep, 32 buttons aka the limit of the DInput version they used.

@CookiePLMonster
Copy link
Author

I thought about C wrapper, but C++ for ARM can be patched too - it's auto-generated, but if it's simple, you can do it manually 🙂

In this case other than the ASM repo, where else should I apply this change?

@zaps166
Copy link
Owner

zaps166 commented Sep 16, 2023

Here if possible

@CookiePLMonster
Copy link
Author

Both are done, although I've not tested either. Considering how simple this change is and that it's been field-tested via a runtime patch, I would guess it's all fine.

@zaps166
Copy link
Owner

zaps166 commented Oct 1, 2023

Thanks, currently I don't have any device with > 15 buttons to test.

@CookiePLMonster
Copy link
Author

Me neither - but on Windows, an Xbone gamepad claims to have 16 buttons (despite having less physically), triggering the issue.

@zaps166
Copy link
Owner

zaps166 commented Mar 2, 2024

It's handled here and here. I have to check whether your patch works and remove all limits from my C code.

I have also numAxes limited to 4 (don't remember why, probably similar issue). Does your PR can fix this issue, too?

@CookiePLMonster
Copy link
Author

Does your PR can fix this issue, too?

Unlikely.

@zaps166
Copy link
Owner

zaps166 commented Mar 2, 2024

Unlikely.

I see.

  • axis 0, 1, 2, 5 - works
  • axis 3, 4 - doesn't work.

@zaps166
Copy link
Owner

zaps166 commented Mar 2, 2024

Thanks for the PR!

@zaps166 zaps166 closed this as completed Mar 2, 2024
@CookiePLMonster
Copy link
Author

zaps166/NFSIISE-CPP#4 can probably also be merged.

@zaps166
Copy link
Owner

zaps166 commented Mar 3, 2024

zaps166/NFSIISE-CPP#4 can probably also be merged.

Merged, but this code is auto-generated 😄


Also axis 6 and 7 are working, I didn't know 😅 So we have 6 working axis!

@CookiePLMonster
Copy link
Author

Merged, but this code is auto-generated 😄

Ah, back then I misunderstood what did you mean by "patching C code". Either way, it's all solved now. Thanks!

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

2 participants