CHANGE: Split Runtime and Editor code into separate folders to prepare for Module migration#2321
CHANGE: Split Runtime and Editor code into separate folders to prepare for Module migration#2321jfreire-unity wants to merge 131 commits intodevelopfrom
Conversation
- Move InputSystem/* to Runtime/ (preserving history) - Move InputSystem/Editor to Editor/ (preserving history) - Add meta files for new folder structure - Follows Unity package layout conventions - All file history preserved via git mv
Also exposes the required internals between assemblies.
This is done to avoid calling into Edito specific code. Instead, it will be called if Editor code has registered a callback.
Due to refactoring, a lot of paths still contained the InputSystem folder path that no longer exists. We only have Editor and Runtime folders.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr |
Some files were moved to other folders, after merging conflicts arise but since they showed as removed changes didn't land on the moved files. This commit is fixing this
|
@u-pr can you check which review problems you mentioned have been addressed? |
|
I've checked the latest changes against the review problems previously mentioned. Here is the status of those items: ✅ Addressed
|
|
/crc |
| internal static Action<bool> s_SuppressAnalytics; | ||
|
|
||
| // API constants for analytics (matches Editor InputExitPlayModeAnalytic.Api enum) | ||
| internal enum Api |
There was a problem hiding this comment.
Status? - Would be great with clear follow up on the concern
| @@ -2985,23 +2963,25 @@ internal static bool ShouldDrawWarningIconForBinding(string bindingPath) | |||
| #region Actions | |||
|
|
|||
|
|
|||
| // This is called from InitializeInEditor() and InitializeInPlayer() to make sure | |||
| // This is called to make sure | |||
| // project-wide actions are all active in they are active in all of these MonoBehavior methods: | |||
…alInitialize() Co-authored-by: u-pr[bot] <205906871+u-pr[bot]@users.noreply.github.com>
ekcoh
left a comment
There was a problem hiding this comment.
Adding a first batch of comments and will resume from InputActionReference in the diff when I can review more.
| UnityEngine.TestTools.LogAssert.Expect(LogType.Error, "ScriptableSingleton already exists. Did you query the singleton in a constructor?"); | ||
| UnityEngine.TestTools.LogAssert.Expect(LogType.Error, "ScriptableSingleton already exists. Did you query the singleton in a constructor?"); | ||
| // Ensure the singleton is initialized deterministically in editor. | ||
| _ = RemoteInputPlayerConnection.instance; |
There was a problem hiding this comment.
Exactly, most commonly seen in in inline lambdas though. This looks very odd. Why do we need for this "singleton" to instantiate on this row? It seems like it's created below. Is there some brittle design that might need addressing here?
| @@ -216,13 +216,13 @@ public static Command DeleteSelectedControlScheme() | |||
| if (indexOfArrayElement > serializedArray.arraySize - 1) | |||
| return state.With( | |||
| selectedControlSchemeIndex: serializedArray.arraySize - 1, | |||
| selectedControlScheme: new InputControlScheme(serializedArray.GetArrayElementAtIndex(serializedArray.arraySize - 1)), selectedDeviceRequirementIndex: -1); | |||
| selectedControlScheme: InputControlSchemeEditorExtensions.FromSerializedProperty(serializedArray.GetArrayElementAtIndex(serializedArray.arraySize - 1)), selectedDeviceRequirementIndex: -1); | |||
There was a problem hiding this comment.
I would suggest making a separate bug ticket on this specific issue and related fix, then fix it directly after landing this.
| @@ -895,10 +894,14 @@ IEnumerator IEnumerable.GetEnumerator() | |||
| internal void MarkAsDirty() | |||
There was a problem hiding this comment.
Hmmm... who calls this? Given that the function is a no-op outside editor builds I wonder if this should only be tracked in the editor context, e.g. list of InputActionAsset references which are dirty instead of doing it this way.
There was a problem hiding this comment.
Seems mostly to undo changes that were done in playmode? The hash we are populating is inspected in UnityEngine.InputSystem.Utilities.DirtyAssetTracker.ReloadDirtyAssets()
I don't oppose doing what you mentioned to avoid this hook mechanism. We can probably improve things here so I'll create a ticket to do it. https://jira.unity3d.com/browse/ISX-2558
I could potentially renamed it to EditorMarkAsDirty or MarkAsDirtyInEditor() for now so it's more explicit for now?
There was a problem hiding this comment.
That's sounds good, then it sticks out even more. Since we are always in playmate for any player build it makes sense. Thanks for filing a follow up as well.
ekcoh
left a comment
There was a problem hiding this comment.
I think InputActionReference need some work. Will post again when I have reviewed this whole PR.
3161712 to
eccb175
Compare
eccb175 to
670f071
Compare
|
❌ Need to fix 670f071, InputAnalytics seems to not be the best place to have the Api analytics enum as is, |
ekcoh
left a comment
There was a problem hiding this comment.
Follow up review done based on commits since last review. Thanks for doing changes to InputActionReference, but now I have shifted to be more concerned about the analytics. Most analytics are editor only (except initialise, shutdown analytics which are fuzzy what they are - the older analytics). I might need a third pass on this but not sure if you are changing additional things with analytics or if planned changes are in? Probably need to re-review that part in combined (full review mode then).
| @@ -2985,23 +2963,25 @@ internal static bool ShouldDrawWarningIconForBinding(string bindingPath) | |||
| #region Actions | |||
|
|
|||
|
|
|||
| // This is called from InitializeInEditor() and InitializeInPlayer() to make sure | |||
| // This is called to make sure | |||
| // project-wide actions are all active in they are active in all of these MonoBehavior methods: | |||
| /// <remarks> | ||
| /// This enumeration may be added to, but NEVER changed, since it would break older data. | ||
| /// </remarks> | ||
| internal enum AuthoringApi |
There was a problem hiding this comment.
I am confused why this moved to Runtime? As far as I remember this is an editor-only analytic and irrelevant for runtime builds?
There was a problem hiding this comment.
What was the problem with keeping it in editor? I believe the culprit is the shared submission interface via runtime interface. Editor analytics should probably not be piped through that interface, instead they should be taking a different path that is editor only (as the underlying SDK API)
There was a problem hiding this comment.
The problem is that InputActionSetupExtensions.cs uses it, and it's a Runtime class. Even though it's usages it's guarded by UNITY_EDITOR, we can't pull in any Editor type to Runtime classes. This would be clearer if we also had assemblies split in this PR. (and will be clearer once we are a module in trunk).
The constraint is Runtime code can't have any InputSystem.Editor references (or any types inside the Editor folder) or any UnityEditor references as well.
Runtime folder => play mode (editor) and player builds
Editor folder => Editor only functionality.
There was a problem hiding this comment.
I can guarde it with UNITY_EDITOR so that it doesn't step into player builds at all (I don't remember the constraint of UNITY_ANALYTICS...) Let me know what u think
There was a problem hiding this comment.
The alternative is to keep the enum type in an Editor class, and Runtime references of it are just an int. This then requires us to cast the enum.
There was a problem hiding this comment.
OK, a bit of a mess I assume. Forgot about the reference the enum part. OK, if you think it's best to leave it as is for now then maybe that is best. I will approve the PR since I see no need to block this.

Description
Background & Goal
NOTE 🤯: This PR is huge and hard to review, but a lot of the changes are moving files and folders around. I know it's not easy to navigate these huge changes so I tried my best to do a good PR description.
The Unity Input System package historically had
UnityEditor.*references scattered throughout its runtime code, guarded by#if UNITY_EDITORpreprocessor directives, but still in the same files as runtime logic.This was making the codebase hard to port into a Unity module in trunk (
unity/unity), and it was also harder to reason about the Editor vs Standalone player boundary.This PR tries to break the dependency of Editor references in Runtime code to help us migrate to a Unity module and does the following changes:
UnityEditor.*references remain in the runtime source files.Unity.InputSystemassembly is kept . The folder reorganisation (Runtime/andEditor/subfolders) is the externally visible result, making forward-ports and movinggithistory to trunk and eventual module migration easier. We decided not to introduce 2 assemblies as this would duplicate our upgrade path when projects upgrade to a Unity version using the new Input System module.Ideally, we would have liked to also have a Runtime and Editor assembly.
That work was initially done but then we reverted it due to the last point above.
This PR was done with the help of agentic coding as well.
High-Level Changes
InputSystem/Runtime/, all editor source toInputSystem/Editor/. 1,201 files renamed (history preserved viagit mv).Unity.InputSystem.asmdefkept as single monolith. AUnity.InputSystem.Editorasmdef was prototyped and then rolled back — the folder split remains as foundation for a future module migration.UnityEditor.*call in runtime files with internalstaticcallback fields (Action,Func). The Editor registers these at startup via[InitializeOnLoad]. In player builds these fields arenulland all call sites are null-safe (?.Invoke()).InputSystemEditorInitializer.cs[InitializeOnLoad]class inEditor/that owns all editor lifecycle concerns previously scattered across the runtime (play-mode changes, domain reload, asset tracking, analytics, Unity Remote, remoting, etc.).InputSystemState.csInputSystemObjectand editor-specific paths) into its own runtime class to help with the dependency boundary.*Editorclasses (Interaction editors, Composite editors, Processor editors, OnScreen editors) removed from runtime files and placed in dedicated files underEditor/.#if UNITY_EDITORmoved toEditor/(e.g.PlayerInputEditor,UnityRemoteSupport,iOSPostProcessBuild).InputTestRuntime,InputTestStateManagerupdated to reflect the new interface contracts (e.g.onPlayModeChanged: Action<int>instead ofAction<PlayModeStateChange>).Pattern used for these changes
The approach used throughout this PR is maybe worth understanding before reviewing individual files:
This pattern appears in:
InputSystem.cs,InputManager.cs,NativeInputRuntime.cs,InputActionSetupExtensions.cs,InputActionAsset.cs,InputActionReference.cs,EnhancedTouchSupport.cs,RemoteInputPlayerConnection.cs,OnScreenStick.cs,TrackedPoseDriver.cs,InputSystemUIInputModule.cs.Review Guide Suggestions
1.
InputSystem/Runtime/InputSystem.csusing UnityEditor;is gone.s_OnSettingsChanged,s_OnActionsChanging,s_ShouldEnableActions,s_OnPlayModeChangeCallback) are used consistently and null-safely.#if UNITY_EDITORblock forOnPlayModeChangeis intentional — it is a forwarding shim for tests that call it directly.2.
InputSystem/Runtime/NativeInputRuntime.csEditorApplication.isPlaying,EditorApplication.isPaused,InternalEditorUtility.isApplicationActive, andEditorAnalyticscalls.internal boolfields (m_IsInPlayMode,m_IsEditorPaused,m_IsEditorActive) and callbacks (m_SendEditorAnalytic,m_SetUnityRemoteMessageHandler, etc.).UpdateEditorState()method inInputSystemEditorInitializeris pumping those fields onEditorApplication.update.3.
InputSystem/Runtime/InputManager.csEditorApplication.isPaused→m_Runtime.isEditorPaused(fromIInputRuntime).InputSystem.s_SystemObjectreferences replaced with localm_ExitEditModeTime/m_EnterPlayModeTimefields.ProjectWideActionsBuildProvider.actionsToIncludeInPlayerBuildreplaced withs_GetProjectWideActionscallback.4.
InputSystem/Runtime/IInputRuntime.csonPlayModeChangedchanged fromAction<PlayModeStateChange>toAction<int>— removes theUnityEditorenum from the interface.bool isEditorPaused { get; }.5.
InputSystem/Editor/InputSystemEditorInitializer.cs(new file)OnEditorPlayModeStateChanged→ setsm_IsInPlayMode,m_IsEditorPaused, dispatchesDispatchPlayModeChange, etc.InputSystemStateis loaded/restored here.6. Extracted
*Editorclasses (Editor/Actions/Interactions/,Editor/Actions/Composites/,Editor/Controls/Processors/,Editor/Plugins/OnScreen/)7.
InputSystem/Runtime/Actions/InputActionReference.cs,InputActionAsset.cs,EnhancedTouchSupport.csAssetDatabase/AssemblyReloadEventscalls replaced with callback fields.#if UNITY_EDITOR-guarded.8. Test fixtures —
Tests/TestFixture/InputTestRuntime.cs:onPlayModeChangedis nowAction<int>— make sure test call sites cast correctly.InputTestStateManager.cs: there were some naming changes, so something to be aware.9. Assembly /
.asmdeffilesUnity.InputSystem.asmdefshould still reference a single assembly (no Editor split).Unity.InputSystemonly.AssemblyInfo.cs:InternalsVisibleToentries cover test assemblies — verify no entry was accidentally dropped.InputSystemForUI.Editor.asmdef, since the previous folder containingInputSystemForUI.asmdefwas split into Editor and Runtime. This is part of internal API so I don't see any breakages in this area when upgrading.Testing status & QA
Built and run Samples on both Windows and macOS Play-mode and Standalone builds.
I recommend validating all the Samples work and do kind of the sames tests as we would do for a Release since this PR touches a lot of things.
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.
Comments to reviewers
Some sanity check guides:
using UnityEditor;in any file underInputSystem/Runtime/InputSystemEditorInitializerregisters every callback that has a field on the runtime sideInputTestRuntimecompiles and tests pass with the updatedAction<int>signatureUnity.InputSystemwas accidentally wired to a now-deletedUnity.InputSystem.Editorassembly. Hopefully there are not left overs of this initial work.One thing is not in place is a mechanism to guarantee we don't leak UnityEditor references in the Runtime folder. I'll follow up on that in another PR.
Checklist
Before review:
Changed,Fixed,Addedsections.Area_CanDoX,Area_CanDoX_EvenIfYIsTheCase,Area_WhenIDoX_AndYHappens_ThisIsTheResult.During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.