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

CoreCLR/FEPM Refactor - REVIEW ONLY - DO NOT MERGE #1915

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

timkeo
Copy link
Collaborator

@timkeo timkeo commented Apr 25, 2024

Description

The initial InputSystem refactor to support CoreCLR and FEPM.

Performing the code review in this repo but changes will ultimately be ported/landed in another future work repository.

Changes made

Please write down a short description of what changes were made.

Notes

Please write down any additional notes, remove the section if not applicable.

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 Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
    • Performance tests.
    • Integration tests.
  • 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.

@timkeo timkeo force-pushed the isx-1842-add-fepm-support-rebased branch 5 times, most recently from 7525527 to 541074d Compare May 3, 2024 23:05
- Rework InputSystem's Init functions to handle optional domain reloads
- Bypass InitializeInPlayer() "Reset" and "Restore" functions if DRs disabled

Only the first step to getting this working, but enables FEPM support for golden path scenarios.
- Move Reset/Restore state functionality out of InputSystem to the Test assembly (InputTestStateManager.cs)
- Refactor InputManager Init/Dispose to be cleaner and better abstracted:
    * Adds CreateAndInitialize static method
    * Replaces Destroy() with IDisposable implementation
    * InputManager creates "default" InputSettings object if none provided
    * Runtime, Settings, and Metrics fields now private
- Update InitializeInEditor() to incorporate changes
- Update and fix tests

For the most part, the logic should be mostly preserved. InitializeInEditor() has the biggest (logical)
change because Reset() (moved to Tests) contained some actual init calls that needed to be pulled out.
However, we *should* be making the same calls in the same order.

However, this change does seem to "break" some of the OnScreenTests(); they're now unstable.
This will need to be fixed in a later commit.
- Rework the class so it operates as a Singleton (basically how it's being used)
- Move actions get/set implementations from InputSystem to ProjectWideActions class
    * Combine functionality for tighter cohesion despite being an Editor
    * Update #ifefs to allow Player to access some of these members
- Call EnsureInitialized() from Editor init flows to fix the actual bug

Tested using ProjectWideActions sample and verified could move cube when domain reloads disabled.
Ran Package tests (Edit/Play/Player) to verify no (new) failures.
…ns (ISX-1842)

- Make InputSystem static fields private with read-only accessors
- Add InputSystemTestHooks.cs for "Test Hook" operations that need direct access to fields
- Factor out InputSystemObject (and related State) and rename to InputSystemStateManager
- Factor out "dirty asset tracking" functionality to a separate Utility class

Despite touching a bunch of files, this is a low-risk set of changes; functions are moved
to new files and types renamed, but no real changes to functionality.

Validated changes with Edit/Play/Player tests.
- Most ProjectWideAction changes discarded since feature was re-implemented (no longer relevant)
- Recreate InputSystem "Test Hook" for Enabling/Disabling Actions from tests
- Small fixes from unresolved conflicts or bad merges during rebase
- Various formatting changes (to be more consistant)
- Consolidate functionality into DeferBindingResolutionContext class
- Instantiate Context object as a non-static InputManager field exposed via InputManager methods
- Static field (in each class) initialized via RuntimeInitializeOnLoadMethod() hook
- This functionality is a bit clunky and should be refactored but that's out-of-scope for this work
- Add a flag to guard against re-initializing Plugins within PerformDefaultPluginInitialization()
- Refactor SteamSupport to fix Init/Shutdown flows and improve state management
- Refactor PlayerInput's static fields into a single GlobalState struct (matching other classes)
- Move EnhancedTouch static fields into Touch's GlobalState struct
- Minor refactoring of SteamSupport to improve init/cleanp-up flows (especially with tests)
…ts (ISX-1840)

- Consolidate Touchscreen's cached settings into a separate struct
- Rework NativeInputRuntime initialization to (fully) employ Singleton pattern
- Refactor Actions_CanHandleModification TestCase generator to work without Domain Reloads
- Fix Device static fields not getting reset during SimulateDomainReload()
@timkeo timkeo force-pushed the isx-1842-add-fepm-support-rebased branch from 541074d to 495af61 Compare May 14, 2024 21:19
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

1 participant