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

NEW: Added support for F13 to F24 keys #2075

Merged
merged 16 commits into from
Feb 25, 2025
Merged

Conversation

bmalrat
Copy link
Collaborator

@bmalrat bmalrat commented Dec 4, 2024

Description

https://jira.unity3d.com/browse/UUM-44328

  • Added support of F13-F24 keys for enabled editors(6000.0.38f1, 6000.1.0a10)
  • Changed enum value Key.IMESelected to obsolete which was not a real key. Please use the ButtonControl imeSelected.
  • Changed the version to 1.4 to accept api changes.

Testing status & QA

tested on mac/windows/linux

Overall Product Risks

Please rate the potential complexity and halo effect from low to high for the reviewers. Note down potential risks to specific Editor branches if any.

  • Complexity:
  • Halo Effect:

Comments to reviewers

Please describe any additional information such as what to focus on, or historical info for the reviewers.

Checklist

Before review:

  • [ X] Changelog entry added.
    • Explains the change in Changed, Fixed, Added sections.
    • For API change contains an example snippet and/or migration example.
    • JIRA ticket linked, example (case %%). If it is a private issue, just add the case ID without a link.
    • Jira port for the next release set as "Resolved".
  • [X ] Tests added/changed, if applicable.
    • Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
    • Performance tests.
    • Integration tests.
  • [X ] Docs for new/changed API's.
    • Xmldoc cross references are set correctly.
    • Added explanation how the API works.
    • Usage code examples added.
    • The manual is updated, if needed.

During merge:

  • Commit message for squash-merge is prefixed with one of the list:
    • NEW: ___.
    • FIX: ___.
    • DOCS: ___.
    • CHANGE: ___.
    • RELEASE: 1.1.0-preview.3.

After merge:

  • Create forward/backward port if needed. If you are blocked from creating a forward port now please add a task to ISX-1444.

@bmalrat bmalrat force-pushed the uum-44328-fix-f13-f14-f15 branch from aacee0a to 0db68be Compare December 13, 2024 19:56
@bmalrat bmalrat force-pushed the uum-44328-fix-f13-f14-f15 branch from 0db68be to a0bec17 Compare January 31, 2025 20:28
@bmalrat bmalrat force-pushed the uum-44328-fix-f13-f14-f15 branch from a0bec17 to 3d98e5d Compare February 3, 2025 18:41
@bmalrat bmalrat force-pushed the uum-44328-fix-f13-f14-f15 branch from e01e2a2 to b081c2c Compare February 14, 2025 14:59
@bmalrat bmalrat requested a review from ekcoh February 17, 2025 17:20
@bmalrat bmalrat marked this pull request as ready for review February 17, 2025 19:13
@bmalrat bmalrat requested a review from Pauliusd01 February 17, 2025 19:13
/// Don't use this. This is a dummy key that is only used internally to represent the IME selected state.
/// Will be removed in the future.
/// </summary>
IMESelected,
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 can add an [Obsolete(..., false)] do this one to generate a warning, to discourage usage
the package api validation tool accept it : https://unity-ci.cds.internal.unity3d.com/job/47020274
I only need to add an exception in our test APIVerificationTests.
Let me know your opinion, IMAO we should add it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If it's going to need exception in APIVerificationTests if marked obsolete maybe leave it as IMO. However we should get a ticket filed on removing this one.

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 pushed a commit on a test branch d09d546#diff-9de7ba7ca03e154a8f9bb4204633a956e529e0a8cb309c70ab7be6f3c28bc4dd it seems pretty trivial and would prevent people from using it.
let me know

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great, yeah that looks clean, let's get that in then. (I will assume this happens without additional review/approval since you provided a reference)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Feel free to resolve this thread when you have read it

@ekcoh
Copy link
Collaborator

ekcoh commented Feb 18, 2025

@bmalrat Looks like there is a bunch of conflicts on the branch in assembly files and generated precompiled layouts that need to be addressed?

Copy link
Collaborator

@ekcoh ekcoh left a comment

Choose a reason for hiding this comment

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

Multiple conflicts in precompiled layouts and assembly info files needs to be addressed

…f14-f15

# Conflicts:
#	Assets/Samples/InGameHints/InGameHintsActions.cs
#	Assets/Samples/SimpleDemo/SimpleControls.cs
#	Assets/Tests/InputSystem/InputActionCodeGeneratorActions.cs
#	Packages/com.unity.inputsystem/InputSystem/AssemblyInfo.cs
#	Packages/com.unity.inputsystem/InputSystem/Devices/Precompiled/FastKeyboard.cs
#	Packages/com.unity.inputsystem/InputSystem/Devices/Precompiled/FastMouse.cs
#	Packages/com.unity.inputsystem/InputSystem/Devices/Precompiled/FastTouchscreen.cs
#	Packages/com.unity.inputsystem/Tests/TestFixture/AssemblyInfo.cs
#	Packages/com.unity.inputsystem/package.json
@bmalrat
Copy link
Collaborator Author

bmalrat commented Feb 18, 2025

Multiple conflicts in precompiled layouts and assembly info files needs to be addressed
should be fixed.

Copy link
Collaborator

@Pauliusd01 Pauliusd01 left a comment

Choose a reason for hiding this comment

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

Seemed fine to me, able to use the additional function keys in scripts just fine (Both with Input manager and Input System versions) as well as in the shortcut manager

@ekcoh
Copy link
Collaborator

ekcoh commented Feb 21, 2025

Seems conflicts are resolved now so will approve this PR. The suggested obsoletion mentioned in one of the threads makes sense to add @bmalrat but I will approve this upfront assuming that part is included in the merge.

@bmalrat bmalrat merged commit d395aa1 into develop Feb 25, 2025
94 checks passed
@bmalrat bmalrat deleted the uum-44328-fix-f13-f14-f15 branch February 25, 2025 01:26
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.

4 participants