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

Implementing the use of the touchscreen as camera movement #1896

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

vitor251093
Copy link

Basically, this changes allows a user to use keys (or an analog joystick) to move the camera by using the touchscreen.

This method, ofc, is only useful for games which allow you to use the touchscreen to move the camera, such as Kingdom Hearts 358/2 Days.

There is still room for improvement. The movement could be smoother, and, when using an analog stick, the camera movement could move more or less depending on how tilted is the analog stick. Still, this implementation is already useful as it is.

src/SPI.cpp Outdated
@@ -462,6 +462,30 @@ void TSC::SetTouchCoords(u16 x, u16 y)
NDS.KeyInput &= ~(1 << (16+6));
}

void TSC::MoveTouchCoords(u16 x, u16 y) // 0 -> negative, 1 -> neutral, 2 -> positive
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think you could make this an enum instead of a series of magic numbers? I could easily see some poor bastard trying to pass mouse coordinates in here, wondering why nothing works.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Any preferable place where I should place the enum?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SPI.h would be my first instinct.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enum added, but now I'm wondering if replacing x and y enums with left, top, right and down booleans wouldn't make more sense lol

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think @JesseTG ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry, I didn't think you were asking me the first time.

No, keep the two-argument version; the "cursor" can move along both axes, right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct

@@ -121,6 +121,11 @@ void DSi_TSC::SetTouchCoords(u16 x, u16 y)
}
}

void DSi_TSC::MoveTouchCoords(SPITouchScreenMovement x, SPITouchScreenMovement y)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason that DSi_TSC's override of this method does nothing?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry, I forgot to mention that.

I've only tested the touchscreen camera controls with the DS mode, but not with the DSi mode. Since the implementation of SetTouchCoords was different for each of them, I decided to include that override to avoid unexpected behaviours. At the time I had implemented that only for KH Days, so there was no use in implementing that function either.

I can make some tests with the DSi mode to implement that function, but that may take a little while.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think that would be for the best.

@RSDuck
Copy link
Member

RSDuck commented Dec 3, 2023

why does this touch the core at all, looks to me like something which could be implemented on the frontend side only?

@vitor251093
Copy link
Author

vitor251093 commented Dec 3, 2023

why does this touch the core at all, looks to me like something which could be implemented on the frontend side only?

Because the input only allow me to update an existing coordinate within the touchscreen, requiring me to know the previous value, while TouchX and TouchY are protected values.

So I either needed to add a function to SPI which updates TouchX and TouchY based on the movement (which was my approach), or I would need to make TouchX and TouchY public (so I could extract that logic to NDS.cpp).

EDIT: It's also worth noting that, based in the DSi SPI class, TouchX and TouchY have different possible values for the DS and the DSi (the DSi ones also include the touch/release states). With that, obtaining TouchX and TouchY would actually be a problem, so I would need to maintain two new variables at NDS.h/NDS.cpp, or main.cpp, instead.

I can update the pull request to achieve that approach if you want. But like I told Jesse before, this may take a little while.

@CasualPokePlayer
Copy link
Contributor

CasualPokePlayer commented Dec 3, 2023

With that, obtaining TouchX and TouchY would actually be a problem, so I would need to maintain two new variables at NDS.h/NDS.cpp, or main.cpp, instead.

It wouldn't really be that hard to just mask out stuff you don't need, a simple & ~0x8000 and checking if the value is 0x700 (and probably checking if y == 0xFFF for DS mode) would work (or maybe a getter would do most of that for you)

@JesseTG
Copy link
Contributor

JesseTG commented Dec 3, 2023

or I would need to make TouchX and TouchY public (so I could extract that logic to NDS.cpp).

Is there a reason you can't add GetTouchX() and GetTouchY() to expose these members without letting them be modified arbitrarily?

@vitor251093
Copy link
Author

vitor251093 commented Dec 3, 2023

or I would need to make TouchX and TouchY public (so I could extract that logic to NDS.cpp).

Is there a reason you can't add GetTouchX() and GetTouchY() to expose these members without letting them be modified arbitrarily?

I will try to do that, following @CasualPokePlayer 's suggestion. Seems to be an approach that will fit all the needs we talked about.

I will leave a comment here once I have time to code, and test, the needed changes.

@vitor251093
Copy link
Author

Alright, just finished implementing the changes.

The SPIs now were only changed to add the getters, and the logic that converts inputs into touchscreen movements is now contained within NDS::SetTouchKeyMask.

Also now it can also detect every scenario when the coordinates becomes out of bounds, which is then considered as the touchscreen being released, so the movement can restart on the next frame starting from the middle of the touchscreen.

@vitor251093
Copy link
Author

I just realized there was a conflict. Fixed it

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

Successfully merging this pull request may close these issues.

None yet

4 participants