diff --git a/.agents/skills/uloop-hot-reload/references/output.md b/.agents/skills/uloop-hot-reload/references/output.md index 3a640d368..fed7ea4a2 100644 --- a/.agents/skills/uloop-hot-reload/references/output.md +++ b/.agents/skills/uloop-hot-reload/references/output.md @@ -5,7 +5,7 @@ Returns JSON with: - `Success` (boolean): `false` on parameter validation failure or when any method outcome is `Failed`, or when any `IntroducedTypes` row is `Failed`. `Skipped` outcomes alone never force `false` - `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. 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 }` +- `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. - `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. diff --git a/.claude/skills/uloop-hot-reload/references/output.md b/.claude/skills/uloop-hot-reload/references/output.md index 3a640d368..fed7ea4a2 100644 --- a/.claude/skills/uloop-hot-reload/references/output.md +++ b/.claude/skills/uloop-hot-reload/references/output.md @@ -5,7 +5,7 @@ Returns JSON with: - `Success` (boolean): `false` on parameter validation failure or when any method outcome is `Failed`, or when any `IntroducedTypes` row is `Failed`. `Skipped` outcomes alone never force `false` - `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. 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 }` +- `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. - `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. diff --git a/Assets/Tests/Editor/HotReload/HotReloadDomainTests.cs b/Assets/Tests/Editor/HotReload/HotReloadDomainTests.cs index 5cdc9ce36..86ad594ed 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadDomainTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadDomainTests.cs @@ -970,11 +970,12 @@ public void FindUnappliedRowForMethod_NullMethod_ReturnsNull() } /// - /// What: a worker row spells a constructed generic parameter type the way Cecil does, so a - /// method taking one finds no row; the match is exact and does not convert the spelling. + /// What: a worker row spells a constructed generic parameter type the way metadata does, and + /// the label of the resolved method spells it the same way, so a method taking one finds + /// its own row. /// [Test] - public void FindUnappliedRowForMethod_MethodWithAConstructedGenericParameter_ReturnsNull() + public void FindUnappliedRowForMethod_MethodWithAConstructedGenericParameter_FindsItsWorkerRow() { string workerLabel = HotReloadMethodKeys.FormatMethodLabelParts( new HotReloadMetadataTypeName(typeof(RowLabelHost).FullName.Replace('+', '/')), @@ -992,7 +993,8 @@ public void FindUnappliedRowForMethod_MethodWithAConstructedGenericParameter_Ret HotReloadUnappliedRow row = port.FindUnappliedRowForMethod(FileOne, RowLabelHostMethod(nameof(RowLabelHost.TakeList))); - Assert.That(row, Is.Null); + Assert.That(row, Is.Not.Null); + Assert.That(row.Label, Is.EqualTo(workerLabel)); } // Why a generated name: an artifact assembly is compiled under a name of its own, so a diff --git a/Assets/Tests/Editor/HotReload/HotReloadLabelShapeWriterHost.cs b/Assets/Tests/Editor/HotReload/HotReloadLabelShapeWriterHost.cs new file mode 100644 index 000000000..31e5a5129 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadLabelShapeWriterHost.cs @@ -0,0 +1,31 @@ +using System.Collections.Generic; +using System.Runtime.CompilerServices; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// Compiled host whose writers take a constructed generic and a multidimensional array, the + /// parameter shapes reflection and metadata spell differently, and a reader the hot reload + /// tests edit to read what a writer assigns. + /// + public class HotReloadLabelShapeWriterHost + { + [MethodImpl(MethodImplOptions.NoInlining)] + public int WriteFromList(List values) + { + return values.Count; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public int WriteFromGrid(int[,] values) + { + return values.Length; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public int Read(int value) + { + return value; + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadLabelShapeWriterHost.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadLabelShapeWriterHost.cs.meta new file mode 100644 index 000000000..b2eb86f92 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadLabelShapeWriterHost.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: d2dcc96ca79844ceaab57c94ebf80464 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadMethodLabelParityTests.cs b/Assets/Tests/Editor/HotReload/HotReloadMethodLabelParityTests.cs new file mode 100644 index 000000000..d45043abc --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadMethodLabelParityTests.cs @@ -0,0 +1,229 @@ +using System; +using System.Collections.Generic; +using System.Reflection; + +using Mono.Cecil; +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// EditMode coverage for the method label every hot reload row and ledger shares: the label + /// built from a resolved method must equal the label built from the same method's metadata + /// spelling, which is what worker rows and call-site hits carry. + /// + public class HotReloadMethodLabelParityTests + { + private const BindingFlags FixtureMethodFlags = + BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly; + + private AssemblyDefinition _fixtureAssembly; + + [OneTimeSetUp] + public void OneTimeSetUp() + { + _fixtureAssembly = AssemblyDefinition.ReadAssembly( + typeof(HotReloadLabelShapeHost).Assembly.Location, + new ReaderParameters { InMemory = true }); + } + + [OneTimeTearDown] + public void OneTimeTearDown() + { + _fixtureAssembly?.Dispose(); + _fixtureAssembly = null; + } + + private static IEnumerable ParameterShapeMethods() + { + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeInt)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeNested)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeList)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeDictionaryOfLists)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeGrid)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeCube)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeGridOfVectors)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeVector)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeJagged)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeVectorOfLists)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeListOfGrids)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeRefList)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeOutGrid)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeNullable)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeTuple)); + yield return Shape( + typeof(HotReloadLabelShapeHost), + nameof(HotReloadLabelShapeHost.TakeNestedOfConstructedOuter)); + yield return Shape( + typeof(HotReloadLabelShapeHost), + nameof(HotReloadLabelShapeHost.TakeNestedGenericOfConstructedOuter)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeListOfNested)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeGeneric)); + yield return Shape( + typeof(HotReloadLabelShapeGenericHost<>), + nameof(HotReloadLabelShapeGenericHost.TakeTypeParameterList)); + yield return Shape( + typeof(HotReloadLabelShapeGenericHost<>), + nameof(HotReloadLabelShapeGenericHost.TakeSelf)); + yield return Shape( + typeof(HotReloadLabelShapeGenericHost<>), + nameof(HotReloadLabelShapeGenericHost.TakeOwnInner)); + yield return Shape( + typeof(HotReloadLabelShapeGenericHost<>), + nameof(HotReloadLabelShapeGenericHost.TakeOwnInnerOf)); + } + + /// + /// Verifies that for each parameter shape a worker row or a call-site hit can carry, the + /// label built from the resolved method reads the same as the label built from Cecil's + /// spelling of that method, so a label made in either world matches the other. + /// + [TestCaseSource(nameof(ParameterShapeMethods))] + public void FormatMethodLabel_ParameterShape_MatchesTheLabelBuiltFromTheMetadataSpelling( + Type hostType, + string methodName) + { + MethodInfo method = hostType.GetMethod(methodName, FixtureMethodFlags); + Assert.That(method, Is.Not.Null, "Precondition: the fixture method must exist."); + string fromMetadata = FormatLabelFromMetadataSpelling(method); + + string fromMethod = HotReloadMethodKeys.FormatMethodLabel(method); + + Assert.That(fromMethod, Is.EqualTo(fromMetadata)); + } + + private static TestCaseData Shape(Type hostType, string methodName) + { + return new TestCaseData(hostType, methodName); + } + + private string FormatLabelFromMetadataSpelling(MethodInfo method) + { + MethodDefinition definition = (MethodDefinition)_fixtureAssembly.MainModule.LookupToken(method.MetadataToken); + string[] parameterTypeFullNames = new string[definition.Parameters.Count]; + for (int index = 0; index < definition.Parameters.Count; index++) + { + parameterTypeFullNames[index] = definition.Parameters[index].ParameterType.FullName; + } + + return HotReloadMethodKeys.FormatMethodLabelParts( + new HotReloadMetadataTypeName(definition.DeclaringType.FullName), + definition.Name, + parameterTypeFullNames, + definition.GenericParameters.Count); + } + } + + public sealed class HotReloadLabelShapeHost + { + public sealed class Inner + { + } + + public void TakeInt(int value) + { + } + + public void TakeNested(Inner value) + { + } + + public void TakeList(List values) + { + } + + public void TakeDictionaryOfLists(Dictionary> values) + { + } + + public void TakeGrid(int[,] values) + { + } + + public void TakeCube(int[,,] values) + { + } + + public void TakeGridOfVectors(int[,][] values) + { + } + + public void TakeVector(int[] values) + { + } + + public void TakeJagged(int[][] values) + { + } + + public void TakeVectorOfLists(List[] values) + { + } + + public void TakeListOfGrids(List values) + { + } + + public void TakeRefList(ref List values) + { + } + + public void TakeOutGrid(out int[,] values) + { + values = null; + } + + public void TakeNullable(int? value) + { + } + + public void TakeTuple((int, string) value) + { + } + + public void TakeNestedOfConstructedOuter(HotReloadLabelShapeGenericHost.Inner value) + { + } + + public void TakeNestedGenericOfConstructedOuter(HotReloadLabelShapeGenericHost.InnerOf value) + { + } + + public void TakeListOfNested(List values) + { + } + + public void TakeGeneric(TItem value, List values) + { + } + } + + public sealed class HotReloadLabelShapeGenericHost + { + public sealed class Inner + { + } + + public sealed class InnerOf + { + } + + public void TakeTypeParameterList(List values) + { + } + + public void TakeSelf(HotReloadLabelShapeGenericHost other) + { + } + + public void TakeOwnInner(Inner inner) + { + } + + public void TakeOwnInnerOf(InnerOf inner) + { + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadMethodLabelParityTests.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadMethodLabelParityTests.cs.meta new file mode 100644 index 000000000..bc407b203 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadMethodLabelParityTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a27af35bbcd074d13ba89d7a6d4d8ce6 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs b/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs index d866d9e51..2a25894ce 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs @@ -5555,6 +5555,116 @@ public async Task Run_SkippedWriterWithEarlierPatch_WarningNamesTheEarlierPatch( allWarnings); } + /// + /// What: when the skipped writer with an earlier patch takes a constructed generic + /// parameter, the skipped-writer warning still names the earlier patch: the Editor's + /// active-patch label spells the parameter the way the worker's skipped row does. + /// + [Test] + public async Task Run_SkippedWriterTakingAConstructedGeneric_WarningNamesTheEarlierPatch() + { + await AssertSkippedWriterWarningNamesTheEarlierPatch( + nameof(HotReloadLabelShapeWriterHost.WriteFromList), + "List values", + "values.Count", + host => host.WriteFromList(new List { 1, 2, 3 }), + 3); + } + + /// + /// What: when the skipped writer with an earlier patch takes a multidimensional array, + /// the skipped-writer warning still names the earlier patch: the Editor's active-patch + /// label spells the array rank the way the worker's skipped row does. + /// + [Test] + public async Task Run_SkippedWriterTakingAMultidimensionalArray_WarningNamesTheEarlierPatch() + { + await AssertSkippedWriterWarningNamesTheEarlierPatch( + nameof(HotReloadLabelShapeWriterHost.WriteFromGrid), + "int[,] values", + "values.Length", + host => host.WriteFromGrid(new int[2, 3]), + 6); + } + + // Runs the two reloads of the skipped-writer-with-earlier-patch case against one writer + // of the label-shape host: the first applies the writer assigning an added auto-property + // that Read reads, the second makes the worker skip that writer. + private static async Task AssertSkippedWriterWarningNamesTheEarlierPatch( + string writerName, + string writerParameterList, + string writerResult, + Func invokeWriter, + int expectedAssigned) + { + string fixturePath = ResolveLabelShapeWriterHostPath(); + string onDisk = File.ReadAllText(fixturePath); + string writerHeader = " public int " + writerName + "(" + writerParameterList + ")\n {\n"; + string writerOriginal = writerHeader + " return " + writerResult + ";\n }"; + const string readOriginal = + " public int Read(int value)\n {\n return value;\n }"; + const string readingRead = + " public int Read(int value)\n {\n return AddedCount + value;\n }\n\n" + + " public int AddedCount { get; private set; }"; + Assert.That(onDisk, Does.Contain(writerOriginal), "Precondition: the writer must be on disk as written."); + Assert.That(onDisk, Does.Contain(readOriginal), "Precondition: Read must be on disk as written."); + string firstEdit = onDisk + .Replace( + writerOriginal, + writerHeader + " AddedCount = " + writerResult + ";\n" + + " return " + writerResult + ";\n }", + StringComparison.Ordinal) + .Replace(readOriginal, readingRead, StringComparison.Ordinal); + + HotReloadOrchestratorResult first = await HotReloadCompositionRoot.Services.Orchestrator.RunAsync( + new[] { fixturePath }, + WriteEditedSource("LabelShape" + writerName + "1.cs", firstEdit), + CancellationToken.None); + AssertNoFileLevelFailure(first); + AssertHasPatched(first, writerName); + + // Why base.GetHashCode(): a base call is a worker-side skip, so the worker itself + // sees this writer as skipped while the first run's patch stays what runs. + string secondEdit = onDisk + .Replace( + writerOriginal, + writerHeader + " AddedCount = " + writerResult + ";\n" + + " return " + writerResult + " + base.GetHashCode() * 0;\n }", + StringComparison.Ordinal) + .Replace(readOriginal, readingRead, StringComparison.Ordinal); + + HotReloadOrchestratorResult second = await HotReloadCompositionRoot.Services.Orchestrator.RunAsync( + new[] { fixturePath }, + WriteEditedSource("LabelShape" + writerName + "2.cs", secondEdit), + CancellationToken.None); + AssertNoFileLevelFailure(second); + Assert.That(FindSkippedReason(second, writerName), Is.Not.Null, FormatOutcomes(second)); + + // The first run's writer patch still assigns the property the applied Read reads. + HotReloadLabelShapeWriterHost host = new HotReloadLabelShapeWriterHost(); + invokeWriter(host); + Assert.That(host.Read(0), Is.EqualTo(expectedAssigned)); + + string writerLabel = HotReloadMethodKeys.FormatMethodLabel( + typeof(HotReloadLabelShapeWriterHost).GetMethod(writerName)); + List writerWarnings = new List(); + foreach (string warning in second.Warnings) + { + if (warning.Contains("'AddedCount'", StringComparison.Ordinal)) + { + writerWarnings.Add(warning); + } + } + + string allWarnings = "Warnings were:\n" + string.Join("\n", second.Warnings); + Assert.That(writerWarnings, Has.Count.EqualTo(1), allWarnings); + Assert.That(writerWarnings[0], Does.Contain("an earlier hot reload applied " + writerLabel), allWarnings); + Assert.That( + writerWarnings[0], + Does.Not.Contain("reads it; the property keeps its default value"), + allWarnings); + } + /// /// What: when the only writer of an added field is an added method that an earlier /// reload added and this reload skips, the skipped-writer warning names that earlier @@ -7762,6 +7872,21 @@ private static string ResolveSignatureChangeExternalHostPath() return Path.GetFullPath(path); } + private static string ResolveLabelShapeWriterHostPath() + { + string path = Path.Combine( + Application.dataPath, + "Tests", + "Editor", + "HotReload", + "HotReloadLabelShapeWriterHost.cs"); + Assert.That( + File.Exists(path), + Is.True, + "Label-shape writer host source missing: " + path); + return Path.GetFullPath(path); + } + private static string ResolveSignatureChangeUnchangedCallerFixturePath() { string path = Path.Combine( diff --git a/Assets/Tests/Editor/HotReload/HotReloadPatcherTests.cs b/Assets/Tests/Editor/HotReload/HotReloadPatcherTests.cs index 73d792a6e..459588393 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadPatcherTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadPatcherTests.cs @@ -451,19 +451,20 @@ public void FormatMethodKey_DistinguishesOverloadsByParameterTypes() } /// - /// What: constructed generic parameters use Type.ToString (List`1[System.Int32]), not - /// assembly-qualified FullName (FullName would embed Version/PublicKeyToken). + /// What: constructed generic parameters are spelled the way metadata spells them, with the + /// type arguments in angle brackets, never with Type.ToString's square brackets or the + /// assembly-qualified FullName (which would embed Version/PublicKeyToken). /// [Test] - public void FormatMethodKey_ConstructedGenericParameter_OmitsAssemblyQualification() + public void FormatMethodKey_ConstructedGenericParameter_UsesMetadataSpellingWithoutAssemblyQualification() { MethodInfo take = AccessTools.Method( typeof(HotReloadGenericKeyFixture), nameof(HotReloadGenericKeyFixture.Take)); string key = HotReloadMethodKeys.FormatMethodLabel(take); - Assert.That(key, Does.Contain("System.Collections.Generic.List`1[System.Int32]")); - Assert.That(key, Does.Contain("System.Collections.Generic.Dictionary`2[System.String,System.Int32]")); + Assert.That(key, Does.Contain("System.Collections.Generic.List`1")); + Assert.That(key, Does.Contain("System.Collections.Generic.Dictionary`2")); Assert.That(key, Does.Not.Contain("Version=")); Assert.That(key, Does.Not.Contain("PublicKeyToken=")); Assert.That(key, Does.Not.Contain("mscorlib")); diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeCoverage.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeCoverage.cs index a9712e52b..859de3423 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeCoverage.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeCoverage.cs @@ -93,8 +93,7 @@ internal static List CollectStaleSignatureWarnings( // removals, so restrict to replacements that remain in the apply set. Known // limits: an already-patched caller that is also edited this run is // over-reported (the text is still true); a caller that stayed Skipped and - // drifted is missed; constructed generics can miss when the label space differs - // from the wire key (same constraint as IsActiveMember). + // drifted is missed. internal static void AppendSignatureChangeCallersRepatchedWarnings( List warnings, string assemblyName, diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadPausePointPort.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadPausePointPort.cs index bb02062d3..75c000929 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadPausePointPort.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadPausePointPort.cs @@ -118,9 +118,9 @@ public HotReloadLatestFileReload GetLatestReloadOfFile(string file) return new HotReloadLatestFileReload(fileChangedSince, record.UnappliedRows); } - // Why the label match is exact: a worker row spells a constructed generic parameter type - // the way Cecil does (List`1) while a MethodBase spells it the way the CLR - // does (List`1[System.Int32]); such a method finds no row rather than a guessed one. + // Why the label match is exact: the label built from the MethodBase spells its parameter + // types the way the worker row does (List`1), so a method whose row is + // absent finds none rather than a guessed one. public HotReloadUnappliedRow FindUnappliedRowForMethod(string file, MethodBase method) { if (method == null || method.DeclaringType == null) diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadMethodKeys.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadMethodKeys.cs index 09869aac9..c93230642 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadMethodKeys.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadMethodKeys.cs @@ -12,6 +12,8 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// Single Unity-side formatter for the two method identifiers hot reload uses: the wire key /// (Type::Method`N(params), exchanged with the worker and the call-site scanner) and the /// display label (Type.Method`N(params), shown in Methods[].Method and --status rows). + /// Both spell parameter types the way metadata does (List`1<System.Int32>), whether they + /// are built from worker fields or from a resolved MethodBase. /// Mirrored on the worker side by WorkerMethodKeys — keep the two files in sync. /// internal static class HotReloadMethodKeys @@ -46,10 +48,13 @@ internal static string BuildMethodKeyParts( // What: status / counter label from a resolved MethodBase (apply outcomes use the same // helper after Resolve so --status rows match Patched Methods[].Method). - // Why parameter ToString (+ generic arity): MethodBase ledger entries distinguish - // overloads, so a name-only key would merge counts and let Revert of one overload - // zero the other's counter. FullName embeds assembly Version/PublicKeyToken for - // constructed generics (List`1[[Int32, mscorlib, ...]]), which bloated labels. + // Parameter types (+ generic arity) are part of the label because MethodBase ledger + // entries distinguish overloads, so a name-only key would merge counts and let Revert + // of one overload zero the other's counter. They are spelled the way metadata spells + // them because worker rows and call-site hits carry that spelling and the ledgers match + // labels by ordinal equality: Type.ToString writes List`1[System.Int32] and + // System.Int32[,] where Cecil writes List`1 and System.Int32[0...,0...], + // and FullName embeds assembly Version/PublicKeyToken for constructed generics. internal static string FormatMethodLabel(MethodBase method) { Debug.Assert(method != null, "method must not be null."); @@ -60,7 +65,7 @@ internal static string FormatMethodLabel(MethodBase method) string[] parameterTypeFullNames = new string[parameters.Length]; for (int index = 0; index < parameters.Length; index++) { - parameterTypeFullNames[index] = parameters[index].ParameterType.ToString(); + parameterTypeFullNames[index] = FormatParameterTypeName(parameters[index].ParameterType); } int genericArity = 0; @@ -76,6 +81,77 @@ internal static string FormatMethodLabel(MethodBase method) genericArity); } + // Spells a parameter type the way Cecil's TypeReference.FullName spells it, except that a + // nested type keeps reflection's '+', which is what every label turns Cecil's '/' into. + // A structural walk rather than a rewrite of Type.ToString, because a '[' in its output + // opens either type arguments or an array rank and only the Type knows which. + private static string FormatParameterTypeName(Type type) + { + if (type.IsByRef) + { + return FormatParameterTypeName(type.GetElementType()) + "&"; + } + + if (type.IsPointer) + { + return FormatParameterTypeName(type.GetElementType()) + "*"; + } + + if (type.IsArray) + { + return FormatParameterTypeName(type.GetElementType()) + FormatArrayRankSuffix(type.GetArrayRank()); + } + + if (type.IsGenericParameter) + { + return type.Name; + } + + if (type.IsGenericType) + { + return FormatGenericTypeName(type); + } + + return type.FullName; + } + + // Rank alone decides, as it does in the worker's CecilTypeNames: C# declares a rank-1 + // array only as a vector, and metadata gives each dimension of a C# multidimensional + // array a zero lower bound, which Cecil spells "0...". + private static string FormatArrayRankSuffix(int rank) + { + if (rank == 1) + { + return "[]"; + } + + string[] dimensions = new string[rank]; + for (int index = 0; index < rank; index++) + { + dimensions[index] = "0..."; + } + + return "[" + string.Join(",", dimensions) + "]"; + } + + // Every generic type takes this path, definitions included: FullName is null for a + // constructed type that still holds a type parameter (List), and a definition, which + // is what a generic type's method gets for its own type or a type nested in it, has a + // FullName without the type parameters metadata still spells (Box`1). + // GetGenericArguments lists a nested type's arguments outer type first, which is the + // order metadata writes them in. + private static string FormatGenericTypeName(Type genericType) + { + Type[] arguments = genericType.GetGenericArguments(); + string[] argumentNames = new string[arguments.Length]; + for (int index = 0; index < arguments.Length; index++) + { + argumentNames[index] = FormatParameterTypeName(arguments[index]); + } + + return genericType.GetGenericTypeDefinition().FullName + "<" + string.Join(",", argumentNames) + ">"; + } + // What: display label from worker DTO fields (pre-Resolve failures) using the same shape // as FormatMethodLabel. Cecil nested separators ('/') are normalized to reflection ('+'). // Keep in sync with WorkerMethodKeys.FormatMethodLabelParts. @@ -94,7 +170,7 @@ internal static string FormatMethodLabelParts( // What: the shared assembly of a label once every name it holds is in reflection form. // Why a parameter type is converted here too: a worker row spells a nested parameter type - // in metadata form, while a resolved MethodBase already spells it as reflection does, so + // in metadata form, while FormatParameterTypeName already spells it as reflection does, so // the conversion is a no-op on the reflection path and both paths produce one string. private static string FormatMethodLabelFromReflection( HotReloadReflectionTypeName typeReflectionName, diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md index 3a640d368..fed7ea4a2 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md @@ -5,7 +5,7 @@ Returns JSON with: - `Success` (boolean): `false` on parameter validation failure or when any method outcome is `Failed`, or when any `IntroducedTypes` row is `Failed`. `Skipped` outcomes alone never force `false` - `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. 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 }` +- `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. - `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.