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

OSK overhaul: Adjust button functionality, and some text positioning. #3922

Merged
merged 5 commits into from
Sep 25, 2013

Conversation

thedax
Copy link
Collaborator

@thedax thedax commented Sep 25, 2013

This pullrq makes the OSK much, much more convenient to use in the following ways:

  • Makes select function as a real shift button(meaning it swaps upper/lowercase)
  • Makes the R trigger cycle forward through all available keyboard languages
  • Makes the L trigger cycle backward through all available keyboard languages
  • Moves text around so it looks better
  • Makes tasks like Valkyria Chronicles 2 passwords problems. #3915 far easier(via the select button functionality change)

Screenshot 01

…er/lower case; in the case of Korean, it does nothing. In the case of Japanese, it swaps Katakana/Hiragana). R Trigger now acts as a language selector, cycling through the 5 keyboard languages(English, English full-width, Japanese, Korean, Russian).
@thedax
Copy link
Collaborator Author

thedax commented Sep 25, 2013

Should I perhaps move X & O closer to the Start/Select group?

@thedax
Copy link
Collaborator Author

thedax commented Sep 25, 2013

Also, please hold off on merging this, I want to add one more thing.

@Ritori
Copy link

Ritori commented Sep 25, 2013

Why don't L and R switch between language :)?

@thedax
Copy link
Collaborator Author

thedax commented Sep 25, 2013

I see no reason to make L do it. R cycles languages already.

@Ritori
Copy link

Ritori commented Sep 25, 2013

Umm I though it became more easy to switch other language on android and window?

@thedax
Copy link
Collaborator Author

thedax commented Sep 25, 2013

It does..

Consider this:

Game asks for OSK -> OSK starts off in English -> You press R -> It switches to Japanese -> Press R again -> Switches to Korean.

I don't get where the confusion is.

@Ritori
Copy link

Ritori commented Sep 25, 2013

That see quick and don't get confused. I agreed with this though :)
but liked this is fine?
OSK starts off in English -> press R -> Japanese -> Press L -> English.

It more quick to change this:-
English -> Press L -> English full-width -> Press R -> English

@solarmystic
Copy link
Contributor

Good stuff as always @thedax.

Got confused by the "R trigger" in the description, and then realized you're referring to the R button itself, since the PSP doesn't have a R2 button (or what we'd commonly refer to as the RT (Right/R Trigger) for the X360 pad).

I've no complaints.

@thedax
Copy link
Collaborator Author

thedax commented Sep 25, 2013

I'm adding in some code for the L trigger as well to cycle backward.

@thedax
Copy link
Collaborator Author

thedax commented Sep 25, 2013

Okay, I'm finished with this now, unless more suggestions are made.

@Ritori
Copy link

Ritori commented Sep 25, 2013

Thank @thedax sorry for my suggestions though.

hrydgard added a commit that referenced this pull request Sep 25, 2013
Osk overhaul: Adjust button functionality, and some text positioning.
@hrydgard hrydgard merged commit 0f67666 into hrydgard:master Sep 25, 2013
@thedax
Copy link
Collaborator Author

thedax commented Sep 25, 2013

@Ritori: It's fine. It made the OSK even better.
@solarmystic: Thanks.

@solarmystic
Copy link
Contributor

@thedax

I'm getting compile errors with MSVC 2010 x64 release after this pull request was merged to master.

Perhaps you didn't take into account Headless.

Error   22  error LNK2019: unresolved external symbol "class std::map<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >,struct std::pair<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >,int>,struct std::less<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > >,class std::allocator<struct std::pair<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const ,struct std::pair<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> >,int> > > > __cdecl GetLangValuesMapping(void)" (?GetLangValuesMapping@@YA?AV?$map@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@U?$pair@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@H@2@U?$less@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@2@V?$allocator@U?$pair@$$CBV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@U?$pair@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@H@2@@std@@@2@@std@@XZ) referenced in function "public: virtual int __cdecl PSPOskDialog::Init(unsigned int)" (?Init@PSPOskDialog@@UEAAHI@Z)    D:\Programming\ppsspp\headless\Core.lib(PSPOskDialog.obj)
Error   23  error LNK1120: 1 unresolved externals   D:\Programming\ppsspp\Windows\x64\Release\PPSSPPHeadless.exe    1

The actual ppsspp executable compiles nicely though, just a heads up, since the buildbot may be broken.

@thedax
Copy link
Collaborator Author

thedax commented Sep 25, 2013

Heh, guess that trick won't work this time. Alright, fix coming up soon.

@thedax thedax mentioned this pull request Sep 25, 2013
@@ -912,8 +945,11 @@ int PSPOskDialog::Update()
}
else if (IsButtonPressed(CTRL_SELECT))
{
// TODO: Limit by allowed keyboards...
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know that removing this comment makes people happy (must be since every time anyone touches the code they remove it), but I think we probably still should limit by the keyboards the game specifies as allowed within the parameter struct.

(doing that would also mean that VC2's password would just work by default, no keyboard swapping required, just like on the PSP.)

-[Unknown]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@unknownbrackets: I saw that previous issue which you commented on, and it looks like a nightmare to implement, though..

I might look at it in more detail sometime, but for now, this should work as a stopgap.

Leaving this here as a reminder(for whom? Well, either myself, or if someone beats me to implementing it): #2281 (comment)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, whether it looks hard or not, that doesn't seem like a reason to remove the comment from the code.

-[Unknown]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll add it back in then, in my next pullrq.

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.

5 participants