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

LilyPad: Updates #1831

Merged
merged 5 commits into from Mar 16, 2017

Conversation

Projects
None yet
3 participants
@FlatOutPS2
Member

FlatOutPS2 commented Feb 16, 2017

Adds 4 new commits:

  • Improve analog/pressure sensitive detection:

Improves detection of analog or pressure sensitive support, which previously made some games unable to detect the correct mode.

  • Add Quick Setup:

Adds quick setup that allows for quickly setting up all the default pad
buttons without having to click on each one separately.

  • Display a warning when all controls are inactive:

Displays a console warning if no controls/controllers are active and the
emulation cannot be controlled.

  • General fixes and UI improvements

Some general fixes and UI improvements. And adds Restore Defaults option.

@turtleli

I'm not sure it's a good idea having an additional "page" for special inputs. The dialog could always be made wider.

@@ -187,6 +187,8 @@ struct PadFreezeData
// Digital / Analog / DS2 Native
u8 mode;
u8 previousType;

This comment has been minimized.

@turtleli

turtleli Feb 24, 2017

Member

Save state version needs to be incremented if the save state format is changed.

plugins/LilyPad/LilyPad.cpp Outdated
Pad *pad = &pads[query.port][query.slot];
int padtype = config.padConfigs[query.port][query.slot].type;
if (padtype != pad->previousType) {

This comment has been minimized.

@turtleli

turtleli Feb 24, 2017

Member

Under what situations does this occur? I can think of changing the pad type in-game, but are there other situations?

This comment has been minimized.

@FlatOutPS2

FlatOutPS2 Feb 24, 2017

Member

There are no other situations this currently occurs under, and any new situation would be handled elsewhere, so I've moved it to PADopen.

This comment has been minimized.

@turtleli

turtleli Feb 26, 2017

Member

What about loading a savestate?

plugins/LilyPad/LilyPad.cpp Outdated
lockStateChanged[port][slot] |= LOCK_DIRECTION;
} else if (cmd == 0x0D) {
} else if (cmd == 0x2C) { // Lock Direction

This comment has been minimized.

@turtleli

turtleli Feb 24, 2017

Member

Comments and code do not match, so it's likely both are wrong (Lock input and Lock direction seem to have been swapped).

plugins/LilyPad/LilyPad.cpp Outdated
@@ -531,6 +531,8 @@ void Update(unsigned int port, unsigned int slot)
dm->Update(&info);
static int rapidFire = 0;
rapidFire++;
static bool anyDevActiveAndBound = true;
bool currentDevActiveAndBound = false;

This comment has been minimized.

@turtleli

turtleli Feb 24, 2017

Member

Can this stuff be moved to PADOpen? (I don't know whether it's possible).

Also, device could be spelt out in full in the variable name - my first thought was dev = developer, which makes for some strange reading.

This comment has been minimized.

@FlatOutPS2

FlatOutPS2 Feb 24, 2017

Member

No, it shouldn't be moved as a new pad can (dis)connect at other times than just when PADopen is triggered.

I actually used dev because of the code below Device *dev, but I've changed it to avoid the confusion.

@@ -1580,6 +1580,9 @@ keyEvent *CALLBACK PADkeyEvent()
#ifdef _MSC_VER
static char shiftDown = 0;
static char altDown = 0;
if (!activeWindow)
altDown = shiftDown = 0;

This comment has been minimized.

@turtleli

turtleli Feb 24, 2017

Member

Is this related to #1797?

This comment has been minimized.

@FlatOutPS2

FlatOutPS2 Feb 24, 2017

Member

Yes, I forgot to add that to the commit message. It fixes the issue.

This comment has been minimized.

@turtleli

turtleli Feb 26, 2017

Member

This change should probably be in a separate commit.

@FlatOutPS2 FlatOutPS2 force-pushed the FlatOutPS2:Lily branch 2 times, most recently Feb 24, 2017

@FlatOutPS2

This comment has been minimized.

Member

FlatOutPS2 commented Feb 24, 2017

I'm not sure it's a good idea having an additional "page" for special inputs. The dialog could always be made wider.

I don't want to make the dialog wider. As far as I'm concerned it's as wide as it should be. And there will be more special inputs added later that would further increase the required space. It also avoids confusion between the default pad inputs and any extra inessential ones.

@turtleli

This comment has been minimized.

Member

turtleli commented Feb 26, 2017

Another thing - the "General fixes and UI improvement" commit message is lacking a bit of detail. It doesn't mention the addition of the "restore defaults" button, nor does it mention the double click change.

I don't want to make the dialog wider. As far as I'm concerned it's as wide as it should be. And there will be more special inputs added later that would further increase the required space. It also avoids confusion between the default pad inputs and any extra inessential ones.

Personally I still think it could it be wider. But anyway, I think it would probably be nicer to have a tab or something instead of a "Go to Special Inputs button" and "Back to Controls" button.

@FlatOutPS2 FlatOutPS2 force-pushed the FlatOutPS2:Lily branch 2 times, most recently Mar 10, 2017

@FlatOutPS2

This comment has been minimized.

Member

FlatOutPS2 commented Mar 12, 2017

@turtleli I've changed the Special Inputs, added the missing details to the commit message and made a seperate commit for the alt-tab issue fix as requested.

@FlatOutPS2 FlatOutPS2 added this to the Release 1.6 milestone Mar 12, 2017

plugins/LilyPad/Config.cpp Outdated
sensitivity = BASE_SENSITIVITY;
if (!sensitivity) {
if (((uid >> 16) & 0xFF) == ABSAXIS) {
sensitivity = 87183;

This comment has been minimized.

@turtleli

turtleli Mar 13, 2017

Member

What does this number mean?

This comment has been minimized.

@FlatOutPS2

FlatOutPS2 Mar 13, 2017

Member

87183 is the default sensitivity for the analog sticks in default.ini. But if you created a new binding for analog sticks you always got BASE_SENSITIVITY(which results in 65536) in previous LilyPad versions.

This comment has been minimized.

@turtleli

turtleli Mar 13, 2017

Member

You should probably use a constant to represent this (maybe DEFAULT_ANALOG_SENSITIVITY or any other suitable name).

plugins/LilyPad/Config.cpp Outdated
if (i == IDC_SLIDER_SENSITIVITY) {
int val = BASE_SENSITIVITY;
if (((control->uid >> 16) & 0xFF) == ABSAXIS) {
val = 87183;

This comment has been minimized.

@turtleli

turtleli Mar 13, 2017

Member

Ditto.

plugins/LilyPad/Config.cpp Outdated
break;
case IDYES:
static char iniLocation[MAX_PATH * 2] = "inis/LilyPad.ini";
remove(iniLocation);

This comment has been minimized.

@turtleli

turtleli Mar 13, 2017

Member

Would it be sufficient to just use iniFile here?

This comment has been minimized.

@FlatOutPS2

FlatOutPS2 Mar 13, 2017

Member

No, it didn't convert properly.

This comment has been minimized.

@turtleli

turtleli Mar 14, 2017

Member

Ok. Does iniLocation need to be static?

@@ -1217,23 +1227,14 @@ u8 CALLBACK PADpoll(u8 value)
switch (value) {
// CONFIG_MODE
case 0x43:
if (pad->config) {

This comment has been minimized.

@turtleli

turtleli Mar 13, 2017

Member

I don't get why you moved the code here into an if/else in the following switch case. Couldn't it just be:

case 0x43:
    if (pad->config && padtype != neGconPad) {
        // blah blah
        return whatever;
    }
    // Fallthrough
case 0x42:

FlatOutPS2 added some commits Feb 1, 2017

LilyPad: Improve analog/pressure sensitive detection
Improves detection of analog or pressure sensitive support, which previously made some games unable to detect the correct mode.
LilyPad: Add Quick Setup
Adds quick setup that allows for quickly setting up all the default pad
buttons without having to click on each one separately.

Hides special inputs(inputs that aren't available on a PS(2) controller) by default.
LilyPad: Display a warning when all controls are inactive
Displays a console warning if no controls/controllers are active and the
emulation cannot be controlled.
LilyPad: Fix F4 button (PCSX2 FrameLimiter toggle) getting blocked
Fixes issue where losing focus after pressing alt-tab would end up
blocking the F4 button (which is used for turning the PCSX2 FrameLimiter
on and off).

Fixes: #1797
LilyPad: General fixes and UI improvements
Some general fixes and UI improvements.

Adds Reset Configuration to Input/Force Feedback configuration screens that resets the configuration for the selected control(s).

Adds Restore Defaults button to the General tab that deletes all LilyPad
Settings and bindings and resets to the default settings.

Adds double-click functionality to the PAD list on the General tab,
which will now send the user straight to the corresponding PAD tab.

@FlatOutPS2 FlatOutPS2 force-pushed the FlatOutPS2:Lily branch to 19a7b42 Mar 16, 2017

@@ -1342,7 +1350,7 @@ u8 CALLBACK PADpoll(u8 value)
// QUERY_DS2_ANALOG_MODE
case 0x41:
// Right? Wrong? No clue.
if (pad->mode == MODE_DIGITAL || pad->mode == MODE_PS1_MOUSE) {
if (pad->mode == MODE_PS1_MOUSE || pad->mode == MODE_NEGCON) {

This comment has been minimized.

@turtleli

turtleli Mar 16, 2017

Member

pad->mode == MODE_DIGITAL has been removed. Is this intentional?

Apart from that, the code seems fine.

This comment has been minimized.

@FlatOutPS2

FlatOutPS2 Mar 16, 2017

Member

Yes, that is very much intentional. It was one of the reasons why some games didn't enable analog/pressure sensitive mode.

@turtleli turtleli merged commit f3a89f5 into PCSX2:master Mar 16, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pgert

This comment has been minimized.

Contributor

pgert commented Apr 14, 2017

One of these commits caused "Network Access Disc" to become unresponsive to DS2 input.

turtleli added a commit that referenced this pull request Jul 24, 2017

lilypad: Fix button detection issue in Kung Fu Panda
Kung Fu Panda becomes stuck at an autosave warning screen since it
cannot detect button presses correctly. This fixes the issue, though
potentially it may negatively affect some other games (see #1831,
unfortunately no game titles weren't mentioned).

The issue was introduced in commit 3075ec2

Closes #1965.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment