Skip to content

Conversation

@jfreire-unity
Copy link
Collaborator

Description

Adds the test setup for the InputSystemProvider that is used on the 2023.2+ InputForUI module.

Changes made

Sets up the Unity project to test the InputForUI module with the InputSystemProvider implementation.
Adds a basic test with a mouse to validate that the test setup is working and that events are being dispatched.

Notes

This depends on #1679 and should not be merged before

Checklist

Before review:

  • Changelog entry added.
    • Explains the change in Changed, Fixed, Added sections.
    • For API change contains an example snippet and/or migration example.
    • FogBugz ticket attached, example ([case %number%](https://issuetracker.unity3d.com/issues/...)).
    • FogBugz is marked as "Resolved" with next release version correctly set.
  • Tests added/changed, if applicable.
    • Functional tests : 'Plugins/InputForUITests.cs`

During merge:

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

@jfreire-unity jfreire-unity changed the base branch from develop to dmytro/common-input-api-for-ui May 19, 2023 15:26
Copy link
Collaborator

@lyndon-unity lyndon-unity left a comment

Choose a reason for hiding this comment

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

Looks like a sensible addition.
Obviously wait for the main provider to land (or merge into that branch)

@lyndon-unity
Copy link
Collaborator

That said this is failing tests which are related so it looks like it needs a fix

Copy link
Collaborator

@lyndon-unity lyndon-unity left a comment

Choose a reason for hiding this comment

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

Tests are failing ATM

error CS0122: 'EventProvider' is inaccessible due to its protection level

@jfreire-unity
Copy link
Collaborator Author

@lyndon-unity Yeah, this PR requires some trunk changes to land first.
Currently it's also a Draft PR. Once changes land in trunk, I'll re-run the tests and ask again for review.

@jfreire-unity jfreire-unity marked this pull request as ready for review May 23, 2023 08:28
Base automatically changed from dmytro/common-input-api-for-ui to develop May 23, 2023 10:53
@jfreire-unity jfreire-unity force-pushed the input-for-ui/add-automated-tests branch from 890bc6d to 7063aa7 Compare May 23, 2023 12:01
@jfreire-unity jfreire-unity mentioned this pull request May 23, 2023
4 tasks
Copy link
Collaborator

@lyndon-unity lyndon-unity left a comment

Choose a reason for hiding this comment

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

Seems reasonable, happy to see test coverage expanding

// Start touch and move mouse to the same position to replicate the issue of duplicated Mouse events for
// Touch events on Windows.
BeginTouch(1, new Vector2(100f, 0.5f));
Move(mouse.position, new Vector2(100f, 0.5f));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we be testing a mouse click too here ?

@jfreire-unity jfreire-unity force-pushed the input-for-ui/add-automated-tests branch from 884f0b6 to 4dca8fe Compare May 23, 2023 13:35
@jfreire-unity
Copy link
Collaborator Author

Holding this for after the 1.6.0 release since the trunk PR has not landed yet.

@jfreire-unity jfreire-unity force-pushed the input-for-ui/add-automated-tests branch from 4dca8fe to bb96940 Compare May 31, 2023 08:09
@jfreire-unity jfreire-unity merged commit 458c308 into develop May 31, 2023
@jfreire-unity jfreire-unity deleted the input-for-ui/add-automated-tests branch May 31, 2023 09:18
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.

3 participants