diff --git a/.agents/skills/uloop-hot-reload/references/output.md b/.agents/skills/uloop-hot-reload/references/output.md index fed7ea4a2..72a3dec2f 100644 --- a/.agents/skills/uloop-hot-reload/references/output.md +++ b/.agents/skills/uloop-hot-reload/references/output.md @@ -6,7 +6,7 @@ Returns JSON with: - `ErrorCode` (string, optional): Present on parameter validation failure. Values are `HOT_RELOAD_FILES_REQUIRED` when an omitted apply has no compile snapshots, `HOT_RELOAD_NO_CHANGED_FILES` when snapshots contain no changed `.cs` files, `HOT_RELOAD_INVALID_FILES` when `--files` contains a null or empty path, and `HOT_RELOAD_STATUS_CONFLICT` when `--status` is combined with `--files` or `--revert-all`. - `NextActions` (array, optional): Ordered recovery steps, present only with `ErrorCode` on a parameter validation failure. Omitted from every other response, including successful apply, plain `--status`, and `--revert-all` runs. - `Methods` (array): Per-method `{ Kind, Method, Reason, FilePath, InvocationCount, LifecycleNote, ReappliedFromSibling }` where `Kind` is `Patched`, `Skipped`, `Failed`, `Added`, `AlreadyActive`, or `Stale` on apply runs, and `Active`, `Added`, or `AddedField` on `--status` runs; empty on `--revert-all` runs. `AlreadyActive` means this file's source matched the last fully applied reload (a run with no Skipped or Failed outcomes), so the existing patch was left in place and the row carries the live `InvocationCount`. `Stale` means the method was deleted from the edited source while its patch is still installed: compiled callers keep running the patched body until `uloop compile`, `--revert-all`, or a later reload whose source restores the method to the compiled baseline clears it; a reload that declares the method with a different body replaces the patch instead of clearing it. Stale rows keep counting toward `ActivePatchTotal`, and the Message summary includes `Stale=N`. `InvocationCount` is meaningful on `Active` rows and on `AlreadyActive` and `Stale` apply rows (calls since the current patch was applied); it is `0` on other apply/revert outcomes. On `--status`, an `Active` row with `InvocationCount` 0 sets `Reason` to explain that the method has not run since the patch: finished calls do not re-run, the patched body takes effect on the next call, and how to retrigger an initialization-only path. When the edited source later declares a different signature, that `Active` row's `Reason` instead explains it is superseded by a new declaration of that signature and is no longer the entry point for new calls (superseded wins over the never-invoked sentence). Added-member rows always show InvocationCount 0 — added-member calls are not instrumented, and the row's Reason says so. `AddedField` rows list a live added field as `Type.field` with an empty `Reason`; they are not method patches. `LifecycleNote` is set when a patched method is a Unity one-shot lifecycle message (`private void Awake`/`Start`/`OnEnable`/`OnDisable`/`OnDestroy` on a `MonoBehaviour`), or when every compiled call path into the patched method (callers of callers are followed a few levels within the compiled assemblies) starts at such a message; empty otherwise — it does not change `Kind`. On an `Added` row whose method is a Unity message, `LifecycleNote` instead says whether the engine will reach it: a forwarded message (`Start`, `Update`, the collision/trigger/mouse messages, and the rest listed in [scope-and-limits.md](scope-and-limits.md)) carries the note that a hot-reload proxy component delivers it to live instances while Play Mode runs, that the proxy is rebuilt only when a later reload changes which messages the type adds or their signatures, that execution order relative to other components is not guaranteed, and that it is gone on any compile or domain reload (only an added `Start` row also says that it runs once on each existing instance when the proxy attaches, and again when the proxy is rebuilt); a message this feature leaves to the compiler (`Awake`, `OnEnable`, `OnDisable`, `OnDestroy`, the editor-only messages, and any non-void message) carries the note that the engine does not invoke it until `uloop compile`, and the run adds one `Warnings` line naming every such message together. `Added` rows carry the added member's signature and file; their `InvocationCount` is always `0` (added-member calls are not instrumented). `ReappliedFromSibling` is `true` on every apply row, whatever its `Kind`, that belongs to a sibling file the run pulled in to re-apply changes from earlier reloads rather than to a file passed in `--files`; it is `false` on the other rows and on every `--status` and `--revert-all` row. Message's re-applied count covers only the `Patched` and `Added` rows among them. `Method` spells parameter types as .NET metadata does, the same on every method row: a constructed generic as ``System.Collections.Generic.List`1``, a multidimensional array as `System.Int32[0...,0...]`, and a nested type with `+`. Example `--status` row: `{ "Kind": "Added", "Method": "Ns.Host.NewHelper(System.Int32)", "Reason": "Added-member calls are not instrumented, so InvocationCount is always 0 for this row.", "FilePath": "Assets/Scripts/Host.cs", "InvocationCount": 0, "LifecycleNote": "", "ReappliedFromSibling": false }` -- `Warnings` (array): Non-fatal notes — one aggregated line listing the patched methods at risk of being already JIT-inlined into existing callers — those marked `[AggressiveInlining]`, plus (only when Code Optimization is Release) those with tiny pre-patch bodies — meaning the change may not show at those call sites, the pause-point interaction (see [pause-point-interaction.md](pause-point-interaction.md)), and the const drift, outside-body drift, and missing-baseline entries described in [scope-and-limits.md](scope-and-limits.md). Skipped outcomes are echoed here as `Skipped : `, or as one `Skipped N methods: ()` line per reason when several share it, so checking `Warnings` alone is enough to see that an edit was not applied. When a reload re-applies unchanged files so their patches bind to this run's shim, Warnings includes `Also re-applied N unchanged file(s) with active patches in assembly '...' so their patches bind to this reload's shim: ...`. When a pulled-in sibling fails in that reload, Warnings includes `'...' was pulled in to re-bind its active patches but this reload failed for it; see its rows for which patches changed and run uloop compile to clear the run.` instead of the re-applied line. When every row of that sibling was `Skipped` and none failed, Warnings instead includes `'...' was pulled in to re-bind its active patches, but every method there was Skipped this time; see its rows for the reasons. Any earlier patches there stay active until uloop compile clears the run.` When the reload stopped before re-applying anything at all, so that sibling has no rows, Warnings instead includes `'...' was pulled in to re-bind its active patches, but this reload stopped before re-applying them, so its active patches are unchanged. Fix the refused declaration and rerun, or run uloop compile to clear the run.` When the whole reload was refused, so that sibling's only rows are `Method` = `(file)` `Failed` rows repeating the refusal and none of its unchanged patches were reverted, Warnings instead includes `'...' was pulled in to re-bind its active patches, but the whole reload was refused before re-applying them, so its active patches are unchanged; its rows repeat the refusal reason. Fix that and rerun, or run uloop compile to clear the run.` When a sibling still has active patches but its source changed since they were applied, Warnings includes `'...' has active patches but its source changed since they were applied, so it was not re-applied; pass it to hot-reload to update it.` When a run carries two or more warnings and all of them are hot reload warnings, the Message ends with "A single 'uloop compile' clears all of them at once when you want them gone; none of them has to be cleared before you keep working." — it is the shortest recovery, not an obligation to compile immediately. Pause-point warnings carry their own recovery steps, so that line does not appear when they are present. Nor does it appear when an `IntroducedTypes` row is `Failed` or a warning says a declared type requires a compile, because that type does not exist until one. It is also left off when any `Methods` row is `Failed`, or when a `Methods` row of a file you passed, or of a sibling retried after an earlier Skip, is `Skipped`: that body is not running yet, so it needs a fix or a compile before you keep working. A `Skipped` row of a sibling pulled in only to re-bind its active patches does not leave it off, because the earlier patches there keep running. +- `Warnings` (array): Non-fatal notes — one aggregated line listing the patched methods at risk of being already JIT-inlined into existing callers — those marked `[AggressiveInlining]`, plus (only when Code Optimization is Release) those with tiny pre-patch bodies — meaning the change may not show at those call sites, the pause-point interaction (see [pause-point-interaction.md](pause-point-interaction.md)), and the const drift, outside-body drift, missing-baseline, and left-out enum file entries described in [scope-and-limits.md](scope-and-limits.md). Skipped outcomes are echoed here as `Skipped : `, or as one `Skipped N methods: ()` line per reason when several share it, so checking `Warnings` alone is enough to see that an edit was not applied. When a reload re-applies unchanged files so their patches bind to this run's shim, Warnings includes `Also re-applied N unchanged file(s) with active patches in assembly '...' so their patches bind to this reload's shim: ...`. When a pulled-in sibling fails in that reload, Warnings includes `'...' was pulled in to re-bind its active patches but this reload failed for it; see its rows for which patches changed and run uloop compile to clear the run.` instead of the re-applied line. When every row of that sibling was `Skipped` and none failed, Warnings instead includes `'...' was pulled in to re-bind its active patches, but every method there was Skipped this time; see its rows for the reasons. Any earlier patches there stay active until uloop compile clears the run.` When the reload stopped before re-applying anything at all, so that sibling has no rows, Warnings instead includes `'...' was pulled in to re-bind its active patches, but this reload stopped before re-applying them, so its active patches are unchanged. Fix the refused declaration and rerun, or run uloop compile to clear the run.` When the whole reload was refused, so that sibling's only rows are `Method` = `(file)` `Failed` rows repeating the refusal and none of its unchanged patches were reverted, Warnings instead includes `'...' was pulled in to re-bind its active patches, but the whole reload was refused before re-applying them, so its active patches are unchanged; its rows repeat the refusal reason. Fix that and rerun, or run uloop compile to clear the run.` When a sibling still has active patches but its source changed since they were applied, Warnings includes `'...' has active patches but its source changed since they were applied, so it was not re-applied; pass it to hot-reload to update it.` When a run carries two or more warnings and all of them are hot reload warnings, the Message ends with "A single 'uloop compile' clears all of them at once when you want them gone; none of them has to be cleared before you keep working." — it is the shortest recovery, not an obligation to compile immediately. Pause-point warnings carry their own recovery steps, so that line does not appear when they are present. Nor does it appear when an `IntroducedTypes` row is `Failed` or a warning says a declared type requires a compile, because that type does not exist until one. It is also left off when any `Methods` row is `Failed`, or when a `Methods` row of a file you passed, or of a sibling retried after an earlier Skip, is `Skipped`: that body is not running yet, so it needs a fix or a compile before you keep working. A `Skipped` row of a sibling pulled in only to re-bind its active patches does not leave it off, because the earlier patches there keep running. - `PatchedTotal` (number): Methods patched in this run - `AddedFields` (array): source-level names ("Type.field") of fields this reload added; their values live outside the compiled type until 'uloop compile'. Every run that adds fields also carries one warning stating that the values live outside the compiled assembly and last only until the next 'uloop compile' or domain reload; the warning names exactly the fields listed in AddedFields. An active added field declared with `[SerializeField]`, `[SerializeReference]`, or `[FormerlySerializedAs]` is also named, as `Namespace.Type.field` (nested types joined with `.`), in one `Added field(s) with a serialization attribute will not appear in the Inspector or serialize until 'uloop compile': ...` warning that points at [added-field-wiring.md](added-field-wiring.md). Only the run that first leaves the field active names it; a file that is Skipped or Failed names none of its fields, and a field is named again only after it stopped being active or after `--revert-all`. Pause-point `CapturedVariables` never includes these fields; `enable-pause-point` warns when the resolved type has any. - `AddedConsts` (array): source-level names ("Type.const") of consts this reload added. They are folded into edited bodies as literals, so they are not listed in AddedFields and do not emit the added-field lifetime warning. diff --git a/.agents/skills/uloop-hot-reload/references/scope-and-limits.md b/.agents/skills/uloop-hot-reload/references/scope-and-limits.md index d9f22e25c..903a9997f 100644 --- a/.agents/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.agents/skills/uloop-hot-reload/references/scope-and-limits.md @@ -129,6 +129,11 @@ reason says the name is an enum member this reload adds, and the enum-member and changed-`const` warnings of the files passed to that reload stay in `Warnings` even though that failure stops the file. The drift of a changed sibling file that was not passed is not reported on this failure path. +When `--files` is omitted and the enum's file has no edit besides its new enum members, +the reload leaves that file out instead, so the added members of the other files apply; +a `Warnings` line names the left-out file and the enum members that still need +`uloop compile`. A file that already holds patches or declares a new type stays in the +reload. An added property applies unless its shape is listed below. A bodied getter or setter is emitted like an added method; diff --git a/.claude/skills/uloop-hot-reload/references/output.md b/.claude/skills/uloop-hot-reload/references/output.md index fed7ea4a2..72a3dec2f 100644 --- a/.claude/skills/uloop-hot-reload/references/output.md +++ b/.claude/skills/uloop-hot-reload/references/output.md @@ -6,7 +6,7 @@ Returns JSON with: - `ErrorCode` (string, optional): Present on parameter validation failure. Values are `HOT_RELOAD_FILES_REQUIRED` when an omitted apply has no compile snapshots, `HOT_RELOAD_NO_CHANGED_FILES` when snapshots contain no changed `.cs` files, `HOT_RELOAD_INVALID_FILES` when `--files` contains a null or empty path, and `HOT_RELOAD_STATUS_CONFLICT` when `--status` is combined with `--files` or `--revert-all`. - `NextActions` (array, optional): Ordered recovery steps, present only with `ErrorCode` on a parameter validation failure. Omitted from every other response, including successful apply, plain `--status`, and `--revert-all` runs. - `Methods` (array): Per-method `{ Kind, Method, Reason, FilePath, InvocationCount, LifecycleNote, ReappliedFromSibling }` where `Kind` is `Patched`, `Skipped`, `Failed`, `Added`, `AlreadyActive`, or `Stale` on apply runs, and `Active`, `Added`, or `AddedField` on `--status` runs; empty on `--revert-all` runs. `AlreadyActive` means this file's source matched the last fully applied reload (a run with no Skipped or Failed outcomes), so the existing patch was left in place and the row carries the live `InvocationCount`. `Stale` means the method was deleted from the edited source while its patch is still installed: compiled callers keep running the patched body until `uloop compile`, `--revert-all`, or a later reload whose source restores the method to the compiled baseline clears it; a reload that declares the method with a different body replaces the patch instead of clearing it. Stale rows keep counting toward `ActivePatchTotal`, and the Message summary includes `Stale=N`. `InvocationCount` is meaningful on `Active` rows and on `AlreadyActive` and `Stale` apply rows (calls since the current patch was applied); it is `0` on other apply/revert outcomes. On `--status`, an `Active` row with `InvocationCount` 0 sets `Reason` to explain that the method has not run since the patch: finished calls do not re-run, the patched body takes effect on the next call, and how to retrigger an initialization-only path. When the edited source later declares a different signature, that `Active` row's `Reason` instead explains it is superseded by a new declaration of that signature and is no longer the entry point for new calls (superseded wins over the never-invoked sentence). Added-member rows always show InvocationCount 0 — added-member calls are not instrumented, and the row's Reason says so. `AddedField` rows list a live added field as `Type.field` with an empty `Reason`; they are not method patches. `LifecycleNote` is set when a patched method is a Unity one-shot lifecycle message (`private void Awake`/`Start`/`OnEnable`/`OnDisable`/`OnDestroy` on a `MonoBehaviour`), or when every compiled call path into the patched method (callers of callers are followed a few levels within the compiled assemblies) starts at such a message; empty otherwise — it does not change `Kind`. On an `Added` row whose method is a Unity message, `LifecycleNote` instead says whether the engine will reach it: a forwarded message (`Start`, `Update`, the collision/trigger/mouse messages, and the rest listed in [scope-and-limits.md](scope-and-limits.md)) carries the note that a hot-reload proxy component delivers it to live instances while Play Mode runs, that the proxy is rebuilt only when a later reload changes which messages the type adds or their signatures, that execution order relative to other components is not guaranteed, and that it is gone on any compile or domain reload (only an added `Start` row also says that it runs once on each existing instance when the proxy attaches, and again when the proxy is rebuilt); a message this feature leaves to the compiler (`Awake`, `OnEnable`, `OnDisable`, `OnDestroy`, the editor-only messages, and any non-void message) carries the note that the engine does not invoke it until `uloop compile`, and the run adds one `Warnings` line naming every such message together. `Added` rows carry the added member's signature and file; their `InvocationCount` is always `0` (added-member calls are not instrumented). `ReappliedFromSibling` is `true` on every apply row, whatever its `Kind`, that belongs to a sibling file the run pulled in to re-apply changes from earlier reloads rather than to a file passed in `--files`; it is `false` on the other rows and on every `--status` and `--revert-all` row. Message's re-applied count covers only the `Patched` and `Added` rows among them. `Method` spells parameter types as .NET metadata does, the same on every method row: a constructed generic as ``System.Collections.Generic.List`1``, a multidimensional array as `System.Int32[0...,0...]`, and a nested type with `+`. Example `--status` row: `{ "Kind": "Added", "Method": "Ns.Host.NewHelper(System.Int32)", "Reason": "Added-member calls are not instrumented, so InvocationCount is always 0 for this row.", "FilePath": "Assets/Scripts/Host.cs", "InvocationCount": 0, "LifecycleNote": "", "ReappliedFromSibling": false }` -- `Warnings` (array): Non-fatal notes — one aggregated line listing the patched methods at risk of being already JIT-inlined into existing callers — those marked `[AggressiveInlining]`, plus (only when Code Optimization is Release) those with tiny pre-patch bodies — meaning the change may not show at those call sites, the pause-point interaction (see [pause-point-interaction.md](pause-point-interaction.md)), and the const drift, outside-body drift, and missing-baseline entries described in [scope-and-limits.md](scope-and-limits.md). Skipped outcomes are echoed here as `Skipped : `, or as one `Skipped N methods: ()` line per reason when several share it, so checking `Warnings` alone is enough to see that an edit was not applied. When a reload re-applies unchanged files so their patches bind to this run's shim, Warnings includes `Also re-applied N unchanged file(s) with active patches in assembly '...' so their patches bind to this reload's shim: ...`. When a pulled-in sibling fails in that reload, Warnings includes `'...' was pulled in to re-bind its active patches but this reload failed for it; see its rows for which patches changed and run uloop compile to clear the run.` instead of the re-applied line. When every row of that sibling was `Skipped` and none failed, Warnings instead includes `'...' was pulled in to re-bind its active patches, but every method there was Skipped this time; see its rows for the reasons. Any earlier patches there stay active until uloop compile clears the run.` When the reload stopped before re-applying anything at all, so that sibling has no rows, Warnings instead includes `'...' was pulled in to re-bind its active patches, but this reload stopped before re-applying them, so its active patches are unchanged. Fix the refused declaration and rerun, or run uloop compile to clear the run.` When the whole reload was refused, so that sibling's only rows are `Method` = `(file)` `Failed` rows repeating the refusal and none of its unchanged patches were reverted, Warnings instead includes `'...' was pulled in to re-bind its active patches, but the whole reload was refused before re-applying them, so its active patches are unchanged; its rows repeat the refusal reason. Fix that and rerun, or run uloop compile to clear the run.` When a sibling still has active patches but its source changed since they were applied, Warnings includes `'...' has active patches but its source changed since they were applied, so it was not re-applied; pass it to hot-reload to update it.` When a run carries two or more warnings and all of them are hot reload warnings, the Message ends with "A single 'uloop compile' clears all of them at once when you want them gone; none of them has to be cleared before you keep working." — it is the shortest recovery, not an obligation to compile immediately. Pause-point warnings carry their own recovery steps, so that line does not appear when they are present. Nor does it appear when an `IntroducedTypes` row is `Failed` or a warning says a declared type requires a compile, because that type does not exist until one. It is also left off when any `Methods` row is `Failed`, or when a `Methods` row of a file you passed, or of a sibling retried after an earlier Skip, is `Skipped`: that body is not running yet, so it needs a fix or a compile before you keep working. A `Skipped` row of a sibling pulled in only to re-bind its active patches does not leave it off, because the earlier patches there keep running. +- `Warnings` (array): Non-fatal notes — one aggregated line listing the patched methods at risk of being already JIT-inlined into existing callers — those marked `[AggressiveInlining]`, plus (only when Code Optimization is Release) those with tiny pre-patch bodies — meaning the change may not show at those call sites, the pause-point interaction (see [pause-point-interaction.md](pause-point-interaction.md)), and the const drift, outside-body drift, missing-baseline, and left-out enum file entries described in [scope-and-limits.md](scope-and-limits.md). Skipped outcomes are echoed here as `Skipped : `, or as one `Skipped N methods: ()` line per reason when several share it, so checking `Warnings` alone is enough to see that an edit was not applied. When a reload re-applies unchanged files so their patches bind to this run's shim, Warnings includes `Also re-applied N unchanged file(s) with active patches in assembly '...' so their patches bind to this reload's shim: ...`. When a pulled-in sibling fails in that reload, Warnings includes `'...' was pulled in to re-bind its active patches but this reload failed for it; see its rows for which patches changed and run uloop compile to clear the run.` instead of the re-applied line. When every row of that sibling was `Skipped` and none failed, Warnings instead includes `'...' was pulled in to re-bind its active patches, but every method there was Skipped this time; see its rows for the reasons. Any earlier patches there stay active until uloop compile clears the run.` When the reload stopped before re-applying anything at all, so that sibling has no rows, Warnings instead includes `'...' was pulled in to re-bind its active patches, but this reload stopped before re-applying them, so its active patches are unchanged. Fix the refused declaration and rerun, or run uloop compile to clear the run.` When the whole reload was refused, so that sibling's only rows are `Method` = `(file)` `Failed` rows repeating the refusal and none of its unchanged patches were reverted, Warnings instead includes `'...' was pulled in to re-bind its active patches, but the whole reload was refused before re-applying them, so its active patches are unchanged; its rows repeat the refusal reason. Fix that and rerun, or run uloop compile to clear the run.` When a sibling still has active patches but its source changed since they were applied, Warnings includes `'...' has active patches but its source changed since they were applied, so it was not re-applied; pass it to hot-reload to update it.` When a run carries two or more warnings and all of them are hot reload warnings, the Message ends with "A single 'uloop compile' clears all of them at once when you want them gone; none of them has to be cleared before you keep working." — it is the shortest recovery, not an obligation to compile immediately. Pause-point warnings carry their own recovery steps, so that line does not appear when they are present. Nor does it appear when an `IntroducedTypes` row is `Failed` or a warning says a declared type requires a compile, because that type does not exist until one. It is also left off when any `Methods` row is `Failed`, or when a `Methods` row of a file you passed, or of a sibling retried after an earlier Skip, is `Skipped`: that body is not running yet, so it needs a fix or a compile before you keep working. A `Skipped` row of a sibling pulled in only to re-bind its active patches does not leave it off, because the earlier patches there keep running. - `PatchedTotal` (number): Methods patched in this run - `AddedFields` (array): source-level names ("Type.field") of fields this reload added; their values live outside the compiled type until 'uloop compile'. Every run that adds fields also carries one warning stating that the values live outside the compiled assembly and last only until the next 'uloop compile' or domain reload; the warning names exactly the fields listed in AddedFields. An active added field declared with `[SerializeField]`, `[SerializeReference]`, or `[FormerlySerializedAs]` is also named, as `Namespace.Type.field` (nested types joined with `.`), in one `Added field(s) with a serialization attribute will not appear in the Inspector or serialize until 'uloop compile': ...` warning that points at [added-field-wiring.md](added-field-wiring.md). Only the run that first leaves the field active names it; a file that is Skipped or Failed names none of its fields, and a field is named again only after it stopped being active or after `--revert-all`. Pause-point `CapturedVariables` never includes these fields; `enable-pause-point` warns when the resolved type has any. - `AddedConsts` (array): source-level names ("Type.const") of consts this reload added. They are folded into edited bodies as literals, so they are not listed in AddedFields and do not emit the added-field lifetime warning. diff --git a/.claude/skills/uloop-hot-reload/references/scope-and-limits.md b/.claude/skills/uloop-hot-reload/references/scope-and-limits.md index d9f22e25c..903a9997f 100644 --- a/.claude/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.claude/skills/uloop-hot-reload/references/scope-and-limits.md @@ -129,6 +129,11 @@ reason says the name is an enum member this reload adds, and the enum-member and changed-`const` warnings of the files passed to that reload stay in `Warnings` even though that failure stops the file. The drift of a changed sibling file that was not passed is not reported on this failure path. +When `--files` is omitted and the enum's file has no edit besides its new enum members, +the reload leaves that file out instead, so the added members of the other files apply; +a `Warnings` line names the left-out file and the enum members that still need +`uloop compile`. A file that already holds patches or declares a new type stays in the +reload. An added property applies unless its shape is listed below. A bodied getter or setter is emitted like an added method; diff --git a/Assets/Tests/Editor/HotReload/HotReloadDefaultFileSelectorKindTests.cs b/Assets/Tests/Editor/HotReload/HotReloadDefaultFileSelectorKindTests.cs new file mode 100644 index 000000000..d3798860b --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadDefaultFileSelectorKindTests.cs @@ -0,0 +1,68 @@ +using System; +using System.Collections.Generic; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// Covers whether a resolved selection says it was chosen from compile snapshots, which is + /// what lets a run leave out a file the caller never named. + /// + public sealed class HotReloadDefaultFileSelectorKindTests + { + private const string EnumPath = "Assets/Scripts/Kind.cs"; + private const string CallerPath = "Assets/Scripts/Caller.cs"; + + [SetUp] + public void SetUp() + { + // The production tool captures the package roots before it normalizes paths; a direct + // call to the real normalizer has to do the same. + HotReloadCompositionRoot.Services.PackageRootCapture.CaptureCurrent(); + } + + /// + /// What: files the caller passed are not a default selection. + /// + [Test] + public void Resolve_WhenFilesArePassed_IsNotADefaultSelection() + { + HotReloadDefaultFileSelection selection = HotReloadDefaultFileSelector.Resolve( + new[] { EnumPath, CallerPath }, + () => throw new AssertionException("Explicit --files must not scan for changed files."), + Array.Empty(), + ToProjectRelativePath); + + Assert.That(selection.IsDefaultSelection, Is.False); + } + + /// + /// What: the changed files chosen when the files parameter is omitted are a default selection. + /// + [Test] + public void Resolve_WhenFilesAreOmitted_IsADefaultSelection() + { + HotReloadDefaultFileSelection selection = HotReloadDefaultFileSelector.Resolve( + Array.Empty(), + () => new HotReloadChangedFileAggregationResult( + true, + new List { EnumPath, CallerPath }, + new List()), + Array.Empty(), + ToProjectRelativePath); + + Assert.That(selection.Files, Is.EqualTo(new[] { EnumPath, CallerPath })); + Assert.That(selection.IsDefaultSelection, Is.True); + } + + private static string ToProjectRelativePath(string path) + { + return HotReloadPatchTargetSupport.ToProjectRelativeScriptPath( + HotReloadCompositionRoot.Services.PackageRootCapture, + path); + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadDefaultFileSelectorKindTests.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadDefaultFileSelectorKindTests.cs.meta new file mode 100644 index 000000000..e66caee05 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadDefaultFileSelectorKindTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 826bc396c372f4fc4948b83e829a342f +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadDefaultFilesTests.cs b/Assets/Tests/Editor/HotReload/HotReloadDefaultFilesTests.cs index 48911fc19..d37d988ce 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadDefaultFilesTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadDefaultFilesTests.cs @@ -271,6 +271,35 @@ public async Task ExecuteAsync_WhenFilesArePassed_DoesNotAddDiscardedFiles() Assert.That(appliedFiles, Is.EqualTo(new[] { "Assets/Explicit.cs" })); } + /// + /// What: the tool starts the run as a default selection exactly when --files was omitted, + /// which is what lets a group leave out a selected file the caller never named. + /// + [TestCase(true)] + [TestCase(false)] + public async Task ExecuteAsync_MarksTheRunAsADefaultSelectionOnlyWhenFilesAreOmitted(bool filesOmitted) + { + using IDisposable detectorScope = BeginChangedFiles("Assets/Changed1.cs"); + HotReloadStubOrchestrator orchestrator = new HotReloadStubOrchestrator((files, ignoredCt) => + Task.FromResult( + new HotReloadOrchestratorResult( + new List + { + HotReloadMethodOutcome.Patched("Host.Selected()", "Assets/Changed1.cs") + }, + new List(), + patchedTotal: 1, + activePatchTotal: 1))); + using IDisposable orchestratorScope = HotReloadServicesTestScope.BeginWithOrchestrator(orchestrator); + + await ExecuteAsync( + filesOmitted + ? new JObject() + : new JObject { ["Files"] = new JArray("Assets/Changed1.cs") }); + + Assert.That(orchestrator.ReceivedIsDefaultSelection, Is.EqualTo(filesOmitted)); + } + /// /// What: a script listed twice in --files reaches the run once, as the first raw entry, and /// the response message starts with the sentence saying so. diff --git a/Assets/Tests/Editor/HotReload/HotReloadDefaultSelectionEnumLeaveOutE2ETests.cs b/Assets/Tests/Editor/HotReload/HotReloadDefaultSelectionEnumLeaveOutE2ETests.cs new file mode 100644 index 000000000..584a81f3f --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadDefaultSelectionEnumLeaveOutE2ETests.cs @@ -0,0 +1,311 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// End-to-end coverage of a default-selection run that picks up a file whose only edit adds + /// enum members. Such a file cannot apply anything, and building its enum from source would + /// skip the added methods of the other files that pass the enum to compiled code or an + /// introduced type, so the run leaves it out instead of asking the caller to. + /// + /// + /// Why the introduced type is not a fixture on disk: a .cs under Assets/ is compiled into the + /// test assembly, and a type the compiler already lists is never introduced. + /// + public class HotReloadDefaultSelectionEnumLeaveOutE2ETests : HotReloadIntroducedTypeE2ETestBase + { + private const string SinkOwnerPath = "Assets/Tests/Editor/HotReload/UncompiledDefaultSelectionSink.cs"; + private const string EnumFileName = "HotReloadSiblingEnumDefinitions.cs"; + private const string EnumProjectRelativePath = "Assets/Tests/Editor/HotReload/" + EnumFileName; + private const string HostFileName = "HotReloadCrossFileAddedMemberHost.cs"; + private const string RegistryFileName = "HotReloadCarriedInNextStepRegistry.cs"; + private const string Namespace = "io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload"; + private const string SinkSimpleName = "HotReloadDefaultSelectionSink"; + private const string HelperSimpleName = "HotReloadDefaultSelectionIntroducedHelper"; + private const string EnumLastMemberAnchor = " Second = 2"; + private const string HostValueAnchor = " public int Value()"; + private const string LeftOutWarningStart = "Left '" + EnumProjectRelativePath + "' out of this reload"; + + private static readonly string TakeKindMember = + " public int TakeKind()\n" + + " {\n" + + " return " + SinkSimpleName + ".Take(HotReloadSiblingEnum.First);\n" + + " }\n" + + "\n"; + + private static readonly string AcceptKindMember = + " public int AcceptKind()\n" + + " {\n" + + " return HotReloadCarriedInNextStepRegistry.Accept(HotReloadSiblingEnum.First);\n" + + " }\n" + + "\n"; + + /// + /// What: beside an added method that passes the enum to a type an earlier reload + /// introduced, a default selection leaves the enum-only file out, so the added method + /// applies, the file is named once as left out, its enum-member warning stays, and nothing + /// is left for a compile fallback. A second default selection of the same edits leaves the + /// file out again instead of skipping the method that now holds a patch. + /// + [Test] + public async Task Run_DefaultSelectionBesideAnIntroducedTypeCaller_LeavesTheEnumFileOut() + { + string hostPath = FixturePath(HostFileName); + string enumPath = FixturePath(EnumFileName); + + await RunInIntroducedTypeDomainAsync(async _ => + { + await IntroduceSinkAsync(); + Dictionary edits = WriteEdits( + new Dictionary + { + [enumPath] = InsertEnumMember(File.ReadAllText(enumPath)), + [hostPath] = AddHostMembers(File.ReadAllText(hostPath), TakeKindMember) + }, + "DefaultLeaveOut"); + + HotReloadOrchestratorResult result = await RunDefaultSelectionAsync(new[] { enumPath, hostPath }, edits); + + Assert.That(FindOutcomeKind(result, ".TakeKind("), Is.EqualTo(HotReloadMethodOutcomeKind.Added), DescribeRun(result)); + Assert.That(CountRows(result, HotReloadMethodOutcomeKind.Skipped), Is.EqualTo(0), DescribeRun(result)); + Assert.That(CountFailures(result), Is.EqualTo(0), DescribeRun(result)); + Assert.That(CountWarnings(result, LeftOutWarningStart), Is.EqualTo(1), DescribeRun(result)); + Assert.That(CountWarnings(result, "enum member "), Is.EqualTo(1), DescribeRun(result)); + // Nothing the run was asked to apply stays unapplied, so --compile-on-skip auto + // in Edit Mode no longer compiles for this run. + Assert.That( + HotReloadCompileFallbackDecider.HasUnappliedEdit( + result, + new HotReloadReappliedSiblingFiles(Array.Empty(), path => path)), + Is.False, + DescribeRun(result)); + + HotReloadOrchestratorResult again = await RunDefaultSelectionAsync(new[] { enumPath, hostPath }, edits); + + Assert.That(CountRows(again, HotReloadMethodOutcomeKind.Skipped), Is.EqualTo(0), DescribeRun(again)); + Assert.That(CountFailures(again), Is.EqualTo(0), DescribeRun(again)); + Assert.That(CountWarnings(again, LeftOutWarningStart), Is.EqualTo(1), DescribeRun(again)); + }); + } + + /// + /// What: beside an added method that passes the enum to a compiled API, a default selection + /// leaves the enum-only file out, so the added method applies instead of asking for the + /// API's file to be passed. + /// + [Test] + public async Task Run_DefaultSelectionBesideACompiledApiCaller_LeavesTheEnumFileOut() + { + string hostPath = FixturePath(HostFileName); + string enumPath = FixturePath(EnumFileName); + + await RunInIntroducedTypeDomainAsync(async _ => + { + Dictionary edits = WriteEdits( + new Dictionary + { + [enumPath] = InsertEnumMember(File.ReadAllText(enumPath)), + [hostPath] = AddHostMembers(File.ReadAllText(hostPath), AcceptKindMember) + }, + "DefaultCompiledApi"); + + HotReloadOrchestratorResult result = await RunDefaultSelectionAsync(new[] { enumPath, hostPath }, edits); + + Assert.That(FindOutcomeKind(result, ".AcceptKind("), Is.EqualTo(HotReloadMethodOutcomeKind.Added), DescribeRun(result)); + Assert.That(CountRows(result, HotReloadMethodOutcomeKind.Skipped), Is.EqualTo(0), DescribeRun(result)); + Assert.That(CountWarnings(result, LeftOutWarningStart), Is.EqualTo(1), DescribeRun(result)); + }); + } + + /// + /// What: a default selection that also introduces a type leaves the enum-only file out + /// inside the run that prepared the type, so the type is still introduced and the added + /// method that passes the enum to an earlier introduced type applies. + /// + [Test] + public async Task Run_DefaultSelectionThatAlsoIntroducesAType_LeavesTheEnumFileOut() + { + string hostPath = FixturePath(HostFileName); + string enumPath = FixturePath(EnumFileName); + string registryPath = FixturePath(RegistryFileName); + + await RunInIntroducedTypeDomainAsync(async _ => + { + await IntroduceSinkAsync(); + Dictionary edits = WriteEdits( + new Dictionary + { + [enumPath] = InsertEnumMember(File.ReadAllText(enumPath)), + [hostPath] = AddHostMembers(File.ReadAllText(hostPath), TakeKindMember), + [registryPath] = File.ReadAllText(registryPath) + BuildHelperSource() + }, + "DefaultPrepared"); + + HotReloadOrchestratorResult result = await RunDefaultSelectionAsync( + new[] { enumPath, hostPath, registryPath }, + edits); + + Assert.That(FindOutcomeKind(result, ".TakeKind("), Is.EqualTo(HotReloadMethodOutcomeKind.Added), DescribeRun(result)); + Assert.That(FindIntroducedKind(result, HelperSimpleName), Is.EqualTo(HotReloadIntroducedTypeOutcomeKind.Introduced), DescribeRun(result)); + Assert.That(CountFailures(result), Is.EqualTo(0), DescribeRun(result)); + Assert.That(CountWarnings(result, LeftOutWarningStart), Is.EqualTo(1), DescribeRun(result)); + }); + } + + private static async Task IntroduceSinkAsync() + { + Dictionary edits = WriteEdits( + new Dictionary { [SinkOwnerPath] = BuildSinkSource() }, + "DefaultIntroducing"); + HotReloadOrchestratorResult introducing = await HotReloadCompositionRoot.Services.Orchestrator.RunAsync( + new[] { SinkOwnerPath }, + contentPathOverride: null, + CancellationToken.None, + edits); + Assert.That(CountFailures(introducing), Is.EqualTo(0), DescribeOutcomes(introducing)); + } + + private static HotReloadMethodOutcomeKind FindOutcomeKind( + HotReloadOrchestratorResult result, + string methodFragment) + { + foreach (HotReloadMethodOutcome outcome in result.Methods) + { + if (outcome.Method != null && outcome.Method.Contains(methodFragment, StringComparison.Ordinal)) + { + return outcome.Kind; + } + } + + Assert.Fail("No row for " + methodFragment + ".\n" + DescribeRun(result)); + return default; + } + + private static HotReloadIntroducedTypeOutcomeKind FindIntroducedKind( + HotReloadOrchestratorResult result, + string simpleName) + { + foreach (HotReloadIntroducedTypeOutcome outcome in result.IntroducedTypes) + { + if (outcome.MetadataName != null && outcome.MetadataName.EndsWith("." + simpleName, StringComparison.Ordinal)) + { + return outcome.Kind; + } + } + + Assert.Fail("No introduced type row for " + simpleName + ".\n" + DescribeRun(result)); + return default; + } + + private static int CountRows(HotReloadOrchestratorResult result, HotReloadMethodOutcomeKind kind) + { + int count = 0; + foreach (HotReloadMethodOutcome outcome in result.Methods) + { + if (outcome.Kind == kind) + { + count++; + } + } + + return count; + } + + private static int CountWarnings(HotReloadOrchestratorResult result, string fragment) + { + int count = 0; + foreach (string warning in result.Warnings) + { + if (warning.Contains(fragment, StringComparison.Ordinal)) + { + count++; + } + } + + return count; + } + + private static string DescribeRun(HotReloadOrchestratorResult result) + { + return DescribeOutcomes(result) + "\nWarnings:\n " + string.Join("\n ", result.Warnings); + } + + private static string AddHostMembers(string hostSource, string addedMembers) + { + Assert.That(hostSource, Does.Contain(HostValueAnchor), "Precondition: host value anchor must exist."); + return hostSource.Replace(HostValueAnchor, addedMembers + HostValueAnchor, StringComparison.Ordinal); + } + + private static string InsertEnumMember(string enumSource) + { + Assert.That(enumSource, Does.Contain(EnumLastMemberAnchor), "Precondition: enum anchor must exist."); + return enumSource.Replace( + EnumLastMemberAnchor, + EnumLastMemberAnchor + ",\n Third = 3", + StringComparison.Ordinal); + } + + private static string BuildSinkSource() + { + return + "namespace " + Namespace + "\n" + + "{\n" + + " public static class " + SinkSimpleName + "\n" + + " {\n" + + " public static int Take(HotReloadSiblingEnum kind)\n" + + " {\n" + + " return (int)kind;\n" + + " }\n" + + " }\n" + + "}\n"; + } + + private static string BuildHelperSource() + { + return + "\nnamespace " + Namespace + "\n" + + "{\n" + + " public static class " + HelperSimpleName + "\n" + + " {\n" + + " public static int Seven()\n" + + " {\n" + + " return 7;\n" + + " }\n" + + " }\n" + + "}\n"; + } + + private static Dictionary WriteEdits(Dictionary sources, string label) + { + Dictionary edits = new Dictionary(); + foreach (KeyValuePair source in sources) + { + edits[source.Key] = HotReloadTestSourceWriter.WriteEditedSource( + Path.GetFileNameWithoutExtension(source.Key) + label + ".cs", + source.Value); + } + + return edits; + } + + private static Task RunDefaultSelectionAsync( + string[] files, + Dictionary edits) + { + return HotReloadCompositionRoot.Services.Orchestrator.RunAsync( + files, + contentPathOverride: null, + CancellationToken.None, + new Dictionary(edits), + isDefaultSelection: true); + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadDefaultSelectionEnumLeaveOutE2ETests.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadDefaultSelectionEnumLeaveOutE2ETests.cs.meta new file mode 100644 index 000000000..d386f4aed --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadDefaultSelectionEnumLeaveOutE2ETests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a966af92057a9495f8e411cf76d1232d +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadEnumMemberOnlyLeaveOutTests.cs b/Assets/Tests/Editor/HotReload/HotReloadEnumMemberOnlyLeaveOutTests.cs new file mode 100644 index 000000000..177fc0bd2 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadEnumMemberOnlyLeaveOutTests.cs @@ -0,0 +1,477 @@ +using System; +using System.Collections.Generic; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// Covers which files a default-selection group leaves out after its first transform run, and + /// the worker input of the rerun without them. + /// + public sealed class HotReloadEnumMemberOnlyLeaveOutTests + { + private const string EnumPath = "Assets/Scripts/Kind.cs"; + private const string OtherEnumPath = "Assets/Scripts/Mode.cs"; + private const string CallerPath = "Assets/Scripts/Caller.cs"; + private const string ApiPath = "Assets/Scripts/Registry.cs"; + private const string EnumMemberName = "Game.Kind.Third"; + + /// + /// The one change that keeps a file from being an enum-only file, for the case that + /// breaks exactly one condition. + /// + public enum OtherChange + { + Entry, + OwnSkippedRow, + AddedField, + AddedConst, + RemovedMember, + RemovedMethodSignature, + ParseError, + NoSourceHash, + NoEnumMember, + NewType + } + + private readonly HotReloadEnumMemberOnlyLeaveOut _leaveOut = new HotReloadEnumMemberOnlyLeaveOut(); + + /// + /// What: an enum-only file that a carried-in row names as the source of the type it splits + /// is left out. + /// + [Test] + public void FindLeftOutPaths_EnumOnlyFileNamedByACarriedInRow_FindsIt() + { + IReadOnlyList leftOut = _leaveOut.FindLeftOutPaths( + new[] { DefaultFile(EnumPath), DefaultFile(CallerPath) }, + Output(new[] { EnumOnly(EnumPath), Caller(CallerPath) }, CarriedInRow(CallerPath, EnumPath)), + NoActivePaths()); + + Assert.That(leftOut, Is.EqualTo(new[] { EnumPath })); + } + + /// + /// What: an enum-only file that a compiled-signature row names as the source of the type it + /// splits is left out, even though that row's declaring files name the compiled API. + /// + [Test] + public void FindLeftOutPaths_CompiledSignatureRowNamingTheFile_FindsIt() + { + IReadOnlyList leftOut = _leaveOut.FindLeftOutPaths( + new[] { DefaultFile(EnumPath), DefaultFile(CallerPath) }, + Output(new[] { EnumOnly(EnumPath), Caller(CallerPath) }, CompiledSignatureRow(CallerPath, EnumPath)), + NoActivePaths()); + + Assert.That(leftOut, Is.EqualTo(new[] { EnumPath })); + } + + /// + /// What: a compiled-signature row that names no source file of the run leaves nothing out, + /// because only its declaring files, which name the compiled API, are known. + /// + [Test] + public void FindLeftOutPaths_CompiledSignatureRowWithoutSplitSourceFiles_FindsNone() + { + TransformWorkerSkippedDto row = CompiledSignatureRow(CallerPath, EnumPath); + row.reason.splitSourceFiles = null; + + IReadOnlyList leftOut = _leaveOut.FindLeftOutPaths( + new[] { DefaultFile(EnumPath), DefaultFile(CallerPath) }, + Output(new[] { EnumOnly(EnumPath), Caller(CallerPath) }, row), + NoActivePaths()); + + Assert.That(leftOut, Is.Empty); + } + + /// + /// What: a skipped row of another code leaves nothing out, even when its declaring files + /// happen to name the enum-only file. + /// + [Test] + public void FindLeftOutPaths_SplitRowOfAnotherCode_FindsNone() + { + TransformWorkerSkippedDto row = CarriedInRow(CallerPath, EnumPath); + row.reason.code = HotReloadWorkerReasonCode.AddedMethodBodyUnbound; + + IReadOnlyList leftOut = _leaveOut.FindLeftOutPaths( + new[] { DefaultFile(EnumPath), DefaultFile(CallerPath) }, + Output(new[] { EnumOnly(EnumPath), Caller(CallerPath) }, row), + NoActivePaths()); + + Assert.That(leftOut, Is.Empty); + } + + /// + /// What: an enum-only file is kept when the split row names another file as the source of + /// its type, since leaving it out would not resolve that row. + /// + [Test] + public void FindLeftOutPaths_SplitRowNamingAnotherFile_FindsNone() + { + IReadOnlyList leftOut = _leaveOut.FindLeftOutPaths( + new[] { DefaultFile(EnumPath), DefaultFile(OtherEnumPath), DefaultFile(CallerPath) }, + Output( + new[] { EnumOnly(EnumPath), Caller(OtherEnumPath), Caller(CallerPath) }, + CarriedInRow(CallerPath, OtherEnumPath)), + NoActivePaths()); + + Assert.That(leftOut, Is.Empty); + } + + /// + /// What: of two enum-only files, only the one a split row names is left out. + /// + [Test] + public void FindLeftOutPaths_TwoEnumOnlyFilesOneNamed_FindsOnlyTheNamedFile() + { + IReadOnlyList leftOut = _leaveOut.FindLeftOutPaths( + new[] { DefaultFile(EnumPath), DefaultFile(OtherEnumPath), DefaultFile(CallerPath) }, + Output( + new[] { EnumOnly(EnumPath), EnumOnly(OtherEnumPath), Caller(CallerPath) }, + CarriedInRow(CallerPath, OtherEnumPath)), + NoActivePaths()); + + Assert.That(leftOut, Is.EqualTo(new[] { OtherEnumPath })); + } + + /// + /// What: two enum-only files named by split rows are both left out, in the order of the files. + /// + [Test] + public void FindLeftOutPaths_TwoEnumOnlyFilesBothNamed_FindsBothInFileOrder() + { + IReadOnlyList leftOut = _leaveOut.FindLeftOutPaths( + new[] { DefaultFile(CallerPath), DefaultFile(OtherEnumPath), DefaultFile(EnumPath) }, + Output( + new[] { Caller(CallerPath), EnumOnly(OtherEnumPath), EnumOnly(EnumPath) }, + CarriedInRow(CallerPath, EnumPath), + CompiledSignatureRow(CallerPath, OtherEnumPath)), + NoActivePaths()); + + Assert.That(leftOut, Is.EqualTo(new[] { OtherEnumPath, EnumPath })); + } + + /// + /// What: an enum-only file with no split row naming it stays in the run. + /// + [Test] + public void FindLeftOutPaths_NoSplitRow_FindsNone() + { + IReadOnlyList leftOut = _leaveOut.FindLeftOutPaths( + new[] { DefaultFile(EnumPath), DefaultFile(CallerPath) }, + Output(new[] { EnumOnly(EnumPath), Caller(CallerPath) }), + NoActivePaths()); + + Assert.That(leftOut, Is.Empty); + } + + /// + /// What: a file the caller passed, or one the run brought back as a sibling, is never left + /// out, because only a default selection chose files the caller did not name. + /// + [Test] + public void FindLeftOutPaths_FileNotDefaultSelected_FindsNone() + { + IReadOnlyList leftOut = _leaveOut.FindLeftOutPaths( + new[] { new HotReloadLeaveOutFile(EnumPath, false, false), DefaultFile(CallerPath) }, + Output(new[] { EnumOnly(EnumPath), Caller(CallerPath) }, CarriedInRow(CallerPath, EnumPath)), + NoActivePaths()); + + Assert.That(leftOut, Is.Empty); + } + + /// + /// What: a file that holds changes of its own in the domain stays in the run, because the + /// sibling plan would bring it back and its step is a compile, not leaving it out. + /// + [Test] + public void FindLeftOutPaths_ActiveFile_FindsNone() + { + IReadOnlyList leftOut = _leaveOut.FindLeftOutPaths( + new[] { DefaultFile(EnumPath), DefaultFile(CallerPath) }, + Output(new[] { EnumOnly(EnumPath), Caller(CallerPath) }, CarriedInRow(CallerPath, EnumPath)), + new HashSet(StringComparer.Ordinal) { EnumPath }); + + Assert.That(leftOut, Is.Empty); + } + + /// + /// What: unchanged-method rows of the file do not keep it in the run, since they apply nothing. + /// + [Test] + public void FindLeftOutPaths_UnchangedRowsOfTheFile_StillFindsIt() + { + TransformWorkerOutputDto output = Output( + new[] { EnumOnly(EnumPath), Caller(CallerPath) }, + CarriedInRow(CallerPath, EnumPath)); + output.unchangedMethods = new[] + { + new TransformWorkerUnchangedMethodDto + { + sourceProjectRelativePath = EnumPath, + typeMetadataName = "Game.KindNames", + methodName = "Describe", + parameterTypeFullNames = Array.Empty() + } + }; + + IReadOnlyList leftOut = _leaveOut.FindLeftOutPaths( + new[] { DefaultFile(EnumPath), DefaultFile(CallerPath) }, + output, + NoActivePaths()); + + Assert.That(leftOut, Is.EqualTo(new[] { EnumPath })); + } + + /// + /// What: a file with any change besides added enum members stays in the run, one condition + /// at a time, because leaving it out would drop that change. + /// + [TestCase(OtherChange.Entry)] + [TestCase(OtherChange.OwnSkippedRow)] + [TestCase(OtherChange.AddedField)] + [TestCase(OtherChange.AddedConst)] + [TestCase(OtherChange.RemovedMember)] + [TestCase(OtherChange.RemovedMethodSignature)] + [TestCase(OtherChange.ParseError)] + [TestCase(OtherChange.NoSourceHash)] + [TestCase(OtherChange.NoEnumMember)] + [TestCase(OtherChange.NewType)] + public void FindLeftOutPaths_FileWithAnotherChange_FindsNone(OtherChange change) + { + TransformWorkerFileOutputDto enumOutput = EnumOnly(EnumPath); + List skipped = new List + { + CarriedInRow(CallerPath, EnumPath) + }; + List entries = new List(); + bool declaresNewType = false; + switch (change) + { + case OtherChange.Entry: + entries.Add(new TransformWorkerEntryDto { sourceProjectRelativePath = EnumPath, methodName = "Describe" }); + break; + case OtherChange.OwnSkippedRow: + skipped.Add(new TransformWorkerSkippedDto + { + sourceProjectRelativePath = EnumPath, + method = "Game.KindNames.Describe()", + reason = new TransformWorkerReasonDto { code = HotReloadWorkerReasonCode.AddedMethodBodyUnbound } + }); + break; + case OtherChange.AddedField: + enumOutput.addedFieldNames = new[] { "Game.KindNames.count" }; + break; + case OtherChange.AddedConst: + enumOutput.addedConstNames = new[] { "Game.KindNames.Limit" }; + break; + case OtherChange.RemovedMember: + enumOutput.removedMembers = new[] { new TransformWorkerRemovedMemberDto() }; + break; + case OtherChange.RemovedMethodSignature: + enumOutput.removedMethodSignatures = new[] { new TransformWorkerRemovedMethodSignatureDto() }; + break; + case OtherChange.ParseError: + enumOutput.parseErrors = new[] { "CS1002: ; expected" }; + break; + case OtherChange.NoSourceHash: + enumOutput.sourceContentSha256 = string.Empty; + break; + case OtherChange.NoEnumMember: + enumOutput.addedEnumMemberNames = Array.Empty(); + break; + case OtherChange.NewType: + declaresNewType = true; + break; + default: + throw new ArgumentOutOfRangeException(nameof(change), change, null); + } + + TransformWorkerOutputDto output = Output(new[] { enumOutput, Caller(CallerPath) }, skipped.ToArray()); + output.entries = entries.ToArray(); + + IReadOnlyList leftOut = _leaveOut.FindLeftOutPaths( + new[] { new HotReloadLeaveOutFile(EnumPath, true, declaresNewType), DefaultFile(CallerPath) }, + output, + NoActivePaths()); + + Assert.That(leftOut, Is.Empty); + } + + /// + /// What: the rerun's input drops only the left-out sources, keeps the others in order, and + /// carries every other field of the first input over unchanged, so the rerun binds the same + /// artifacts, siblings and labels as the first run. + /// + [Test] + public void BuildRetryInput_LeavesOutOnlyTheNamedSourcesAndCopiesTheRest() + { + TransformWorkerSourceDto caller = Source(CallerPath); + TransformWorkerSourceDto enumSource = Source(EnumPath); + TransformWorkerSourceDto other = Source(OtherEnumPath); + TransformWorkerInputDto first = new TransformWorkerInputDto + { + // A prepare operation is never sent with a transform input; it is set here only to + // show the rerun is a transform whatever the first input carries. + operation = "prepareIntroducedTypes", + sources = new[] { caller, enumSource, other }, + defines = new[] { "UNITY_EDITOR" }, + referencePaths = new[] { "/refs/UnityEngine.dll" }, + targetTypesAssemblyPath = "/Library/ScriptAssemblies/Game.dll", + targetAssemblyName = "Game", + targetAssemblyMvid = "mvid", + excludedMethodKeys = new[] { "excluded" }, + excludedAddedMethodKeys = new[] { "excludedAdded" }, + assemblySourcePaths = new[] { "/project/" + EnumPath }, + changedSiblingSourcePaths = new[] { "/project/Assets/Scripts/Sibling.cs" }, + introducedTypeArtifacts = new[] { new TransformWorkerIntroducedTypeArtifactDto() }, + activeMethodLabels = new[] { "Game.Caller.Run()" } + }; + + TransformWorkerInputDto retry = _leaveOut.BuildRetryInput(first, new[] { EnumPath }); + + Assert.That(retry, Is.Not.SameAs(first)); + Assert.That(retry.sources, Is.EqualTo(new[] { caller, other })); + Assert.That(retry.operation, Is.Null); + Assert.That(retry.defines, Is.SameAs(first.defines)); + Assert.That(retry.referencePaths, Is.SameAs(first.referencePaths)); + Assert.That(retry.targetTypesAssemblyPath, Is.EqualTo(first.targetTypesAssemblyPath)); + Assert.That(retry.targetAssemblyName, Is.EqualTo(first.targetAssemblyName)); + Assert.That(retry.targetAssemblyMvid, Is.EqualTo(first.targetAssemblyMvid)); + Assert.That(retry.excludedMethodKeys, Is.SameAs(first.excludedMethodKeys)); + Assert.That(retry.excludedAddedMethodKeys, Is.SameAs(first.excludedAddedMethodKeys)); + Assert.That(retry.assemblySourcePaths, Is.SameAs(first.assemblySourcePaths)); + Assert.That(retry.changedSiblingSourcePaths, Is.SameAs(first.changedSiblingSourcePaths)); + Assert.That(retry.introducedTypeArtifacts, Is.SameAs(first.introducedTypeArtifacts)); + Assert.That(retry.activeMethodLabels, Is.SameAs(first.activeMethodLabels)); + } + + /// + /// What: a left-out path that is not among the first run's sources stops the rerun, since + /// the rerun would otherwise report a different set of files than the group holds. + /// + [Test] + public void BuildRetryInput_PathNotAmongTheSources_Throws() + { + TransformWorkerInputDto first = new TransformWorkerInputDto + { + sources = new[] { Source(CallerPath), Source(EnumPath) } + }; + + Assert.Throws(() => _leaveOut.BuildRetryInput(first, new[] { OtherEnumPath })); + } + + /// + /// What: leaving out every source stops the rerun, since a group result needs a file the + /// rerun reports on. + /// + [Test] + public void BuildRetryInput_LeavingOutEverySource_Throws() + { + TransformWorkerInputDto first = new TransformWorkerInputDto + { + sources = new[] { Source(EnumPath) } + }; + + Assert.Throws(() => _leaveOut.BuildRetryInput(first, new[] { EnumPath })); + } + + /// + /// What: the left-out warning names the file, its added enum members, and the compile + /// that adds them. + /// + [Test] + public void FormatLeftOutWarning_NamesTheFileItsMembersAndTheCompile() + { + string warning = _leaveOut.FormatLeftOutWarning(EnumPath, new[] { EnumMemberName, "Game.Kind.Fourth" }); + + Assert.That(warning, Does.StartWith("Left '" + EnumPath + "' out of this reload")); + Assert.That(warning, Does.Contain(EnumMemberName + ", Game.Kind.Fourth")); + Assert.That(warning, Does.Contain("'uloop compile'")); + } + + private static HotReloadLeaveOutFile DefaultFile(string path) + { + return new HotReloadLeaveOutFile(path, true, false); + } + + private static HashSet NoActivePaths() + { + return new HashSet(StringComparer.Ordinal); + } + + private static TransformWorkerSourceDto Source(string path) + { + return new TransformWorkerSourceDto { sourcePath = "/project/" + path, projectRelativePath = path }; + } + + private static TransformWorkerFileOutputDto EnumOnly(string path) + { + TransformWorkerFileOutputDto output = Caller(path); + output.addedEnumMemberNames = new[] { EnumMemberName }; + return output; + } + + private static TransformWorkerFileOutputDto Caller(string path) + { + return new TransformWorkerFileOutputDto + { + projectRelativePath = path, + sourceContentSha256 = "hash-of-" + path, + parseErrors = Array.Empty(), + declarationDriftWarnings = Array.Empty(), + removedMembers = Array.Empty(), + removedMethodSignatures = Array.Empty(), + addedFieldNames = Array.Empty(), + addedConstNames = Array.Empty(), + addedEnumMemberNames = Array.Empty() + }; + } + + private static TransformWorkerSkippedDto CarriedInRow(string callerPath, string declaringFile) + { + return new TransformWorkerSkippedDto + { + sourceProjectRelativePath = callerPath, + method = "Game.Caller.TakeKind()", + reason = new TransformWorkerReasonDto + { + code = HotReloadWorkerReasonCode.AddedMethodCallsIntroducedMemberBoundToCompiledType, + declaringFiles = new[] { declaringFile } + } + }; + } + + private static TransformWorkerSkippedDto CompiledSignatureRow(string callerPath, string splitSourceFile) + { + return new TransformWorkerSkippedDto + { + sourceProjectRelativePath = callerPath, + method = "Game.Caller.AcceptKind()", + reason = new TransformWorkerReasonDto + { + code = HotReloadWorkerReasonCode.AddedMethodBodyBindsCompiledSignature, + declaringFiles = new[] { ApiPath }, + splitSourceFiles = new[] { splitSourceFile } + } + }; + } + + private static TransformWorkerOutputDto Output( + TransformWorkerFileOutputDto[] files, + params TransformWorkerSkippedDto[] skipped) + { + return new TransformWorkerOutputDto + { + files = files, + entries = Array.Empty(), + skipped = skipped, + unchangedMethods = Array.Empty() + }; + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadEnumMemberOnlyLeaveOutTests.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadEnumMemberOnlyLeaveOutTests.cs.meta new file mode 100644 index 000000000..ef0d55097 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadEnumMemberOnlyLeaveOutTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: c48e9367347f64e99a1aa50a3add60ce +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadGroupProcessorLeaveOutTests.cs b/Assets/Tests/Editor/HotReload/HotReloadGroupProcessorLeaveOutTests.cs new file mode 100644 index 000000000..ad6ad81f0 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadGroupProcessorLeaveOutTests.cs @@ -0,0 +1,557 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +using NUnit.Framework; + +using UnityEditor.Compilation; +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// Covers how a group leaves out a default-selected file whose only change is added enum + /// members: how often the worker runs, what the rest of the group receives, and the results + /// the group returns. + /// + public sealed class HotReloadGroupProcessorLeaveOutTests + { + private const string AssemblyName = "UnityCLILoop.Tests.Editor.HotReload"; + private const string EnumPath = "Assets/Tests/Editor/HotReload/LeaveOutKind.cs"; + private const string OtherEnumPath = "Assets/Tests/Editor/HotReload/LeaveOutMode.cs"; + private const string CallerPath = "Assets/Tests/Editor/HotReload/LeaveOutCaller.cs"; + private const string OtherCallerPath = "Assets/Tests/Editor/HotReload/LeaveOutOtherCaller.cs"; + private const string SiblingPath = "Assets/Tests/Editor/HotReload/LeaveOutSibling.cs"; + private const string EnumMemberName = "LeaveOut.Kind.Third"; + private const string EnumDriftWarning = + "enum member LeaveOut.Kind.Third exists only in the edited source, not in the compiled assembly."; + private const string LeftOutWarningStart = "Left '" + EnumPath + "' out of this reload"; + private const string FileLevelRowLabel = "(file)"; + + /// + /// Which flag says the enum-only file also declares a new type. + /// + public enum NewTypeFlag + { + Introduced, + Refused + } + + private HotReloadDomainTestScope _scope; + private string _projectRoot; + private Assembly _compilationAssembly; + + [SetUp] + public void SetUp() + { + _scope = new HotReloadDomainTestScope(); + _projectRoot = Path.GetFullPath(Path.Combine(Application.dataPath, "..")); + _compilationAssembly = FindCompilationAssembly(); + } + + [TearDown] + public void TearDown() + { + _scope.Dispose(); + } + + /// + /// What: an enum-only file that no skipped row names stays in the run, and the worker runs once. + /// + [Test] + public async Task ProcessGroupAsync_DefaultSelectedEnumOnlyFileWithoutASplitRow_RunsTheWorkerOnce() + { + HotReloadGroupFile enumFile = CreateDefaultFile(EnumPath); + HotReloadGroupFile callerFile = CreateDefaultFile(CallerPath); + FakeWorker worker = new FakeWorker(EnumPath) { ReportsSplit = false }; + GateRecorder gate = new GateRecorder(); + + await RunGroupAsync(new[] { enumFile, callerFile }, worker, gate); + + Assert.That(worker.Inputs, Has.Count.EqualTo(1)); + Assert.That(gate.Contexts, Has.Count.EqualTo(1)); + Assert.That(gate.Contexts[0].Files, Is.EqualTo(new[] { enumFile, callerFile })); + } + + /// + /// What: an enum-only file that holds an added member in the domain stays in the run, since + /// the group reads which files hold changes from the domain itself. + /// + [Test] + public async Task ProcessGroupAsync_DefaultSelectedEnumOnlyFileHoldingAnAddedMember_KeepsItInTheRun() + { + SeedActiveAddedMember(EnumPath); + HotReloadGroupFile enumFile = CreateDefaultFile(EnumPath); + HotReloadGroupFile callerFile = CreateDefaultFile(CallerPath); + FakeWorker worker = new FakeWorker(EnumPath); + GateRecorder gate = new GateRecorder(); + + await RunGroupAsync(new[] { enumFile, callerFile }, worker, gate); + + Assert.That(worker.Inputs, Has.Count.EqualTo(1)); + Assert.That(gate.Contexts[0].Files, Is.EqualTo(new[] { enumFile, callerFile })); + } + + /// + /// What: an enum-only file that also declares a new type, introduced or refused, stays in + /// the run, since leaving the owner out would drop its type from the group. + /// + [TestCase(NewTypeFlag.Introduced)] + [TestCase(NewTypeFlag.Refused)] + public async Task ProcessGroupAsync_DefaultSelectedEnumOnlyFileDeclaringANewType_KeepsItInTheRun(NewTypeFlag flag) + { + HotReloadGroupFile enumFile = CreateDefaultFile(EnumPath); + enumFile.DeclaresIntroducedType = flag == NewTypeFlag.Introduced; + enumFile.DeclaresRefusedIntroducedType = flag == NewTypeFlag.Refused; + HotReloadGroupFile callerFile = CreateDefaultFile(CallerPath); + FakeWorker worker = new FakeWorker(EnumPath); + GateRecorder gate = new GateRecorder(); + + await RunGroupAsync(new[] { enumFile, callerFile }, worker, gate); + + Assert.That(worker.Inputs, Has.Count.EqualTo(1)); + Assert.That(gate.Contexts[0].Files, Is.EqualTo(new[] { enumFile, callerFile })); + } + + /// + /// What: when the rerun without the enum-only file fails, the rest of the group fails with + /// the rerun's error, the left-out file keeps its left-out result, and the results stay in + /// the group's order. + /// + [Test] + public async Task ProcessGroupAsync_LeaveOutRetryFails_FailsTheRestAndKeepsTheOrder() + { + HotReloadGroupFile enumFile = CreateDefaultFile(EnumPath); + HotReloadGroupFile callerFile = CreateDefaultFile(CallerPath); + FakeWorker worker = new FakeWorker(EnumPath) { RetryFailure = "rerun failed" }; + GateRecorder gate = new GateRecorder(); + + IReadOnlyList results = + await RunGroupAsync(new[] { enumFile, callerFile }, worker, gate); + + Assert.That(worker.Inputs, Has.Count.EqualTo(2)); + Assert.That(gate.Contexts, Is.Empty); + AssertResultsFollow(results, new[] { enumFile, callerFile }); + Assert.That(CountFileFailedRows(callerFile), Is.EqualTo(1)); + Assert.That(CountFileFailedRows(enumFile), Is.EqualTo(0)); + Assert.That(CountWarnings(results[0], LeftOutWarningStart), Is.EqualTo(1)); + } + + /// + /// What: a cancellation during the rerun propagates and stops the group before the gate, so + /// nothing is reverted or applied. + /// + [Test] + public async Task ProcessGroupAsync_LeaveOutRetryCancelled_ThrowsWithoutReachingTheGate() + { + HotReloadGroupFile enumFile = CreateDefaultFile(EnumPath); + HotReloadGroupFile callerFile = CreateDefaultFile(CallerPath); + FakeWorker worker = new FakeWorker(EnumPath) { CancelsRetry = true }; + GateRecorder gate = new GateRecorder(); + + // Why await and catch instead of Assert.ThrowsAsync: that one waits synchronously on + // the main thread, which the EditMode guardrails rule out. + OperationCanceledException cancellation = null; + try + { + await RunGroupAsync(new[] { enumFile, callerFile }, worker, gate); + } + catch (OperationCanceledException exception) + { + cancellation = exception; + } + + Assert.That(cancellation, Is.Not.Null, "The cancellation must propagate out of the group."); + Assert.That(worker.Inputs, Has.Count.EqualTo(2)); + Assert.That(gate.Contexts, Is.Empty); + } + + /// + /// What: two enum-only files named by the split are both left out by one rerun. + /// + [Test] + public async Task ProcessGroupAsync_TwoEnumOnlyFiles_LeavesBothOutInOneRetry() + { + HotReloadGroupFile enumFile = CreateDefaultFile(EnumPath); + HotReloadGroupFile otherEnumFile = CreateDefaultFile(OtherEnumPath); + HotReloadGroupFile callerFile = CreateDefaultFile(CallerPath); + FakeWorker worker = new FakeWorker(EnumPath, OtherEnumPath); + GateRecorder gate = new GateRecorder(); + + IReadOnlyList results = + await RunGroupAsync(new[] { enumFile, otherEnumFile, callerFile }, worker, gate); + + Assert.That(worker.Inputs, Has.Count.EqualTo(2)); + Assert.That(CollectSourcePaths(worker.Inputs[1]), Is.EqualTo(new[] { CallerPath })); + Assert.That(gate.Contexts[0].Files, Is.EqualTo(new[] { callerFile })); + AssertResultsFollow(results, new[] { enumFile, otherEnumFile, callerFile }); + } + + /// + /// What: wherever the left-out file sits among the inputs, including right before a + /// re-applied sibling, each result stays at its file's position and the rest of the group + /// keeps its order. + /// + [TestCase(0)] + [TestCase(1)] + [TestCase(2)] + public async Task ProcessGroupAsync_EnumOnlyFileLeftOut_ReturnsResultsInInputOrder(int enumPosition) + { + HotReloadGroupFile callerFile = CreateDefaultFile(CallerPath); + HotReloadGroupFile otherCallerFile = CreateDefaultFile(OtherCallerPath); + List remaining = new List { callerFile, otherCallerFile }; + HotReloadGroupFile enumFile = CreateDefaultFile(EnumPath); + List group = new List(remaining); + group.Insert(enumPosition, enumFile); + HotReloadGroupFile sibling = CreateSibling(callerFile, SiblingPath); + group.Add(sibling); + remaining.Add(sibling); + FakeWorker worker = new FakeWorker(EnumPath); + GateRecorder gate = new GateRecorder(); + + IReadOnlyList results = await RunGroupAsync(group, worker, gate); + + AssertResultsFollow(results, group); + Assert.That(gate.Contexts[0].Files, Is.EqualTo(remaining)); + Assert.That( + CollectSourcePaths(worker.Inputs[1]), + Is.EqualTo(new[] { CallerPath, OtherCallerPath, SiblingPath })); + } + + /// + /// What: the rest of the group is handed the rerun's files, input and output, not the first + /// run's, so every later stage and its own retries see the group without the left-out file. + /// + [Test] + public async Task ProcessGroupAsync_EnumOnlyFileLeftOut_HandsTheBodyTheRetryInputAndOutput() + { + HotReloadGroupFile enumFile = CreateDefaultFile(EnumPath); + HotReloadGroupFile callerFile = CreateDefaultFile(CallerPath); + FakeWorker worker = new FakeWorker(EnumPath); + GateRecorder gate = new GateRecorder(); + + await RunGroupAsync(new[] { enumFile, callerFile }, worker, gate); + + Assert.That(worker.Inputs, Has.Count.EqualTo(2)); + Assert.That(CollectSourcePaths(worker.Inputs[1]), Is.EqualTo(new[] { CallerPath })); + Assert.That(worker.Inputs[1].changedSiblingSourcePaths, Is.SameAs(worker.Inputs[0].changedSiblingSourcePaths)); + Assert.That(gate.Contexts, Has.Count.EqualTo(1)); + HotReloadApplyContext context = gate.Contexts[0]; + Assert.That(context.Files, Is.EqualTo(new[] { callerFile })); + Assert.That(context.WorkerInput, Is.SameAs(worker.Inputs[1])); + Assert.That(context.WorkerOutput, Is.SameAs(worker.Outputs[1])); + } + + /// + /// What: the left-out file's result carries what the first run reported for it — its hash, + /// added enum members, unchanged count and drift warning, each once — plus one warning that + /// says it was left out. + /// + [Test] + public async Task ProcessGroupAsync_EnumOnlyFileLeftOut_KeepsItsFirstPassResultAndWarnings() + { + HotReloadGroupFile enumFile = CreateDefaultFile(EnumPath); + HotReloadGroupFile callerFile = CreateDefaultFile(CallerPath); + FakeWorker worker = new FakeWorker(EnumPath); + GateRecorder gate = new GateRecorder(); + + IReadOnlyList results = + await RunGroupAsync(new[] { enumFile, callerFile }, worker, gate); + + HotReloadFileProcessResult enumResult = results[0]; + Assert.That(enumResult.SourceContentSha256, Is.EqualTo(HashOf(EnumPath))); + Assert.That(enumResult.AddedEnumMemberNames, Is.EqualTo(new[] { EnumMemberName })); + Assert.That(enumResult.UnchangedMethodCount, Is.EqualTo(1)); + Assert.That(enumResult.PatchedCount, Is.EqualTo(0)); + Assert.That(CountWarnings(enumResult, EnumDriftWarning), Is.EqualTo(1)); + Assert.That(CountWarnings(enumResult, LeftOutWarningStart), Is.EqualTo(1)); + Assert.That(CountWarnings(results[1], LeftOutWarningStart), Is.EqualTo(0)); + Assert.That(enumResult.Outcomes, Is.Empty); + } + + private async Task> RunGroupAsync( + IReadOnlyList files, + FakeWorker worker, + GateRecorder gate) + { + using (HotReloadServicesTestScope.BeginWithDependencies(collaborators => + HotReloadGroupProcessorDependencies.Create( + groupFiles => true, + (groupFiles, input, ct) => Task.FromResult( + HotReloadIntroducedTypePreparationResult.NoIntroducedTypes()), + worker.RunAsync, + gate.RecordAsync, + (context, compileResult, entriesToPatch) => HotReloadGroupEntryPreparation.PrepareGroup( + collaborators, context, compileResult, entriesToPatch), + collaborators.EntryApplier.ApplyPreparedEntries))) + { + return await HotReloadCompositionRoot.Services.GroupProcessor.ProcessGroupAsync( + files, + "leave-out-test", + CancellationToken.None); + } + } + + private HotReloadGroupFile CreateDefaultFile(string path) + { + HotReloadGroupFile file = new HotReloadGroupFile( + path, + WriteWorkerSource(path), + path, + AssemblyName, + _compilationAssembly, + HotReloadTypeHome.ScriptAssembliesUnderProject(_projectRoot, AssemblyName), + _projectRoot, + new HotReloadFileSinks(new List(), null, null)); + file.IsDefaultSelected = true; + return file; + } + + private static HotReloadGroupFile CreateSibling(HotReloadGroupFile template, string path) + { + return HotReloadGroupFile.ForActiveSibling( + template, + path, + WriteWorkerSource(path), + new HotReloadFileSinks(new List(), null, null), + null, + new HotReloadSiblingBaselineNotices()); + } + + private static string WriteWorkerSource(string path) + { + return HotReloadTestSourceWriter.WriteEditedSource(Path.GetFileName(path), "// " + path + "\n"); + } + + private static void AssertResultsFollow( + IReadOnlyList results, + IReadOnlyList files) + { + Assert.That(results, Has.Count.EqualTo(files.Count)); + for (int index = 0; index < files.Count; index++) + { + Assert.That( + results[index].WorkerSourcePath, + Is.EqualTo(Path.GetFullPath(files[index].WorkerSourcePath)), + "Result " + index + " must belong to " + files[index].ProjectRelativePath + "."); + } + } + + private static List CollectSourcePaths(TransformWorkerInputDto input) + { + List paths = new List(); + foreach (TransformWorkerSourceDto source in input.sources) + { + paths.Add(source.projectRelativePath); + } + + return paths; + } + + private static int CountWarnings(HotReloadFileProcessResult result, string start) + { + int count = 0; + foreach (string warning in result.Warnings) + { + if (warning.StartsWith(start, StringComparison.Ordinal)) + { + count++; + } + } + + return count; + } + + private static int CountFileFailedRows(HotReloadGroupFile file) + { + int count = 0; + foreach (HotReloadMethodOutcome outcome in file.Sinks.Outcomes) + { + if (outcome.Kind == HotReloadMethodOutcomeKind.Failed && outcome.Method == FileLevelRowLabel) + { + count++; + } + } + + return count; + } + + private static string HashOf(string path) + { + return "hash-of-" + path; + } + + private static Assembly FindCompilationAssembly() + { + foreach (Assembly assembly in CompilationPipeline.GetAssemblies()) + { + if (assembly.name == AssemblyName) + { + return assembly; + } + } + + Assert.Fail("Compilation assembly was not found."); + return null; + } + + private static void SeedActiveAddedMember(string projectRelativePath) + { + System.Reflection.MethodInfo shimMethod = typeof(HotReloadGroupProcessorLeaveOutTests).GetMethod( + nameof(AddedMemberShim), + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static); + Assert.That(shimMethod, Is.Not.Null); + new HotReloadDomainTestAccess().RegisterAddedMember( + projectRelativePath, + "LeaveOut.Kind::Describe()", + shimMethod, + projectRelativePath); + } + + private static void AddedMemberShim() + { + } + + /// + /// Answers each run like the transform worker does for these fixtures: while an enum file + /// is in the run, the caller's added method is skipped with a row naming the enum files of + /// the run as the source of the split type; without one, nothing is skipped. + /// + private sealed class FakeWorker + { + private readonly HashSet _enumPaths; + + internal FakeWorker(params string[] enumPaths) + { + _enumPaths = new HashSet(enumPaths, StringComparer.Ordinal); + } + + internal bool ReportsSplit { get; set; } = true; + + internal string RetryFailure { get; set; } + + internal bool CancelsRetry { get; set; } + + internal List Inputs { get; } = new List(); + + internal List Outputs { get; } = new List(); + + internal Task RunAsync(TransformWorkerInputDto input, CancellationToken ct) + { + Inputs.Add(input); + bool isRetry = Inputs.Count > 1; + if (isRetry && CancelsRetry) + { + return Task.FromCanceled(new CancellationToken(true)); + } + + if (isRetry && RetryFailure != null) + { + return Task.FromResult(TransformWorkerClientResult.Failure(RetryFailure)); + } + + TransformWorkerOutputDto output = BuildOutput(input); + Outputs.Add(output); + return Task.FromResult(TransformWorkerClientResult.SuccessResult(output)); + } + + private TransformWorkerOutputDto BuildOutput(TransformWorkerInputDto input) + { + List files = new List(); + List unchanged = new List(); + List enumPathsInRun = new List(); + foreach (TransformWorkerSourceDto source in input.sources) + { + string path = source.projectRelativePath; + bool isEnum = _enumPaths.Contains(path); + files.Add(CreateFileOutput(path, isEnum)); + if (!isEnum) + { + continue; + } + + enumPathsInRun.Add(path); + unchanged.Add(new TransformWorkerUnchangedMethodDto + { + sourceProjectRelativePath = path, + typeMetadataName = "LeaveOut.KindNames", + methodName = "Describe", + parameterTypeFullNames = Array.Empty() + }); + } + + List skipped = new List(); + if (ReportsSplit && enumPathsInRun.Count > 0) + { + skipped.Add(CreateSplitRow(enumPathsInRun.ToArray())); + } + + return new TransformWorkerOutputDto + { + shimSource = string.Empty, + entries = Array.Empty(), + skipped = skipped.ToArray(), + files = files.ToArray(), + parseErrors = Array.Empty(), + siblingConstDriftWarnings = Array.Empty(), + unchangedMethods = unchanged.ToArray() + }; + } + + private static TransformWorkerFileOutputDto CreateFileOutput(string path, bool isEnum) + { + return new TransformWorkerFileOutputDto + { + projectRelativePath = path, + sourceContentSha256 = HashOf(path), + parseErrors = Array.Empty(), + declarationDriftWarnings = isEnum ? new[] { EnumDriftWarning } : Array.Empty(), + removedMembers = Array.Empty(), + removedMethodSignatures = Array.Empty(), + addedFieldNames = Array.Empty(), + addedConstNames = Array.Empty(), + addedEnumMemberNames = isEnum ? new[] { EnumMemberName } : Array.Empty(), + introducedTypes = Array.Empty(), + introducedTypeDiagnostics = Array.Empty(), + introducedTypeReuses = Array.Empty() + }; + } + + private static TransformWorkerSkippedDto CreateSplitRow(string[] enumPathsInRun) + { + return new TransformWorkerSkippedDto + { + sourceProjectRelativePath = CallerPath, + method = "LeaveOut.Caller.TakeKind(LeaveOut.Kind)", + reason = new TransformWorkerReasonDto + { + code = HotReloadWorkerReasonCode.AddedMethodCallsIntroducedMemberBoundToCompiledType, + args = new[] { "CS1503", "LeaveOut.Sink", "LeaveOut.Kind", string.Join(", ", enumPathsInRun) }, + declaringFiles = enumPathsInRun + } + }; + } + } + + /// + /// Stands in for the gate and first compile: records the context the rest of the group + /// was handed and fails, so the group builds its unapplied results without compiling. + /// + private sealed class GateRecorder + { + internal List Contexts { get; } = new List(); + + internal Task RecordAsync( + HotReloadApplyContext context, + CancellationToken ct) + { + Contexts.Add(context); + return Task.FromResult(HotReloadGroupGateAndCompileResult.Failed()); + } + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadGroupProcessorLeaveOutTests.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadGroupProcessorLeaveOutTests.cs.meta new file mode 100644 index 000000000..a7e3258f9 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadGroupProcessorLeaveOutTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: b9bf4a57cdaf14e149f86fdfcb82d231 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadServicesTestScope.cs b/Assets/Tests/Editor/HotReload/HotReloadServicesTestScope.cs index 039886585..b6ae534e7 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadServicesTestScope.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadServicesTestScope.cs @@ -159,12 +159,17 @@ internal HotReloadStubOrchestrator( _run = run; } + // Whether the last run was started as a default selection; null until a run starts. + internal bool? ReceivedIsDefaultSelection { get; private set; } + public Task RunAsync( IReadOnlyList files, string contentPathOverride, CancellationToken ct, - IReadOnlyDictionary contentPathOverrideByFile = null) + IReadOnlyDictionary contentPathOverrideByFile = null, + bool isDefaultSelection = false) { + ReceivedIsDefaultSelection = isDefaultSelection; return _run(files, ct); } } diff --git a/Assets/Tests/Editor/HotReload/TransformWorkerBindingSplitTests.cs b/Assets/Tests/Editor/HotReload/TransformWorkerBindingSplitTests.cs index b492bee12..2aeb4b1da 100644 --- a/Assets/Tests/Editor/HotReload/TransformWorkerBindingSplitTests.cs +++ b/Assets/Tests/Editor/HotReload/TransformWorkerBindingSplitTests.cs @@ -136,6 +136,30 @@ public async Task Run_HostWithThePayloadFile_NamesTheFileDeclaringTheCompiledSig Assert.That(text, Does.EndWith("so it is skipped."), text); } + /// + /// What: the same row also names the run's file that builds the payload from source, apart + /// from the file declaring the compiled API, so the Editor can tell which file of the run + /// splits the type. + /// + [Test] + public async Task Run_HostWithThePayloadFile_NamesTheFileBuildingTheSplitTypeFromSource() + { + TransformWorkerClientResult result = await RunAsync( + new[] { HostFileName, PayloadFileName }, + new[] { WithMethod(ReadOnDisk(HostFileName), WireMethod), ReadOnDisk(PayloadFileName) }); + + Assert.That(result.Success, Is.True, result.ErrorMessage); + TransformWorkerSkippedDto skipped = FindSkipped(result, "Wire"); + Assert.That(skipped, Is.Not.Null, "Missing skipped row for Wire.\n" + FormatSkipped(result)); + Assert.That(skipped.reason.code, Is.EqualTo(HotReloadWorkerReasonCode.AddedMethodBodyBindsCompiledSignature)); + Assert.That( + skipped.reason.splitSourceFiles, + Is.EqualTo(new[] { "Assets/Tests/Editor/HotReload/" + PayloadFileName })); + Assert.That( + skipped.reason.declaringFiles, + Is.EqualTo(new[] { "Assets/Tests/Editor/HotReload/" + RegistryFileName })); + } + /// /// What: passing the file that declares the compiled API as well lets the added method bind /// against the payload this run declares, so the method is applied, which is the recovery diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadDefaultFileSelection.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadDefaultFileSelection.cs index 0eb7d6229..153578c82 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadDefaultFileSelection.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadDefaultFileSelection.cs @@ -19,16 +19,22 @@ internal sealed class HotReloadDefaultFileSelection internal HotReloadValidationFailure ValidationFailure { get; } + // True when the files came from compile snapshots rather than from the caller, so the run + // may leave out a selected file the caller never named. + internal bool IsDefaultSelection { get; } + internal HotReloadDefaultFileSelection( IReadOnlyList files, IReadOnlyList scanLimitWarnings, string selectionMessage, - HotReloadValidationFailure validationFailure) + HotReloadValidationFailure validationFailure, + bool isDefaultSelection) { Files = files ?? Array.Empty(); ScanLimitWarnings = scanLimitWarnings ?? Array.Empty(); SelectionMessage = selectionMessage ?? string.Empty; ValidationFailure = validationFailure; + IsDefaultSelection = isDefaultSelection; } } @@ -85,7 +91,8 @@ private static HotReloadDefaultFileSelection SelectExplicitFiles( distinctFiles, Array.Empty(), BuildDuplicateFilesMessage(pathsInFirstSeenOrder, countByPath), - validationFailure: null); + validationFailure: null, + isDefaultSelection: false); } private static string BuildDuplicateFilesMessage( @@ -132,7 +139,8 @@ private static HotReloadDefaultFileSelection SelectChangedFiles( { "Run 'uloop compile' to create source snapshots.", HotReloadConstants.PassExplicitFilesNextAction - })); + }), + isDefaultSelection: true); } IReadOnlyList reselectedPaths = ExcludeChangedPaths( @@ -151,7 +159,8 @@ private static HotReloadDefaultFileSelection SelectChangedFiles( { "Save the edited .cs files to disk, then run 'uloop hot-reload' again.", HotReloadConstants.PassExplicitFilesNextAction - })); + }), + isDefaultSelection: true); } List selectedFiles = new List(changedFiles.ChangedProjectRelativePaths); @@ -160,7 +169,8 @@ private static HotReloadDefaultFileSelection SelectChangedFiles( selectedFiles, changedFiles.ScanLimitWarnings, BuildSelectionMessage(changedFiles.ChangedProjectRelativePaths, reselectedPaths), - validationFailure: null); + validationFailure: null, + isDefaultSelection: true); } // A discarded owner file the user has since edited is already a changed file, so only diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEnumMemberOnlyLeaveOut.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEnumMemberOnlyLeaveOut.cs new file mode 100644 index 000000000..04f09024d --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEnumMemberOnlyLeaveOut.cs @@ -0,0 +1,261 @@ +using System; +using System.Collections.Generic; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// What a default-selection group needs to know about one of its files to decide whether to + /// leave it out. + /// + internal readonly struct HotReloadLeaveOutFile + { + internal HotReloadLeaveOutFile(string projectRelativePath, bool isDefaultSelected, bool declaresNewType) + { + ProjectRelativePath = projectRelativePath; + IsDefaultSelected = isDefaultSelected; + DeclaresNewType = declaresNewType; + } + + internal string ProjectRelativePath { get; } + + internal bool IsDefaultSelected { get; } + + // Why the caller passes this instead of the worker output: only the prepare run plans new + // types, so the transform output's introduced-type fields are empty for every file. + internal bool DeclaresNewType { get; } + } + + /// + /// Decides which files of a default-selection group to leave out, and builds the worker input + /// of the rerun without them. + /// + internal sealed class HotReloadEnumMemberOnlyLeaveOut + { + // Format: project-relative path, comma-separated enum member names. + private const string LeftOutWarningFormat = + "Left '{0}' out of this reload: --files was omitted and the file only adds enum members " + + "({1}), which hot reload cannot add. Keeping it would build the enum from source and skip " + + "the added members that pass the enum to or take it from compiled code or an introduced " + + "type. Run 'uloop compile' to add the enum members."; + + /// + /// Returns, in file order, the default-selected files whose only change is added enum + /// members and that a skipped row of the first run names as the source side of a split + /// type. Leaving such a file out loses nothing hot reload could apply, and lets the added + /// members in the other files bind to the compiled enum. + /// + internal IReadOnlyList FindLeftOutPaths( + IReadOnlyList files, + TransformWorkerOutputDto firstPass, + ISet activePaths) + { + if (files == null || files.Count == 0) + { + throw new ArgumentException("A group must hold a file.", nameof(files)); + } + + if (firstPass == null) + { + throw new ArgumentNullException(nameof(firstPass)); + } + + if (activePaths == null) + { + throw new ArgumentNullException(nameof(activePaths)); + } + + HashSet namedSourceSideFiles = CollectNamedSourceSideFiles(firstPass.skipped); + if (namedSourceSideFiles.Count == 0) + { + return Array.Empty(); + } + + HotReloadWorkerRowsByFile rows = HotReloadWorkerRowsByFile.Build(firstPass, CollectPaths(files)); + List leftOutPaths = new List(); + foreach (HotReloadLeaveOutFile file in files) + { + if (!file.IsDefaultSelected) + { + continue; + } + + // Why an active file stays: the sibling plan would bring it back, and its step + // today is a compile that keeps its patches, not leaving it out. + if (activePaths.Contains(file.ProjectRelativePath)) + { + continue; + } + + if (!namedSourceSideFiles.Contains(file.ProjectRelativePath)) + { + continue; + } + + if (!OnlyAddsEnumMembers(file, rows)) + { + continue; + } + + leftOutPaths.Add(file.ProjectRelativePath); + } + + return leftOutPaths; + } + + /// + /// Copies the first run's input without the left-out sources. Every other field is shared, + /// so the rerun binds the same artifacts, siblings, exclusions and labels as the first run. + /// + internal TransformWorkerInputDto BuildRetryInput( + TransformWorkerInputDto firstInput, + IReadOnlyCollection leftOutPaths) + { + if (firstInput == null || firstInput.sources == null) + { + throw new ArgumentNullException(nameof(firstInput)); + } + + if (leftOutPaths == null || leftOutPaths.Count == 0) + { + throw new ArgumentException("A rerun must leave out a file.", nameof(leftOutPaths)); + } + + HashSet leftOutSet = new HashSet(leftOutPaths, StringComparer.Ordinal); + List remainingSources = new List(); + foreach (TransformWorkerSourceDto source in firstInput.sources) + { + if (!leftOutSet.Remove(source.projectRelativePath)) + { + remainingSources.Add(source); + } + } + + // Why throw before the rerun: a path outside the sources, or a rerun with no source, + // would report a different set of files than the group splices its results into. + if (leftOutSet.Count > 0) + { + throw new InvalidOperationException( + "A left-out file must be among the first run's sources: " + + string.Join(", ", leftOutSet)); + } + + if (remainingSources.Count == 0) + { + throw new InvalidOperationException("A rerun must keep at least one source."); + } + + return new TransformWorkerInputDto + { + // Why never the operation: the rerun is a transform, like the isolation retry. + sources = remainingSources.ToArray(), + defines = firstInput.defines, + referencePaths = firstInput.referencePaths, + targetTypesAssemblyPath = firstInput.targetTypesAssemblyPath, + targetAssemblyName = firstInput.targetAssemblyName, + targetAssemblyMvid = firstInput.targetAssemblyMvid, + excludedMethodKeys = firstInput.excludedMethodKeys, + excludedAddedMethodKeys = firstInput.excludedAddedMethodKeys, + assemblySourcePaths = firstInput.assemblySourcePaths, + // Why the left-out files are not added as siblings: the transform reads siblings + // only for const drift, which the left-out file's own notices already report. + changedSiblingSourcePaths = firstInput.changedSiblingSourcePaths, + introducedTypeArtifacts = firstInput.introducedTypeArtifacts, + activeMethodLabels = firstInput.activeMethodLabels + }; + } + + internal string FormatLeftOutWarning(string projectRelativePath, IReadOnlyList addedEnumMemberNames) + { + return string.Format( + LeftOutWarningFormat, + projectRelativePath, + string.Join(", ", addedEnumMemberNames)); + } + + // Why only the top-level reason: a composite row that carries the split in its detail + // has an owner row whose top-level reason names the same split. + private static HashSet CollectNamedSourceSideFiles(TransformWorkerSkippedDto[] skipped) + { + // Why Ordinal: the worker echoes the project-relative paths the Editor sent, the same + // strings the per-file rows are keyed by. + HashSet named = new HashSet(StringComparer.Ordinal); + if (skipped == null) + { + return named; + } + + foreach (TransformWorkerSkippedDto row in skipped) + { + string[] sourceSideFiles = SourceSideFilesOf(row.reason); + if (sourceSideFiles == null) + { + continue; + } + + named.UnionWith(sourceSideFiles); + } + + return named; + } + + // Why each code reads a different field: a carried-in row lists the run's files that build + // the bound type from source in declaringFiles, while a compiled-signature row lists the + // compiled API there and the run's source-side files in splitSourceFiles. + private static string[] SourceSideFilesOf(TransformWorkerReasonDto reason) + { + switch (reason.code) + { + case HotReloadWorkerReasonCode.AddedMethodCallsIntroducedMemberBoundToCompiledType: + return reason.declaringFiles; + case HotReloadWorkerReasonCode.AddedMethodBodyBindsCompiledSignature: + return reason.splitSourceFiles; + default: + return null; + } + } + + // Why unchanged rows and declaration drift do not count: neither applies anything, and + // the left-out file still reports its drift through its own per-file notices. + private static bool OnlyAddsEnumMembers(HotReloadLeaveOutFile file, HotReloadWorkerRowsByFile rows) + { + if (file.DeclaresNewType) + { + return false; + } + + if (rows.EntriesFor(file.ProjectRelativePath).Count > 0 + || rows.SkippedFor(file.ProjectRelativePath).Count > 0) + { + return false; + } + + TransformWorkerFileOutputDto output = rows.FileOutputFor(file.ProjectRelativePath); + if (string.IsNullOrEmpty(output.sourceContentSha256) || IsNullOrEmpty(output.addedEnumMemberNames)) + { + return false; + } + + return IsNullOrEmpty(output.addedFieldNames) + && IsNullOrEmpty(output.addedConstNames) + && IsNullOrEmpty(output.removedMembers) + && IsNullOrEmpty(output.removedMethodSignatures) + && IsNullOrEmpty(output.parseErrors); + } + + private static bool IsNullOrEmpty(T[] values) + { + return values == null || values.Length == 0; + } + + private static List CollectPaths(IReadOnlyList files) + { + List paths = new List(files.Count); + foreach (HotReloadLeaveOutFile file in files) + { + paths.Add(file.ProjectRelativePath); + } + + return paths; + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEnumMemberOnlyLeaveOut.cs.meta b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEnumMemberOnlyLeaveOut.cs.meta new file mode 100644 index 000000000..682c36d61 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEnumMemberOnlyLeaveOut.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 5f21afac214134f488e01fdad9f28eb1 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupFile.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupFile.cs index 51ad957e9..baac5866e 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupFile.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupFile.cs @@ -83,6 +83,11 @@ internal static HotReloadGroupFile ForActiveSibling( // Only ForActiveSibling sets the notices, so they mark a file the run pulled in itself. internal bool ReappliedSibling => SiblingBaselineNotices != null; + // Set for an input the run chose from compile snapshots because the caller omitted the + // files; never for a re-applied sibling. Only such a file may be left out of its group, + // because the caller did not choose to pass it. + internal bool IsDefaultSelected { get; set; } + // The path the caller asked to reload, used as the outcome file path. internal string AssemblyResolvePath { get; } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupLeaveOutSplit.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupLeaveOutSplit.cs new file mode 100644 index 000000000..3ba294dfd --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupLeaveOutSplit.cs @@ -0,0 +1,94 @@ +using System; +using System.Collections.Generic; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Splits a group into the files a default selection leaves out and the files that go on to + /// the rerun, and puts the results of both back in the group's order. + /// + /// + /// Why the order matters: the run matches each result to its file by position, so a group + /// must return one result per file in the order it received them. + /// + internal sealed class HotReloadGroupLeaveOutSplit + { + private readonly IReadOnlyList _files; + private readonly HashSet _leftOutPaths; + + internal HotReloadGroupLeaveOutSplit( + IReadOnlyList files, + IReadOnlyCollection leftOutPaths) + { + if (files == null || leftOutPaths == null) + { + throw new ArgumentNullException(files == null ? nameof(files) : nameof(leftOutPaths)); + } + + _files = files; + _leftOutPaths = new HashSet(leftOutPaths, StringComparer.Ordinal); + List leftOutFiles = new List(); + List remainingFiles = new List(); + foreach (HotReloadGroupFile file in files) + { + if (IsLeftOut(file)) + { + leftOutFiles.Add(file); + } + else + { + remainingFiles.Add(file); + } + } + + // Why throw: a left-out path the group does not hold would splice fewer results than + // the run expects, and a group left without files has nothing for the rerun to report. + if (leftOutFiles.Count != _leftOutPaths.Count || remainingFiles.Count == 0) + { + throw new InvalidOperationException( + "A leave-out must name files of the group and keep at least one of them."); + } + + LeftOutFiles = leftOutFiles; + RemainingFiles = remainingFiles; + } + + internal IReadOnlyList LeftOutFiles { get; } + + internal IReadOnlyList RemainingFiles { get; } + + internal List Splice( + IReadOnlyList leftOutResults, + IReadOnlyList remainingResults) + { + if (leftOutResults.Count != LeftOutFiles.Count || remainingResults.Count != RemainingFiles.Count) + { + throw new InvalidOperationException( + "Each file of the group must have exactly one result before the results are spliced."); + } + + List results = new List(_files.Count); + int leftOutIndex = 0; + int remainingIndex = 0; + foreach (HotReloadGroupFile file in _files) + { + if (IsLeftOut(file)) + { + results.Add(leftOutResults[leftOutIndex]); + leftOutIndex++; + continue; + } + + results.Add(remainingResults[remainingIndex]); + remainingIndex++; + } + + return results; + } + + private bool IsLeftOut(HotReloadGroupFile file) + { + return _leftOutPaths.Contains(file.ProjectRelativePath); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupLeaveOutSplit.cs.meta b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupLeaveOutSplit.cs.meta new file mode 100644 index 000000000..b94676ec7 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupLeaveOutSplit.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: fbb5de39187544acdbb27652d8b47a4e +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupProcessor.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupProcessor.cs index d8a7d44cc..ab852d9f4 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupProcessor.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupProcessor.cs @@ -28,6 +28,7 @@ internal sealed class HotReloadGroupProcessor private readonly HotReloadFileEntryApplier _fileEntryApplier; private readonly HotReloadEntryApplier _entryApplier; private readonly HotReloadGroupCommitStage _commitStage; + private readonly HotReloadEnumMemberOnlyLeaveOut _leaveOut = new HotReloadEnumMemberOnlyLeaveOut(); internal HotReloadGroupProcessor( HotReloadGroupProcessorDependencies dependencies, @@ -61,6 +62,10 @@ internal async Task> ProcessGroupAsync } SnapshotGroupState(files); + // Why here and not at the leave-out decision: the ledgers need the main thread, which + // the worker await leaves, and nothing changes which files hold changes before the + // decision — the domain changes only after it, and a prepared type is not active. + HashSet activePaths = new HotReloadDomainCarriedInLookup(_domain).ListActivePaths(); HotReloadChangedSiblingScanResult siblingScan = HotReloadChangedSiblingSourceDetector.Detect( firstFile.ProjectRoot, @@ -117,7 +122,7 @@ internal async Task> ProcessGroupAsync HotReloadRefusedIntroducedType.CollectFrom(preparation.Notices); if (preparation.Prepared == null) { - return await TransformAndApplyGroupAsync(files, workerInput, null, refusedTypes, correlationId, ct) + return await TransformAndApplyGroupAsync(files, workerInput, null, refusedTypes, activePaths, correlationId, ct) .ConfigureAwait(false); } @@ -126,6 +131,7 @@ internal async Task> ProcessGroupAsync workerInput, preparation.Prepared, refusedTypes, + activePaths, correlationId, ct).ConfigureAwait(false); } @@ -149,6 +155,7 @@ private async Task> TransformAndApplyP TransformWorkerInputDto workerInput, HotReloadPreparedIntroducedTypes prepared, IReadOnlyList refusedTypes, + HashSet activePaths, string correlationId, CancellationToken ct) { @@ -172,8 +179,14 @@ private async Task> TransformAndApplyP // the membership again, so registering anywhere the finally does not cover // would leave the run's membership behind when the scope itself throws. registry.RegisterPrepared(artifact); - return await TransformAndApplyGroupAsync(files, workerInput, prepared, refusedTypes, correlationId, ct) - .ConfigureAwait(false); + return await TransformAndApplyGroupAsync( + files, + workerInput, + prepared, + refusedTypes, + activePaths, + correlationId, + ct).ConfigureAwait(false); } finally { @@ -184,26 +197,132 @@ private async Task> TransformAndApplyP } } + /// + /// Transforms the group, and when a default selection holds a file whose added enum + /// members split a type the other files bind to, transforms the rest again without it. + /// + /// + /// Why decide after the first run and before anything else: only the worker names the + /// file behind a split, and the decision must come before the first per-file notice and + /// the first change to the domain so the rest are processed as if it was never passed. + /// private async Task> TransformAndApplyGroupAsync( IReadOnlyList files, TransformWorkerInputDto workerInput, HotReloadPreparedIntroducedTypes prepared, IReadOnlyList refusedTypes, + HashSet activePaths, + string correlationId, + CancellationToken ct) + { + TransformWorkerClientResult workerResult = await RunWorkerAsync(workerInput, correlationId, ct) + .ConfigureAwait(false); + if (!workerResult.Success) + { + return FailGroup(files, workerResult.ErrorMessage); + } + + TransformWorkerOutputDto workerOutput = workerResult.Output; + Debug.Assert( + workerOutput.files.Length == files.Count, + "A group worker run must return one per-file output per edited file."); + IReadOnlyList leftOutPaths = _leaveOut.FindLeftOutPaths( + DescribeLeaveOutFiles(files), + workerOutput, + activePaths); + if (leftOutPaths.Count == 0) + { + return await ApplyTransformedGroupAsync(files, workerInput, workerOutput, prepared, refusedTypes, correlationId, ct) + .ConfigureAwait(false); + } + + // Why the rerun input first: it refuses a leave-out the sources do not match before + // anything is written to the left-out files. + TransformWorkerInputDto retryInput = _leaveOut.BuildRetryInput(workerInput, leftOutPaths); + HotReloadGroupLeaveOutSplit split = new HotReloadGroupLeaveOutSplit(files, leftOutPaths); + List leftOutResults = BuildLeftOutResults(split, files, workerOutput); + TransformWorkerClientResult retryResult = await RunWorkerAsync(retryInput, correlationId, ct) + .ConfigureAwait(false); + IReadOnlyList remainingResults = retryResult.Success + ? await ApplyTransformedGroupAsync( + split.RemainingFiles, + retryInput, + retryResult.Output, + prepared, + refusedTypes, + correlationId, + ct).ConfigureAwait(false) + : FailGroup(split.RemainingFiles, retryResult.ErrorMessage); + return split.Splice(leftOutResults, remainingResults); + } + + private async Task RunWorkerAsync( + TransformWorkerInputDto workerInput, string correlationId, CancellationToken ct) { - HotReloadGroupFile firstFile = files[0]; TransformWorkerClientResult workerResult = await _dependencies .RunWorker(workerInput, ct) .ConfigureAwait(false); HotReloadOrchestratorLog.LogHotReloadWorkerResult(workerResult, correlationId); - if (!workerResult.Success) + return workerResult; + } + + private List FailGroup(IReadOnlyList files, string errorMessage) + { + HotReloadGroupOutcomeRouter.AppendGroupFailure(files, "(file)", errorMessage); + return _fileEntryApplier.BuildUnappliedGroupResults(files); + } + + // Why a file that declares a new type never counts as enum-only: its artifact is prepared + // against the whole group, and leaving its owner out would drop it from the commit. + private static List DescribeLeaveOutFiles(IReadOnlyList files) + { + List leaveOutFiles = new List(files.Count); + foreach (HotReloadGroupFile file in files) { - HotReloadGroupOutcomeRouter.AppendGroupFailure(files, "(file)", workerResult.ErrorMessage); - return _fileEntryApplier.BuildUnappliedGroupResults(files); + leaveOutFiles.Add(new HotReloadLeaveOutFile( + file.ProjectRelativePath, + file.IsDefaultSelected, + file.DeclaresIntroducedType || file.DeclaresRefusedIntroducedType)); } - TransformWorkerOutputDto workerOutput = workerResult.Output; + return leaveOutFiles; + } + + // Why the first run's rows: the left-out file gets the notices, hash and counts it would + // have had in the run, so only the added members of the other files change. + private List BuildLeftOutResults( + HotReloadGroupLeaveOutSplit split, + IReadOnlyList files, + TransformWorkerOutputDto firstOutput) + { + HotReloadWorkerRowsByFile firstRows = HotReloadWorkerRowsByFile.Build( + firstOutput, + CollectProjectRelativePaths(files)); + HotReloadGroupNotices.AppendPerFileWorkerNotices(split.LeftOutFiles, firstRows); + List results = new List(split.LeftOutFiles.Count); + foreach (HotReloadGroupFile file in split.LeftOutFiles) + { + file.Sinks.Warnings.Add(_leaveOut.FormatLeftOutWarning( + file.ProjectRelativePath, + file.FileOutput.addedEnumMemberNames)); + results.Add(_fileEntryApplier.BuildUnappliedResult(file)); + } + + return results; + } + + private async Task> ApplyTransformedGroupAsync( + IReadOnlyList files, + TransformWorkerInputDto workerInput, + TransformWorkerOutputDto workerOutput, + HotReloadPreparedIntroducedTypes prepared, + IReadOnlyList refusedTypes, + string correlationId, + CancellationToken ct) + { + HotReloadGroupFile firstFile = files[0]; Debug.Assert( workerOutput.files.Length == files.Count, "A group worker run must return one per-file output per edited file."); diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadOrchestrator.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadOrchestrator.cs index 9042b7334..687280f41 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadOrchestrator.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadOrchestrator.cs @@ -60,12 +60,15 @@ internal HotReloadOrchestrator( /// can live under Library/UloopHotReload/TestSources/ without provoking AssetDatabase). /// is the per-file form of that hook, keyed by /// the entry in ; it wins over the single override. + /// marks every input as chosen from compile snapshots, + /// which lets a group leave out a selected file the caller never named. /// public async Task RunAsync( IReadOnlyList files, string contentPathOverride, CancellationToken ct, - IReadOnlyDictionary contentPathOverrideByFile = null) + IReadOnlyDictionary contentPathOverrideByFile = null, + bool isDefaultSelection = false) { Debug.Assert(files != null, "files must not be null."); Debug.Assert(files.Count > 0, "files must not be empty."); @@ -108,6 +111,7 @@ public async Task RunAsync( plannerInput); } + MarkDefaultSelectedInputs(slots, isDefaultSelection); IReadOnlyList plans = HotReloadFileGroupPlanner.Plan(plannerInput); HashSet pathsInRun = new HashSet( HotReloadSourcePathNormalizer.ProjectRelativePathComparer()); @@ -192,6 +196,20 @@ await ProcessPlannedGroupAsync( return run.BuildResult(correlationId); } + // Why on the inputs only: a re-applied sibling joins a group later and was never selected, + // so it keeps the flag unset even in a default-selection run. + private void MarkDefaultSelectedInputs(HotReloadInputResolutionSlot[] slots, bool isDefaultSelection) + { + foreach (HotReloadInputResolutionSlot slot in slots) + { + // An input that failed resolution has no group file and never joins a group. + if (slot.GroupFile != null) + { + slot.GroupFile.IsDefaultSelected = isDefaultSelection; + } + } + } + private async Task ProcessPlannedGroupAsync( IReadOnlyList inputIndexes, HotReloadInputResolutionSlot[] slots, diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadTools.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadTools.cs index 1e7c04d56..393fa6847 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadTools.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadTools.cs @@ -268,7 +268,11 @@ protected override async Task ExecuteAsync( } HotReloadOrchestratorResult result = await services.Orchestrator - .RunAsync(selection.Files, contentPathOverride: null, ct) + .RunAsync( + selection.Files, + contentPathOverride: null, + ct, + isDefaultSelection: selection.IsDefaultSelection) .ConfigureAwait(false); // Why switch back: SessionState for Play-entry drop recovery is a Unity Editor API. await MainThreadSwitcher.SwitchToMainThread(ct); diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/IHotReloadOrchestrator.cs b/Packages/src/Editor/FirstPartyTools/HotReload/IHotReloadOrchestrator.cs index 842983863..c1cb87f21 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/IHotReloadOrchestrator.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/IHotReloadOrchestrator.cs @@ -10,10 +10,13 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// internal interface IHotReloadOrchestrator { + /// True when the files were chosen from compile snapshots + /// because the caller omitted them. Task RunAsync( IReadOnlyList files, string contentPathOverride, CancellationToken ct, - IReadOnlyDictionary contentPathOverrideByFile = null); + IReadOnlyDictionary contentPathOverrideByFile = null, + bool isDefaultSelection = false); } } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerDtos.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerDtos.cs index 0707b0846..5b5f91f09 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerDtos.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerDtos.cs @@ -402,6 +402,11 @@ internal sealed class TransformWorkerReasonDto // compiled assembly's debug data. A type neither can place is left out. Null until one of // them resolves the types, and for a reason that names none. public string[] declaringFiles; + + // Project-relative forward-slash paths of the run's files that build the split types from + // source, for a reason whose compiled signature still names the compiled copies. Apart from + // declaringFiles, which name the files of the compiled API instead. Null for other reasons. + public string[] splitSourceFiles; } [Serializable] diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md index fed7ea4a2..72a3dec2f 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md @@ -6,7 +6,7 @@ Returns JSON with: - `ErrorCode` (string, optional): Present on parameter validation failure. Values are `HOT_RELOAD_FILES_REQUIRED` when an omitted apply has no compile snapshots, `HOT_RELOAD_NO_CHANGED_FILES` when snapshots contain no changed `.cs` files, `HOT_RELOAD_INVALID_FILES` when `--files` contains a null or empty path, and `HOT_RELOAD_STATUS_CONFLICT` when `--status` is combined with `--files` or `--revert-all`. - `NextActions` (array, optional): Ordered recovery steps, present only with `ErrorCode` on a parameter validation failure. Omitted from every other response, including successful apply, plain `--status`, and `--revert-all` runs. - `Methods` (array): Per-method `{ Kind, Method, Reason, FilePath, InvocationCount, LifecycleNote, ReappliedFromSibling }` where `Kind` is `Patched`, `Skipped`, `Failed`, `Added`, `AlreadyActive`, or `Stale` on apply runs, and `Active`, `Added`, or `AddedField` on `--status` runs; empty on `--revert-all` runs. `AlreadyActive` means this file's source matched the last fully applied reload (a run with no Skipped or Failed outcomes), so the existing patch was left in place and the row carries the live `InvocationCount`. `Stale` means the method was deleted from the edited source while its patch is still installed: compiled callers keep running the patched body until `uloop compile`, `--revert-all`, or a later reload whose source restores the method to the compiled baseline clears it; a reload that declares the method with a different body replaces the patch instead of clearing it. Stale rows keep counting toward `ActivePatchTotal`, and the Message summary includes `Stale=N`. `InvocationCount` is meaningful on `Active` rows and on `AlreadyActive` and `Stale` apply rows (calls since the current patch was applied); it is `0` on other apply/revert outcomes. On `--status`, an `Active` row with `InvocationCount` 0 sets `Reason` to explain that the method has not run since the patch: finished calls do not re-run, the patched body takes effect on the next call, and how to retrigger an initialization-only path. When the edited source later declares a different signature, that `Active` row's `Reason` instead explains it is superseded by a new declaration of that signature and is no longer the entry point for new calls (superseded wins over the never-invoked sentence). Added-member rows always show InvocationCount 0 — added-member calls are not instrumented, and the row's Reason says so. `AddedField` rows list a live added field as `Type.field` with an empty `Reason`; they are not method patches. `LifecycleNote` is set when a patched method is a Unity one-shot lifecycle message (`private void Awake`/`Start`/`OnEnable`/`OnDisable`/`OnDestroy` on a `MonoBehaviour`), or when every compiled call path into the patched method (callers of callers are followed a few levels within the compiled assemblies) starts at such a message; empty otherwise — it does not change `Kind`. On an `Added` row whose method is a Unity message, `LifecycleNote` instead says whether the engine will reach it: a forwarded message (`Start`, `Update`, the collision/trigger/mouse messages, and the rest listed in [scope-and-limits.md](scope-and-limits.md)) carries the note that a hot-reload proxy component delivers it to live instances while Play Mode runs, that the proxy is rebuilt only when a later reload changes which messages the type adds or their signatures, that execution order relative to other components is not guaranteed, and that it is gone on any compile or domain reload (only an added `Start` row also says that it runs once on each existing instance when the proxy attaches, and again when the proxy is rebuilt); a message this feature leaves to the compiler (`Awake`, `OnEnable`, `OnDisable`, `OnDestroy`, the editor-only messages, and any non-void message) carries the note that the engine does not invoke it until `uloop compile`, and the run adds one `Warnings` line naming every such message together. `Added` rows carry the added member's signature and file; their `InvocationCount` is always `0` (added-member calls are not instrumented). `ReappliedFromSibling` is `true` on every apply row, whatever its `Kind`, that belongs to a sibling file the run pulled in to re-apply changes from earlier reloads rather than to a file passed in `--files`; it is `false` on the other rows and on every `--status` and `--revert-all` row. Message's re-applied count covers only the `Patched` and `Added` rows among them. `Method` spells parameter types as .NET metadata does, the same on every method row: a constructed generic as ``System.Collections.Generic.List`1``, a multidimensional array as `System.Int32[0...,0...]`, and a nested type with `+`. Example `--status` row: `{ "Kind": "Added", "Method": "Ns.Host.NewHelper(System.Int32)", "Reason": "Added-member calls are not instrumented, so InvocationCount is always 0 for this row.", "FilePath": "Assets/Scripts/Host.cs", "InvocationCount": 0, "LifecycleNote": "", "ReappliedFromSibling": false }` -- `Warnings` (array): Non-fatal notes — one aggregated line listing the patched methods at risk of being already JIT-inlined into existing callers — those marked `[AggressiveInlining]`, plus (only when Code Optimization is Release) those with tiny pre-patch bodies — meaning the change may not show at those call sites, the pause-point interaction (see [pause-point-interaction.md](pause-point-interaction.md)), and the const drift, outside-body drift, and missing-baseline entries described in [scope-and-limits.md](scope-and-limits.md). Skipped outcomes are echoed here as `Skipped : `, or as one `Skipped N methods: ()` line per reason when several share it, so checking `Warnings` alone is enough to see that an edit was not applied. When a reload re-applies unchanged files so their patches bind to this run's shim, Warnings includes `Also re-applied N unchanged file(s) with active patches in assembly '...' so their patches bind to this reload's shim: ...`. When a pulled-in sibling fails in that reload, Warnings includes `'...' was pulled in to re-bind its active patches but this reload failed for it; see its rows for which patches changed and run uloop compile to clear the run.` instead of the re-applied line. When every row of that sibling was `Skipped` and none failed, Warnings instead includes `'...' was pulled in to re-bind its active patches, but every method there was Skipped this time; see its rows for the reasons. Any earlier patches there stay active until uloop compile clears the run.` When the reload stopped before re-applying anything at all, so that sibling has no rows, Warnings instead includes `'...' was pulled in to re-bind its active patches, but this reload stopped before re-applying them, so its active patches are unchanged. Fix the refused declaration and rerun, or run uloop compile to clear the run.` When the whole reload was refused, so that sibling's only rows are `Method` = `(file)` `Failed` rows repeating the refusal and none of its unchanged patches were reverted, Warnings instead includes `'...' was pulled in to re-bind its active patches, but the whole reload was refused before re-applying them, so its active patches are unchanged; its rows repeat the refusal reason. Fix that and rerun, or run uloop compile to clear the run.` When a sibling still has active patches but its source changed since they were applied, Warnings includes `'...' has active patches but its source changed since they were applied, so it was not re-applied; pass it to hot-reload to update it.` When a run carries two or more warnings and all of them are hot reload warnings, the Message ends with "A single 'uloop compile' clears all of them at once when you want them gone; none of them has to be cleared before you keep working." — it is the shortest recovery, not an obligation to compile immediately. Pause-point warnings carry their own recovery steps, so that line does not appear when they are present. Nor does it appear when an `IntroducedTypes` row is `Failed` or a warning says a declared type requires a compile, because that type does not exist until one. It is also left off when any `Methods` row is `Failed`, or when a `Methods` row of a file you passed, or of a sibling retried after an earlier Skip, is `Skipped`: that body is not running yet, so it needs a fix or a compile before you keep working. A `Skipped` row of a sibling pulled in only to re-bind its active patches does not leave it off, because the earlier patches there keep running. +- `Warnings` (array): Non-fatal notes — one aggregated line listing the patched methods at risk of being already JIT-inlined into existing callers — those marked `[AggressiveInlining]`, plus (only when Code Optimization is Release) those with tiny pre-patch bodies — meaning the change may not show at those call sites, the pause-point interaction (see [pause-point-interaction.md](pause-point-interaction.md)), and the const drift, outside-body drift, missing-baseline, and left-out enum file entries described in [scope-and-limits.md](scope-and-limits.md). Skipped outcomes are echoed here as `Skipped : `, or as one `Skipped N methods: ()` line per reason when several share it, so checking `Warnings` alone is enough to see that an edit was not applied. When a reload re-applies unchanged files so their patches bind to this run's shim, Warnings includes `Also re-applied N unchanged file(s) with active patches in assembly '...' so their patches bind to this reload's shim: ...`. When a pulled-in sibling fails in that reload, Warnings includes `'...' was pulled in to re-bind its active patches but this reload failed for it; see its rows for which patches changed and run uloop compile to clear the run.` instead of the re-applied line. When every row of that sibling was `Skipped` and none failed, Warnings instead includes `'...' was pulled in to re-bind its active patches, but every method there was Skipped this time; see its rows for the reasons. Any earlier patches there stay active until uloop compile clears the run.` When the reload stopped before re-applying anything at all, so that sibling has no rows, Warnings instead includes `'...' was pulled in to re-bind its active patches, but this reload stopped before re-applying them, so its active patches are unchanged. Fix the refused declaration and rerun, or run uloop compile to clear the run.` When the whole reload was refused, so that sibling's only rows are `Method` = `(file)` `Failed` rows repeating the refusal and none of its unchanged patches were reverted, Warnings instead includes `'...' was pulled in to re-bind its active patches, but the whole reload was refused before re-applying them, so its active patches are unchanged; its rows repeat the refusal reason. Fix that and rerun, or run uloop compile to clear the run.` When a sibling still has active patches but its source changed since they were applied, Warnings includes `'...' has active patches but its source changed since they were applied, so it was not re-applied; pass it to hot-reload to update it.` When a run carries two or more warnings and all of them are hot reload warnings, the Message ends with "A single 'uloop compile' clears all of them at once when you want them gone; none of them has to be cleared before you keep working." — it is the shortest recovery, not an obligation to compile immediately. Pause-point warnings carry their own recovery steps, so that line does not appear when they are present. Nor does it appear when an `IntroducedTypes` row is `Failed` or a warning says a declared type requires a compile, because that type does not exist until one. It is also left off when any `Methods` row is `Failed`, or when a `Methods` row of a file you passed, or of a sibling retried after an earlier Skip, is `Skipped`: that body is not running yet, so it needs a fix or a compile before you keep working. A `Skipped` row of a sibling pulled in only to re-bind its active patches does not leave it off, because the earlier patches there keep running. - `PatchedTotal` (number): Methods patched in this run - `AddedFields` (array): source-level names ("Type.field") of fields this reload added; their values live outside the compiled type until 'uloop compile'. Every run that adds fields also carries one warning stating that the values live outside the compiled assembly and last only until the next 'uloop compile' or domain reload; the warning names exactly the fields listed in AddedFields. An active added field declared with `[SerializeField]`, `[SerializeReference]`, or `[FormerlySerializedAs]` is also named, as `Namespace.Type.field` (nested types joined with `.`), in one `Added field(s) with a serialization attribute will not appear in the Inspector or serialize until 'uloop compile': ...` warning that points at [added-field-wiring.md](added-field-wiring.md). Only the run that first leaves the field active names it; a file that is Skipped or Failed names none of its fields, and a field is named again only after it stopped being active or after `--revert-all`. Pause-point `CapturedVariables` never includes these fields; `enable-pause-point` warns when the resolved type has any. - `AddedConsts` (array): source-level names ("Type.const") of consts this reload added. They are folded into edited bodies as literals, so they are not listed in AddedFields and do not emit the added-field lifetime warning. diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/scope-and-limits.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/scope-and-limits.md index d9f22e25c..903a9997f 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/scope-and-limits.md +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/scope-and-limits.md @@ -129,6 +129,11 @@ reason says the name is an enum member this reload adds, and the enum-member and changed-`const` warnings of the files passed to that reload stay in `Warnings` even though that failure stops the file. The drift of a changed sibling file that was not passed is not reported on this failure path. +When `--files` is omitted and the enum's file has no edit besides its new enum members, +the reload leaves that file out instead, so the added members of the other files apply; +a `Warnings` line names the left-out file and the enum members that still need +`uloop compile`. A file that already holds patches or declares a new type stays in the +reload. An added property applies unless its shape is listed below. A bodied getter or setter is emitted like an added method; diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/CompiledSignatureSplitCollector.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/CompiledSignatureSplitCollector.cs index 5f1239514..6056c8414 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/CompiledSignatureSplitCollector.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/CompiledSignatureSplitCollector.cs @@ -45,7 +45,8 @@ internal static CompiledSignatureSplit Collect( new List(names.DeclaringTypes), new List(names.ArtifactBoundTypes), new List(names.ArtifactHosts), - new List(names.ArtifactBoundDeclaringFiles)); + new List(names.ArtifactBoundDeclaringFiles), + new List(names.SplitSourceFiles)); } private static bool IsMemberUse(SyntaxNode node) @@ -167,7 +168,11 @@ private static void AddSplit( { names.ArtifactBoundTypes.Add(CecilTypeNames.ToMetadataName(signatureType.OriginalDefinition)); names.ArtifactHosts.Add(CecilTypeNames.ToMetadataName(declaringType.OriginalDefinition)); - AddSourceCopyFiles(signatureType, sourceAssembly, names); + AddSourceCopyFiles( + signatureType, + sourceAssembly, + names.ProjectRelativePathsByBindingTree, + names.ArtifactBoundDeclaringFiles); continue; } @@ -179,6 +184,11 @@ private static void AddSplit( } names.SplitTypes.Add(CecilTypeNames.ToMetadataName(signatureType.OriginalDefinition)); + AddSourceCopyFiles( + signatureType, + sourceAssembly, + names.ProjectRelativePathsByBindingTree, + names.SplitSourceFiles); found = true; } @@ -194,7 +204,8 @@ private static void AddSplit( private static void AddSourceCopyFiles( INamedTypeSymbol signatureType, IAssemblySymbol sourceAssembly, - CompiledSignatureSplitNames names) + IReadOnlyDictionary projectRelativePathsByBindingTree, + SortedSet sourceCopyFiles) { string reflectionName = CecilTypeNames.ToMetadataName(signatureType.OriginalDefinition).Replace('/', '+'); INamedTypeSymbol sourceCopy = sourceAssembly.GetTypeByMetadataName(reflectionName); @@ -202,7 +213,7 @@ private static void AddSourceCopyFiles( { // The compilation holds only the run's binding trees, so a declaration outside them // means the map was built from other trees than the ones this model binds. - if (!names.ProjectRelativePathsByBindingTree.TryGetValue( + if (!projectRelativePathsByBindingTree.TryGetValue( declaration.SyntaxTree, out string projectRelativePath)) { @@ -210,7 +221,7 @@ private static void AddSourceCopyFiles( "The source copy of '" + reflectionName + "' is declared outside the run's binding trees."); } - names.ArtifactBoundDeclaringFiles.Add(projectRelativePath); + sourceCopyFiles.Add(projectRelativePath); } } @@ -318,6 +329,8 @@ internal CompiledSignatureSplitNames(IReadOnlyDictionary pro internal SortedSet ArtifactHosts { get; } = new SortedSet(StringComparer.Ordinal); internal SortedSet ArtifactBoundDeclaringFiles { get; } = new SortedSet(StringComparer.Ordinal); + + internal SortedSet SplitSourceFiles { get; } = new SortedSet(StringComparer.Ordinal); } /// @@ -332,13 +345,15 @@ internal CompiledSignatureSplit( List declaringTypeMetadataNames, List artifactBoundTypeMetadataNames, List artifactHostMetadataNames, - List artifactBoundDeclaringFiles) + List artifactBoundDeclaringFiles, + List splitSourceFiles) { SplitTypeMetadataNames = splitTypeMetadataNames; DeclaringTypeMetadataNames = declaringTypeMetadataNames; ArtifactBoundTypeMetadataNames = artifactBoundTypeMetadataNames; ArtifactHostMetadataNames = artifactHostMetadataNames; ArtifactBoundDeclaringFiles = artifactBoundDeclaringFiles; + SplitSourceFiles = splitSourceFiles; } internal List SplitTypeMetadataNames { get; } @@ -355,4 +370,8 @@ internal CompiledSignatureSplit( // Project-relative paths of the run's files that declare the source copies of those compiled // types. internal List ArtifactBoundDeclaringFiles { get; } + + // Project-relative paths of the run's files that declare the source copies of the split + // types, which the compiled signatures still name by their compiled copies. + internal List SplitSourceFiles { get; } } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/MethodTransformDecider.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/MethodTransformDecider.cs index 4f7f379ef..148c336dc 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/MethodTransformDecider.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/MethodTransformDecider.cs @@ -272,12 +272,17 @@ private static WorkerReason DescribeUnboundBody( return WorkerReason.Of(HotReloadWorkerReasonCode.AddedMethodBodyUnbound, diagnosticText); } - return WorkerReason.NamingCompiledTypes( + WorkerReason compiledSignatureReason = WorkerReason.NamingCompiledTypes( HotReloadWorkerReasonCode.AddedMethodBodyBindsCompiledSignature, split.DeclaringTypeMetadataNames.ToArray(), diagnosticText, QuoteNames(split.SplitTypeMetadataNames), QuoteNames(split.DeclaringTypeMetadataNames)); + // Why apart from the declaring files: the Editor places those from the compiled API's + // debug data, while only this compilation knows which of the run's files built the split + // type from source, which is the file a run can leave out instead. + compiledSignatureReason.SplitSourceFiles = split.SplitSourceFiles.ToArray(); + return compiledSignatureReason; } private static string QuoteNames(List names) diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/WorkerReason.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/WorkerReason.cs index 31807c7cf..268dbf3b4 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/WorkerReason.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/WorkerReason.cs @@ -20,6 +20,10 @@ internal sealed class WorkerReason // types only this process can resolve to files. Null when the Editor resolves them instead. public string[] DeclaringFiles { get; set; } + // Project-relative paths of the run's files that build the split types from source, for a + // reason whose compiled signatures still name the compiled copies. Null for other reasons. + public string[] SplitSourceFiles { get; set; } + internal static WorkerReason Of(HotReloadWorkerReasonCode code, params string[] args) { return new WorkerReason