diff --git a/.agents/skills/uloop-hot-reload/references/output.md b/.agents/skills/uloop-hot-reload/references/output.md index 6185e28a8..b91e2d5b8 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` and `Added` rows of `--status` and on `AlreadyActive` and `Stale` apply rows (calls into the patched or added body since it was applied); it is `0` on other apply/revert outcomes, including the `Added` rows of the run that applied them. 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). On `--status`, an `Added` row counts calls into the added member's body since it was applied; while that count is 0, its `Reason` explains that compiled code cannot call an added member, so only a hot-reloaded body that calls it, or the hot-reload proxy delivering a forwarded Unity message in Play Mode, can run it. An added iterator counts when its enumeration starts, whereas a patched iterator, and an async method of either kind, counts when it is called. An `AlreadyActive` row for an added member carries that member's count. `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. `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": "", "FilePath": "Assets/Scripts/Host.cs", "InvocationCount": 3, "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, 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. +- `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 patch or added member that an earlier reload applied still calls an added member that is no longer registered — a later reload changed its signature, deleted it, or skipped it while the caller did not apply again — Warnings includes `Methods that earlier hot reloads patched or added still call added members that are no longer registered: calls , .... Those calls still run the members' earlier bodies, which match neither the compiled assembly nor the source on disk. Reload until the calling methods apply again, or run 'uloop compile'.` on every reload that includes the caller's file or the member's file; see [troubleshooting.md](troubleshooting.md). 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 903a9997f..be2d4032e 100644 --- a/.agents/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.agents/skills/uloop-hot-reload/references/scope-and-limits.md @@ -213,6 +213,9 @@ method: a copy the JIT inlined into another method before the patch, and a deleg the old method the caller created before it. A call inside a lambda or local function stays listed under its compiler-generated name even when the method declaring it is patched. +An added member that a later reload re-signatures or deletes has no compiled callers, but a +hot-reloaded caller that does not apply again in that reload keeps calling the member's +earlier body; `Warnings` then names that call (see [troubleshooting.md](troubleshooting.md)). Field declarations are stricter: when a compiled field's type — or its `static`/ `const` modifier — differs from the edited source, every edited method that reads diff --git a/.agents/skills/uloop-hot-reload/references/troubleshooting.md b/.agents/skills/uloop-hot-reload/references/troubleshooting.md index fb919684c..7909cf1e8 100644 --- a/.agents/skills/uloop-hot-reload/references/troubleshooting.md +++ b/.agents/skills/uloop-hot-reload/references/troubleshooting.md @@ -75,3 +75,18 @@ the hits with `uloop pause-point-status`: the failing statement sits at or after marked line that records a hit, and before the first one that records none. A line inside an added method cannot hold a pause point until `uloop compile`, so mark the line that calls it instead. + +## When Earlier Patches Still Call an Added Member That Is Gone + +A patched or added body keeps calling the added members it was applied against. When a later +reload changes such a member's signature, deletes it, or reports it `Skipped`, the member is +no longer registered and `--status` stops listing it, but a caller that did not apply again +in that reload — its row is `Failed` or `Skipped`, or its file was not re-applied — still +runs the member's earlier body, which matches neither the compiled assembly nor the source +on disk. `Warnings` then carries one line naming each such call as ` calls `, +with `` in the signature the caller was applied against. + +The line comes back on every reload that includes the caller's file or the member's file, +and stops once the caller applies again: fix what the caller's row reported and reload its +file together with the member's file, or run `uloop compile`. A reload that includes neither +file does not repeat it. diff --git a/.claude/skills/uloop-hot-reload/references/output.md b/.claude/skills/uloop-hot-reload/references/output.md index 6185e28a8..b91e2d5b8 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` and `Added` rows of `--status` and on `AlreadyActive` and `Stale` apply rows (calls into the patched or added body since it was applied); it is `0` on other apply/revert outcomes, including the `Added` rows of the run that applied them. 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). On `--status`, an `Added` row counts calls into the added member's body since it was applied; while that count is 0, its `Reason` explains that compiled code cannot call an added member, so only a hot-reloaded body that calls it, or the hot-reload proxy delivering a forwarded Unity message in Play Mode, can run it. An added iterator counts when its enumeration starts, whereas a patched iterator, and an async method of either kind, counts when it is called. An `AlreadyActive` row for an added member carries that member's count. `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. `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": "", "FilePath": "Assets/Scripts/Host.cs", "InvocationCount": 3, "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, 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. +- `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 patch or added member that an earlier reload applied still calls an added member that is no longer registered — a later reload changed its signature, deleted it, or skipped it while the caller did not apply again — Warnings includes `Methods that earlier hot reloads patched or added still call added members that are no longer registered: calls , .... Those calls still run the members' earlier bodies, which match neither the compiled assembly nor the source on disk. Reload until the calling methods apply again, or run 'uloop compile'.` on every reload that includes the caller's file or the member's file; see [troubleshooting.md](troubleshooting.md). 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 903a9997f..be2d4032e 100644 --- a/.claude/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.claude/skills/uloop-hot-reload/references/scope-and-limits.md @@ -213,6 +213,9 @@ method: a copy the JIT inlined into another method before the patch, and a deleg the old method the caller created before it. A call inside a lambda or local function stays listed under its compiler-generated name even when the method declaring it is patched. +An added member that a later reload re-signatures or deletes has no compiled callers, but a +hot-reloaded caller that does not apply again in that reload keeps calling the member's +earlier body; `Warnings` then names that call (see [troubleshooting.md](troubleshooting.md)). Field declarations are stricter: when a compiled field's type — or its `static`/ `const` modifier — differs from the edited source, every edited method that reads diff --git a/.claude/skills/uloop-hot-reload/references/troubleshooting.md b/.claude/skills/uloop-hot-reload/references/troubleshooting.md index fb919684c..7909cf1e8 100644 --- a/.claude/skills/uloop-hot-reload/references/troubleshooting.md +++ b/.claude/skills/uloop-hot-reload/references/troubleshooting.md @@ -75,3 +75,18 @@ the hits with `uloop pause-point-status`: the failing statement sits at or after marked line that records a hit, and before the first one that records none. A line inside an added method cannot hold a pause point until `uloop compile`, so mark the line that calls it instead. + +## When Earlier Patches Still Call an Added Member That Is Gone + +A patched or added body keeps calling the added members it was applied against. When a later +reload changes such a member's signature, deletes it, or reports it `Skipped`, the member is +no longer registered and `--status` stops listing it, but a caller that did not apply again +in that reload — its row is `Failed` or `Skipped`, or its file was not re-applied — still +runs the member's earlier body, which matches neither the compiled assembly nor the source +on disk. `Warnings` then carries one line naming each such call as ` calls `, +with `` in the signature the caller was applied against. + +The line comes back on every reload that includes the caller's file or the member's file, +and stops once the caller applies again: fix what the caller's row reported and reload its +file together with the member's file, or run `uloop compile`. A reload that includes neither +file does not repeat it. diff --git a/Assets/Tests/Editor/HotReload/HotReloadAddedCalleeIndexTests.cs b/Assets/Tests/Editor/HotReload/HotReloadAddedCalleeIndexTests.cs new file mode 100644 index 000000000..89bce995f --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadAddedCalleeIndexTests.cs @@ -0,0 +1,121 @@ +using System.Collections.Generic; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// Pure coverage for how a group's worker entries name the added members they call: the wire + /// keys in calledAddedMethodKeys are turned into the labels the added-member ledger uses, with + /// the file that declared each member. + /// + public class HotReloadAddedCalleeIndexTests + { + private const string HostPath = "Assets/Host.cs"; + private const string CallerPath = "Assets/Caller.cs"; + + /// + /// What: a call is recorded with the label the ledger registers the added member under, + /// nested types in their reflection spelling, and with the file that declared the member. + /// The keys are written as the worker emits them, so a key format that drifts from the + /// worker's fails here. + /// + [TestCase("Ns.Outer/Inner", "Bar", new[] { "System.Int32" }, "Ns.Outer/Inner::Bar(System.Int32)", "Ns.Outer+Inner.Bar(System.Int32)")] + [TestCase("Ns.Host", "get_Speed", new string[0], "Ns.Host::get_Speed()", "Ns.Host.get_Speed()")] + public void Resolve_CallToAnAddedMember_RecordsItsLedgerLabelAndDeclaringFile( + string typeMetadataName, + string methodName, + string[] parameterTypeFullNames, + string wireKey, + string expectedLabel) + { + TransformWorkerEntryDto caller = BuildCallerEntry(wireKey); + HotReloadAddedCalleeIndex index = new HotReloadAddedCalleeIndex( + new[] + { + BuildAddedEntry(typeMetadataName, methodName, parameterTypeFullNames), + caller + }); + + (IReadOnlyList callees, string errorMessage) = index.Resolve(caller); + + Assert.That(errorMessage, Is.Null); + Assert.That(callees.Count, Is.EqualTo(1)); + Assert.That(callees[0].Label, Is.EqualTo(expectedLabel)); + Assert.That(callees[0].DeclaringFilePath, Is.EqualTo(HostPath)); + } + + /// + /// What: an entry that calls no added member records an empty list rather than null, so a + /// caller never has to tell "calls none" from "not recorded". + /// + [Test] + public void Resolve_EntryWithoutCalls_RecordsNone() + { + TransformWorkerEntryDto caller = BuildCallerEntry(); + caller.calledAddedMethodKeys = null; + HotReloadAddedCalleeIndex index = new HotReloadAddedCalleeIndex(new[] { caller }); + + (IReadOnlyList callees, string errorMessage) = index.Resolve(caller); + + Assert.That(errorMessage, Is.Null); + Assert.That(callees, Is.Empty); + } + + /// + /// What: a call whose key names no added entry of the group is refused with an error naming + /// the key, whether no entry has that key at all or only an entry that patches an existing + /// method does (that entry is not an added member, so its label must not be recorded). + /// + [TestCase(false)] + [TestCase(true)] + public void Resolve_CallNamingNoAddedEntry_ReportsTheKey(bool groupHasExistingMethodEntryWithThatKey) + { + const string wireKey = "Ns.Host::Bar(System.Int32)"; + TransformWorkerEntryDto caller = BuildCallerEntry(wireKey); + List entries = new List { caller }; + if (groupHasExistingMethodEntryWithThatKey) + { + TransformWorkerEntryDto existing = BuildAddedEntry("Ns.Host", "Bar", new[] { "System.Int32" }); + existing.patchKind = null; + entries.Add(existing); + } + + HotReloadAddedCalleeIndex index = new HotReloadAddedCalleeIndex(entries.ToArray()); + + (IReadOnlyList callees, string errorMessage) = index.Resolve(caller); + + Assert.That(callees, Is.Null); + Assert.That(errorMessage, Does.Contain(wireKey)); + } + + private static TransformWorkerEntryDto BuildAddedEntry( + string typeMetadataName, + string methodName, + string[] parameterTypeFullNames) + { + return new TransformWorkerEntryDto + { + sourceProjectRelativePath = HostPath, + typeMetadataName = typeMetadataName, + methodName = methodName, + parameterTypeFullNames = parameterTypeFullNames, + patchKind = HotReloadConstants.PatchKindAddedMethod + }; + } + + private static TransformWorkerEntryDto BuildCallerEntry(params string[] calledAddedMethodKeys) + { + return new TransformWorkerEntryDto + { + sourceProjectRelativePath = CallerPath, + typeMetadataName = "Ns.Caller", + methodName = "Call", + parameterTypeFullNames = new string[0], + calledAddedMethodKeys = calledAddedMethodKeys + }; + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadAddedCalleeIndexTests.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadAddedCalleeIndexTests.cs.meta new file mode 100644 index 000000000..20ffbb934 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadAddedCalleeIndexTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 487019f9af05d4d97906ef88eebf484b +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadCrossFileE2ETests.cs b/Assets/Tests/Editor/HotReload/HotReloadCrossFileE2ETests.cs index e20c5b6da..a24603a7d 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadCrossFileE2ETests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadCrossFileE2ETests.cs @@ -878,7 +878,9 @@ await HotReloadCompositionRoot.Services.Orchestrator.RunAsync( /// What: a sibling pulled in to re-bind is not described as re-applied when the host /// shim compile fails on a broken body beside the added method; isolation reports the /// caller as Skipped, the live patch stays on the previous body, and the sibling gets the - /// Skipped-only warning rather than being told the reload failed for it. + /// Skipped-only warning rather than being told the reload failed for it. The failed host + /// keeps its earlier registration of the added method, so the caller's patch is not named + /// as calling a retired member. /// [Test] public async Task Run_FailedSiblingRebindDoesNotClaimReApplied() @@ -949,6 +951,7 @@ await HotReloadCompositionRoot.Services.Orchestrator.RunAsync( HotReloadConstants.ActiveSiblingRebindSkippedOnlyWarningFormat, CallerProjectRelativePath())), string.Join("\n", second.Warnings)); + HotReloadStaleAddedMemberCallsWarnings.AssertNone(second.Warnings); Assert.That( new HotReloadCrossFileAddedMemberCaller().Call(new HotReloadCrossFileAddedMemberHost()), Is.EqualTo(5)); diff --git a/Assets/Tests/Editor/HotReload/HotReloadEntryResolutionTests.cs b/Assets/Tests/Editor/HotReload/HotReloadEntryResolutionTests.cs index 334b95629..686c36fcc 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadEntryResolutionTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadEntryResolutionTests.cs @@ -83,7 +83,8 @@ public void ResolveEntries_WhenEveryEntryResolves_ReportsAllResolved() FilePath, ShimAssembly, entries, - new Dictionary()); + new Dictionary(), + new HotReloadAddedCalleeIndex(entries)); Assert.That(result.AllResolved, Is.True); Assert.That(result.ResolvedEntries, Has.Count.EqualTo(2)); @@ -116,7 +117,8 @@ public void ResolveEntries_WhenShimMethodIsMissing_FailsTheFileAtomically() FilePath, ShimAssembly, entries, - new Dictionary()); + new Dictionary(), + new HotReloadAddedCalleeIndex(entries)); Assert.That(result.AllResolved, Is.False); Assert.That(result.ResolvedEntries, Is.Empty); @@ -159,7 +161,8 @@ public void ResolveEntries_WhenAddedMethodHasNoUsableCounter_FailsTheFileAtomica FilePath, ShimAssembly, entries, - new Dictionary()); + new Dictionary(), + new HotReloadAddedCalleeIndex(entries)); Assert.That(result.AllResolved, Is.False); Assert.That(result.ResolvedEntries, Is.Empty); @@ -197,7 +200,8 @@ public void ResolveEntries_WhenEntryIsAnAddedMethod_ResolvesWithoutAnOriginalMet FilePath, ShimAssembly, entries, - new Dictionary()); + new Dictionary(), + new HotReloadAddedCalleeIndex(entries)); Assert.That(result.AllResolved, Is.True); Assert.That(result.ResolvedEntries, Has.Count.EqualTo(1)); @@ -210,6 +214,83 @@ public void ResolveEntries_WhenEntryIsAnAddedMethod_ResolvesWithoutAnOriginalMet nameof(HotReloadHandwrittenShims.StaticPing__shim0__uloopCalls)))); } + /// + /// What: each resolved entry carries the added members its body calls, as the ledger + /// labels them, whether the entry patches an existing method or is an added method itself; + /// an entry that calls none carries an empty list. + /// + [Test] + public void ResolveEntries_RecordsTheAddedMembersEachEntryCalls() + { + TransformWorkerEntryDto added = BuildAddedMethodEntry(nameof(HotReloadHandwrittenShims.StaticPing__shim0)); + TransformWorkerEntryDto caller = BuildExistingMethodEntry( + nameof(HotReloadCoreFixture.StaticPing), + new string[0], + "StaticPing__shim0"); + caller.calledAddedMethodKeys = new[] { FixtureTypeMetadataName + "::AddedByThisReload()" }; + TransformWorkerEntryDto[] entries = { added, caller }; + + HotReloadEntryResolution.Result result = HotReloadEntryResolution.ResolveEntries( + TestAssemblyHome, + FileHomeResolver, + FilePath, + ShimAssembly, + entries, + new Dictionary(), + new HotReloadAddedCalleeIndex(entries)); + + Assert.That(result.AllResolved, Is.True); + Assert.That(result.ResolvedEntries[0].CalledAddedMembers, Is.Empty); + Assert.That(result.ResolvedEntries[1].CalledAddedMembers.Count, Is.EqualTo(1)); + Assert.That( + result.ResolvedEntries[1].CalledAddedMembers[0].Label, + Is.EqualTo(FixtureTypeMetadataName + ".AddedByThisReload()")); + Assert.That(result.ResolvedEntries[1].CalledAddedMembers[0].DeclaringFilePath, Is.EqualTo(FilePath)); + } + + /// + /// What: an entry calling an added member the group declares no entry for fails the whole + /// file like a missing shim does, and the Failed row names the call's key. + /// + [Test] + public void ResolveEntries_WhenACallNamesNoAddedEntry_FailsTheFileAtomically() + { + string missingKey = FixtureTypeMetadataName + "::RetiredByThisReload()"; + TransformWorkerEntryDto caller = BuildExistingMethodEntry( + nameof(HotReloadCoreFixture.ReplaceableCompute), + new[] { "System.Int32" }, + "ReplaceableCompute__shim0"); + caller.calledAddedMethodKeys = new[] { missingKey }; + TransformWorkerEntryDto[] entries = + { + BuildExistingMethodEntry( + nameof(HotReloadCoreFixture.StaticPing), + new string[0], + "StaticPing__shim0"), + caller + }; + + HotReloadEntryResolution.Result result = HotReloadEntryResolution.ResolveEntries( + TestAssemblyHome, + FileHomeResolver, + FilePath, + ShimAssembly, + entries, + new Dictionary(), + new HotReloadAddedCalleeIndex(entries)); + + Assert.That(result.AllResolved, Is.False); + Assert.That(result.ResolvedEntries, Is.Empty); + Assert.That(result.FailureOutcomes, Has.Count.EqualTo(2)); + Assert.That( + result.FailureOutcomes[0].Reason, + Is.EqualTo(HotReloadConstants.AtomicFileSkipReason)); + Assert.That( + result.FailureOutcomes[1].Kind, + Is.EqualTo(HotReloadMethodOutcomeKind.Failed)); + Assert.That(result.FailureOutcomes[1].Reason, Does.Contain(missingKey)); + } + private static string ResolveProjectRoot() { return Path.GetFullPath(Path.Combine(Application.dataPath, "..")); diff --git a/Assets/Tests/Editor/HotReload/HotReloadFileGenerationTests.cs b/Assets/Tests/Editor/HotReload/HotReloadFileGenerationTests.cs index 718124c5c..8165887a6 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadFileGenerationTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadFileGenerationTests.cs @@ -369,6 +369,97 @@ public void DeactivateThenReactivatePatch_RestoresTheLivePatch() Assert.That(generation.ActivePatchCount, Is.EqualTo(1)); } + /// + /// What: a patch keeps the added members it was opened with as its calls from pending + /// through live, deactivated, and reactivated, so a restored patch still says what it + /// calls. + /// + [Test] + public void BeginPatch_KeepsItsCalledAddedMembersThroughCommitDeactivateAndReactivate() + { + HotReloadFileGeneration generation = CreateGeneration(); + BeginShimGeneration(generation); + RegisterShim(generation); + HotReloadCalledAddedMember[] calls = { CreateCall("Bar") }; + generation.BeginPatch(GetShimTarget(), GetAddedTarget(), calls); + generation.CommitPatch(GetShimTarget()); + + generation.ReactivatePatch(generation.DeactivatePatch(GetShimTarget())); + + Assert.That(generation.DeactivatePatch(GetShimTarget()).CalledAddedMembers, Is.EqualTo(calls)); + } + + /// + /// What: an added member keeps the added members it was registered as calling, and one + /// registered without any carries an empty list rather than null. + /// + [Test] + public void RegisterAddedMethod_KeepsItsCalledAddedMembers() + { + HotReloadFileGeneration generation = CreateGeneration(); + generation.BeginAddedMemberGeneration(); + HotReloadCalledAddedMember[] calls = { CreateCall("Bar") }; + generation.RegisterAddedMethod( + AddedMethodKey, + GetAddedTarget(), + FixtureProjectRelativePath, + "AddedMember", + AddedMethodType, + HotReloadUnreadInvocationCounter.Field, + calledAddedMembers: calls); + generation.RegisterAddedMethod( + OtherAddedMethodKey, + GetAddedTarget(), + FixtureProjectRelativePath, + "OtherAddedMember", + AddedMethodType, + HotReloadUnreadInvocationCounter.Field); + + Assert.That(generation.FindAddedMember(AddedMethodKey).CalledAddedMembers, Is.EqualTo(calls)); + Assert.That(generation.FindAddedMember(OtherAddedMethodKey).CalledAddedMembers, Is.Empty); + } + + /// + /// What: the calls a generation reports are those of its live patches, named by the + /// patched method's label, and those of its registered added members, named by their key, + /// each with the generation's path; a patch Harmony has not accepted yet reports none. + /// + [Test] + public void CollectAddedMemberCalls_ReportsLivePatchesAndAddedMembersButNotPendingPatches() + { + HotReloadFileGeneration generation = CreateGeneration(); + BeginShimGeneration(generation); + RegisterShim(generation); + generation.RegisterShimMethod( + GetAddedTarget(), + new HotReloadShimMethodEntry(GetShimTarget(), false, 3, 4)); + HotReloadCalledAddedMember livePatchCall = CreateCall("FromLivePatch"); + generation.BeginPatch(GetShimTarget(), GetAddedTarget(), new[] { livePatchCall }); + generation.CommitPatch(GetShimTarget()); + generation.BeginPatch(GetAddedTarget(), GetShimTarget(), new[] { CreateCall("FromPendingPatch") }); + generation.BeginAddedMemberGeneration(); + HotReloadCalledAddedMember addedMemberCall = CreateCall("FromAddedMember"); + generation.RegisterAddedMethod( + AddedMethodKey, + GetAddedTarget(), + FixtureProjectRelativePath, + "AddedMember", + AddedMethodType, + HotReloadUnreadInvocationCounter.Field, + calledAddedMembers: new[] { addedMemberCall }); + + List calls = new List(); + generation.CollectAddedMemberCalls(calls); + + Assert.That(calls.Count, Is.EqualTo(2)); + Assert.That(calls[0].CallerLabel, Is.EqualTo(HotReloadMethodKeys.FormatMethodLabel(GetShimTarget()))); + Assert.That(calls[0].CallerFilePath, Is.EqualTo(FixtureProjectRelativePath)); + Assert.That(calls[0].Callee, Is.SameAs(livePatchCall)); + Assert.That(calls[1].CallerLabel, Is.EqualTo(AddedMethodKey)); + Assert.That(calls[1].CallerFilePath, Is.EqualTo(FixtureProjectRelativePath)); + Assert.That(calls[1].Callee, Is.SameAs(addedMemberCall)); + } + /// /// What: deactivating a method that holds no live patch returns null instead of inventing /// an entry a caller would restore. @@ -635,6 +726,11 @@ private static HotReloadFileGeneration CreateGeneration() return new HotReloadFileGeneration(FixtureProjectRelativePath); } + private static HotReloadCalledAddedMember CreateCall(string addedMethodName) + { + return new HotReloadCalledAddedMember(HostType + "." + addedMethodName + "()", "Assets/Host.cs"); + } + // The store key spells nested types the metadata way, which is what the worker forms and // the Editor carries unchanged, so the fixture builds it from the type name it was given. private static HotReloadAddedFieldDeclaration CreateDeclaration( diff --git a/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs b/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs index 659f7ccd0..1ea8d7d38 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs @@ -4590,8 +4590,9 @@ public async Task Run_BrokenSiblingOfAddedMethodAfterSuccess_ObservesRegistryWhe /// refused by the worker before any shim is compiled: the added method and its caller are /// Skipped with the worker's reasons, nothing fails, and the run deactivates the earlier /// AddedPing registration with one warning naming it and telling the reader to change what - /// the skip reason names before reloading. A third run with the body fixed - /// registers AddedPing again and the caller returns the new value. + /// the skip reason names before reloading, and names the caller's earlier patch as still + /// calling the retired AddedPing. A third run with the body fixed registers AddedPing + /// again, the caller returns the new value, and no call is named any more. /// [Test] public async Task Run_UnboundAddedMethodAfterSuccess_DeactivatesItUntilTheBodyBindsAgain() @@ -4625,6 +4626,16 @@ public async Task Run_UnboundAddedMethodAfterSuccess_DeactivatesItUntilTheBodyBi AssertDeactivatedPatchesWarningsEqual( second, ExpectedDeactivatedSkippedAddedMembersWarning(AddedPingMethodLabel())); + Assert.That( + second.Warnings, + Does.Contain( + HotReloadStaleAddedMemberCallsWarnings.Expected( + HotReloadStaleAddedMemberCallsWarnings.Pair( + HotReloadMethodKeys.FormatMethodLabel( + typeof(HotReloadAddedMethodApplyFixture).GetMethod( + nameof(HotReloadAddedMethodApplyFixture.ExistingCaller))), + AddedPingMethodLabel()))), + string.Join("\n", second.Warnings)); string fixedBody = WithWorkingAddedPing(onDisk).Replace( " return value + 1;\n }", @@ -4638,6 +4649,7 @@ public async Task Run_UnboundAddedMethodAfterSuccess_DeactivatesItUntilTheBodyBi AssertHasAdded(third, "AddedPing"); Assert.That(CountAddedMembersContaining("AddedPing"), Is.EqualTo(1)); Assert.That(new HotReloadAddedMethodApplyFixture().ExistingCaller(3), Is.EqualTo(8)); + HotReloadStaleAddedMemberCallsWarnings.AssertNone(third.Warnings); } /// @@ -4922,7 +4934,8 @@ public async Task Run_VirtualAddedMethodAfterSuccess_EmptyEntries_WarnsDeactivat /// /// What: deleting a previously added method and restoring its caller does not emit - /// the added-member deactivation warning (intentional convergence). + /// the added-member deactivation warning (intentional convergence), nor name the caller as + /// still calling the deleted method, because the restored caller's patch is reverted. /// [Test] public async Task Run_DeleteAddedMethodAndRestoreCaller_DoesNotWarnDeactivatedAddedMembers() @@ -4941,6 +4954,7 @@ public async Task Run_DeleteAddedMethodAndRestoreCaller_DoesNotWarnDeactivatedAd CancellationToken.None); AssertNoDeactivatedPatchesWarning(second); + HotReloadStaleAddedMemberCallsWarnings.AssertNone(second.Warnings); } /// diff --git a/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallE2ETests.cs b/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallE2ETests.cs new file mode 100644 index 000000000..b92db2642 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallE2ETests.cs @@ -0,0 +1,326 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +using NUnit.Framework; + +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// End-to-end coverage of the warning a run gives when a caller that did not apply keeps + /// calling an added member a later reload retired: the member leaves --status, but the caller's + /// earlier patch still runs the member's earlier body. + /// + public class HotReloadStaleAddedMemberCallE2ETests + { + private const string HostFileName = "HotReloadCrossFileAddedMemberHost.cs"; + private const string CallerFileName = "HotReloadCrossFileAddedMemberCaller.cs"; + private const string HostValueAnchor = " public int Value()"; + private const string CallerCallBodyAnchor = "return host.Value();"; + private const string CallerOtherBodyAnchor = " return 7;"; + private const string CallerSecondMemberAnchor = " // Second editable member of this file"; + private const string OneArgumentAddedMember = + " public int Added(int seed)\n {\n return seed + 40;\n }\n\n"; + private const string TwoArgumentAddedMember = + " public int Added(int seed, int extra)\n {\n return seed + extra;\n }\n\n"; + + // A type error in an existing body fails the caller's shim compile, so isolation fails the + // whole file and leaves its earlier generation in place, whatever the rest of the file says. + private const string BrokenOtherBody = " int broken = \"not an int\";\n return broken;"; + + private HotReloadDomainTestScope _scope; + + [SetUp] + public void SetUp() + { + _scope = new HotReloadDomainTestScope(); + } + + [TearDown] + public void TearDown() + { + _scope.Dispose(); + } + + /// + /// What: a caller that fails in the run that changes the signature of the added member it + /// calls keeps running the member's earlier body, and that run names the call; a later run + /// given only the member's unchanged file names it again; the run that applies the caller on + /// the new signature no longer does. + /// + [Test] + public async Task Run_CallerFailsWhenItsAddedCalleeChangesSignature_NamesTheCallUntilTheCallerApplies() + { + string hostPath = FixturePath(HostFileName); + string callerPath = FixturePath(CallerFileName); + HotReloadOrchestratorResult first = await RunAsync( + new[] { hostPath, callerPath }, + Override(hostPath, "StaleCallFailedHost1.cs", InsertHostMember(OneArgumentAddedMember)), + Override(callerPath, "StaleCallFailedCaller1.cs", ReplaceInCaller(CallerCallBodyAnchor, "return host.Added(1);"))); + AssertNoFailure(first); + HotReloadStaleAddedMemberCallsWarnings.AssertNone(first.Warnings); + Assert.That(CallThroughCaller(), Is.EqualTo(41)); + + string secondHostEditPath = HotReloadTestSourceWriter.WriteEditedSource( + "StaleCallFailedHost2.cs", + InsertHostMember(TwoArgumentAddedMember)); + HotReloadOrchestratorResult second = await RunAsync( + new[] { hostPath, callerPath }, + (hostPath, secondHostEditPath), + Override( + callerPath, + "StaleCallFailedCaller2.cs", + ReplaceInCaller(CallerCallBodyAnchor, "return host.Added(1);", CallerOtherBodyAnchor, BrokenOtherBody))); + AssertHasOutcomeForFile(second, callerPath, HotReloadMethodOutcomeKind.Failed); + AssertHasOutcome(second, HotReloadMethodOutcomeKind.Added, ".Added(System.Int32,System.Int32)"); + Assert.That(second.Warnings, Does.Contain(ExpectedCallIntoOneArgumentAdded()), string.Join("\n", second.Warnings)); + Assert.That(CallThroughCaller(), Is.EqualTo(41)); + + HotReloadOrchestratorResult third = await RunAsync(new[] { hostPath }, (hostPath, secondHostEditPath)); + AssertHasOutcome(third, HotReloadMethodOutcomeKind.AlreadyActive, ".Added("); + Assert.That(third.Warnings, Does.Contain(ExpectedCallIntoOneArgumentAdded()), string.Join("\n", third.Warnings)); + + HotReloadOrchestratorResult fourth = await RunAsync( + new[] { hostPath, callerPath }, + (hostPath, secondHostEditPath), + Override(callerPath, "StaleCallFailedCaller4.cs", ReplaceInCaller(CallerCallBodyAnchor, "return host.Added(1, 2);"))); + AssertNoFailure(fourth); + HotReloadStaleAddedMemberCallsWarnings.AssertNone(fourth.Warnings); + Assert.That(CallThroughCaller(), Is.EqualTo(3)); + } + + /// + /// What: a caller pulled back in to re-bind when the added member it calls changes signature + /// is Skipped because its unchanged body no longer binds, keeps running the member's earlier + /// body, and the run names the call beside the Skipped-only sibling warning. + /// + [Test] + public async Task Run_SiblingSkippedWhenItsAddedCalleeChangesSignature_NamesTheCall() + { + string hostPath = FixturePath(HostFileName); + string callerPath = FixturePath(CallerFileName); + string firstCallerEditPath = HotReloadTestSourceWriter.WriteEditedSource( + "StaleCallSiblingCaller1.cs", + ReplaceInCaller(CallerCallBodyAnchor, "return host.Added(1);")); + HotReloadOrchestratorResult first = await RunAsync( + new[] { hostPath, callerPath }, + Override(hostPath, "StaleCallSiblingHost1.cs", InsertHostMember(OneArgumentAddedMember)), + (callerPath, firstCallerEditPath)); + AssertNoFailure(first); + + HotReloadOrchestratorResult second = await RunAsync( + new[] { hostPath }, + Override(hostPath, "StaleCallSiblingHost2.cs", InsertHostMember(TwoArgumentAddedMember)), + (callerPath, firstCallerEditPath)); + + AssertHasOutcomeForFile(second, CallerProjectRelativePath(), HotReloadMethodOutcomeKind.Skipped); + Assert.That( + second.Warnings, + Does.Contain( + string.Format( + HotReloadConstants.ActiveSiblingRebindSkippedOnlyWarningFormat, + CallerProjectRelativePath())), + string.Join("\n", second.Warnings)); + Assert.That(second.Warnings, Does.Contain(ExpectedCallIntoOneArgumentAdded()), string.Join("\n", second.Warnings)); + Assert.That(CallThroughCaller(), Is.EqualTo(41)); + } + + /// + /// What: an added member that stays registered because its own file failed, and whose body + /// calls an added member another file retired, is the caller the run names; the patch that + /// calls the still-registered added member is not named. + /// + [Test] + public async Task Run_RegisteredAddedMemberCallsARetiredAddedMember_NamesTheAddedMember() + { + string hostPath = FixturePath(HostFileName); + string callerPath = FixturePath(CallerFileName); + const string helperMember = + " public int Helper(HotReloadCrossFileAddedMemberHost host)\n {\n return host.Added(1);\n }\n\n"; + HotReloadOrchestratorResult first = await RunAsync( + new[] { hostPath, callerPath }, + Override(hostPath, "StaleCallAddedCallerHost1.cs", InsertHostMember(OneArgumentAddedMember)), + Override( + callerPath, + "StaleCallAddedCallerCaller1.cs", + ReplaceInCaller( + CallerCallBodyAnchor, + "return Helper(host);", + CallerSecondMemberAnchor, + helperMember + CallerSecondMemberAnchor))); + AssertNoFailure(first); + Assert.That(CallThroughCaller(), Is.EqualTo(41)); + + HotReloadOrchestratorResult second = await RunAsync( + new[] { hostPath, callerPath }, + Override(hostPath, "StaleCallAddedCallerHost2.cs", InsertHostMember(TwoArgumentAddedMember)), + Override( + callerPath, + "StaleCallAddedCallerCaller2.cs", + ReplaceInCaller( + CallerCallBodyAnchor, + "return Helper(host);", + CallerSecondMemberAnchor, + helperMember.Replace("host.Added(1)", "host.Added(1, 0)") + CallerSecondMemberAnchor, + CallerOtherBodyAnchor, + BrokenOtherBody))); + + AssertHasOutcomeForFile(second, callerPath, HotReloadMethodOutcomeKind.Failed); + string helperLabel = HotReloadMethodKeys.FormatMethodLabelParts( + new HotReloadMetadataTypeName(typeof(HotReloadCrossFileAddedMemberCaller).FullName), + "Helper", + new[] { typeof(HotReloadCrossFileAddedMemberHost).FullName }, + 0); + Assert.That( + second.Warnings, + Does.Contain( + HotReloadStaleAddedMemberCallsWarnings.Expected( + HotReloadStaleAddedMemberCallsWarnings.Pair(helperLabel, OneArgumentAddedLabel()))), + string.Join("\n", second.Warnings)); + Assert.That(CallThroughCaller(), Is.EqualTo(41)); + } + + private static string ExpectedCallIntoOneArgumentAdded() + { + string callLabel = HotReloadMethodKeys.FormatMethodLabel( + typeof(HotReloadCrossFileAddedMemberCaller).GetMethod(nameof(HotReloadCrossFileAddedMemberCaller.Call))); + return HotReloadStaleAddedMemberCallsWarnings.Expected( + HotReloadStaleAddedMemberCallsWarnings.Pair(callLabel, OneArgumentAddedLabel())); + } + + private static string OneArgumentAddedLabel() + { + return HotReloadMethodKeys.FormatMethodLabelParts( + new HotReloadMetadataTypeName(typeof(HotReloadCrossFileAddedMemberHost).FullName), + "Added", + new[] { "System.Int32" }, + 0); + } + + private static int CallThroughCaller() + { + return new HotReloadCrossFileAddedMemberCaller().Call(new HotReloadCrossFileAddedMemberHost()); + } + + private static (string Path, string EditedSourcePath) Override(string path, string editedFileName, string source) + { + return (path, HotReloadTestSourceWriter.WriteEditedSource(editedFileName, source)); + } + + private static Task RunAsync( + string[] files, + params (string Path, string EditedSourcePath)[] overrides) + { + Dictionary overrideByFile = new Dictionary(); + foreach ((string path, string editedSourcePath) in overrides) + { + overrideByFile[path] = editedSourcePath; + } + + return HotReloadCompositionRoot.Services.Orchestrator.RunAsync( + files, + contentPathOverride: null, + CancellationToken.None, + overrideByFile); + } + + private static string InsertHostMember(string memberText) + { + string source = File.ReadAllText(FixturePath(HostFileName)); + Assert.That(source, Does.Contain(HostValueAnchor), "Precondition: host anchor must exist."); + return source.Replace(HostValueAnchor, memberText + HostValueAnchor, StringComparison.Ordinal); + } + + // Pairs of anchor and replacement, applied in order to the caller fixture. + private static string ReplaceInCaller(params string[] anchorsAndReplacements) + { + string source = File.ReadAllText(FixturePath(CallerFileName)); + for (int index = 0; index < anchorsAndReplacements.Length; index += 2) + { + Assert.That( + source, + Does.Contain(anchorsAndReplacements[index]), + "Precondition: caller anchor must exist: " + anchorsAndReplacements[index]); + source = source.Replace( + anchorsAndReplacements[index], + anchorsAndReplacements[index + 1], + StringComparison.Ordinal); + } + + return source; + } + + private static void AssertNoFailure(HotReloadOrchestratorResult result) + { + foreach (HotReloadMethodOutcome outcome in result.Methods) + { + Assert.That( + outcome.Kind, + Is.Not.EqualTo(HotReloadMethodOutcomeKind.Failed), + outcome.Method + " " + outcome.Reason); + } + } + + private static void AssertHasOutcome( + HotReloadOrchestratorResult result, + HotReloadMethodOutcomeKind kind, + string methodToken) + { + foreach (HotReloadMethodOutcome outcome in result.Methods) + { + if (outcome.Kind == kind && outcome.Method.Contains(methodToken)) + { + return; + } + } + + Assert.Fail("No " + kind + " outcome containing '" + methodToken + "' in:\n" + FormatOutcomes(result)); + } + + private static void AssertHasOutcomeForFile( + HotReloadOrchestratorResult result, + string filePath, + HotReloadMethodOutcomeKind kind) + { + foreach (HotReloadMethodOutcome outcome in result.Methods) + { + if (outcome.Kind == kind && outcome.FilePath == filePath) + { + return; + } + } + + Assert.Fail("No " + kind + " outcome for '" + filePath + "' in:\n" + FormatOutcomes(result)); + } + + private static string FormatOutcomes(HotReloadOrchestratorResult result) + { + List lines = new List(); + foreach (HotReloadMethodOutcome outcome in result.Methods) + { + lines.Add(outcome.Kind + " " + outcome.Method + " [" + outcome.FilePath + "] " + outcome.Reason); + } + + return string.Join("\n", lines); + } + + private static string FixturePath(string fileName) + { + string path = Path.GetFullPath( + Path.Combine(Application.dataPath, "Tests", "Editor", "HotReload", fileName)); + Assert.That(File.Exists(path), Is.True, "Fixture missing: " + path); + return path; + } + + private static string CallerProjectRelativePath() + { + return "Assets/Tests/Editor/HotReload/" + CallerFileName; + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallE2ETests.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallE2ETests.cs.meta new file mode 100644 index 000000000..988b0506c --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallE2ETests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 4362d4fd3cb78446892cd909307f39b6 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallersTests.cs b/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallersTests.cs new file mode 100644 index 000000000..8a2bd85b8 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallersTests.cs @@ -0,0 +1,105 @@ +using System; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// Pure coverage for the run-level warning about calls that earlier reloads left running into + /// added members no generation registers any more: which calls it names, and on which runs. + /// + public class HotReloadStaleAddedMemberCallersTests + { + private const string CallerPath = "Assets/Caller.cs"; + private const string HostPath = "Assets/Host.cs"; + private const string CallerLabel = "Ns.Caller.Call()"; + private const string RetiredLabel = "Ns.Host.Bar(System.Int32)"; + + /// + /// What: a call into an added member no generation registers is named as "caller calls + /// member" in the one warning of a run that touched both files. + /// + [Test] + public void DescribeOrNull_CallIntoAnUnregisteredMember_NamesThePair() + { + string warning = new HotReloadStaleAddedMemberCallers().DescribeOrNull( + new[] { CreateCall(CallerLabel, RetiredLabel) }, + Array.Empty(), + new[] { CallerPath, HostPath }); + + Assert.That( + warning, + Is.EqualTo( + string.Format( + HotReloadConstants.StaleAddedMemberCallsWarningFormat, + CallerLabel + " calls " + RetiredLabel))); + } + + /// + /// What: a call into a member some generation still registers under that label is not + /// reported, whichever file registers it now. + /// + [Test] + public void DescribeOrNull_CallIntoARegisteredMember_ReportsNothing() + { + string warning = new HotReloadStaleAddedMemberCallers().DescribeOrNull( + new[] { CreateCall(CallerLabel, RetiredLabel) }, + new[] { new HotReloadAddedMemberInfo(RetiredLabel, "Assets/HostPart.cs", null) }, + new[] { CallerPath, HostPath }); + + Assert.That(warning, Is.Null); + } + + /// + /// What: a run that touched only the caller's file, or only the file that declared the + /// member, still reports the call; a run that touched neither does not. + /// + [TestCase(CallerPath, true)] + [TestCase(HostPath, true)] + [TestCase("Assets/Other.cs", false)] + public void DescribeOrNull_ReportsOnlyWhenTheRunTouchedEitherEnd(string pathInRun, bool expectWarning) + { + string warning = new HotReloadStaleAddedMemberCallers().DescribeOrNull( + new[] { CreateCall(CallerLabel, RetiredLabel) }, + Array.Empty(), + new[] { pathInRun }); + + Assert.That(warning != null, Is.EqualTo(expectWarning), warning); + } + + /// + /// What: several stale calls are listed once each, in ordinal order, whatever order the + /// generations reported them in. + /// + [Test] + public void DescribeOrNull_ListsEachStaleCallOnceInOrdinalOrder() + { + string warning = new HotReloadStaleAddedMemberCallers().DescribeOrNull( + new[] + { + CreateCall("Ns.Caller.Zed()", RetiredLabel), + CreateCall(CallerLabel, RetiredLabel), + CreateCall(CallerLabel, RetiredLabel) + }, + Array.Empty(), + new[] { CallerPath }); + + Assert.That( + warning, + Is.EqualTo( + string.Format( + HotReloadConstants.StaleAddedMemberCallsWarningFormat, + CallerLabel + " calls " + RetiredLabel + ", Ns.Caller.Zed() calls " + RetiredLabel))); + } + + private static HotReloadAddedMemberCall CreateCall(string callerLabel, string calleeLabel) + { + return new HotReloadAddedMemberCall( + callerLabel, + CallerPath, + new HotReloadCalledAddedMember(calleeLabel, HostPath)); + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallersTests.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallersTests.cs.meta new file mode 100644 index 000000000..64783b9cb --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallersTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 391722b9d9d0e4aac9edfed11a479b44 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallsWarnings.cs b/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallsWarnings.cs new file mode 100644 index 000000000..87cd4881f --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallsWarnings.cs @@ -0,0 +1,42 @@ +using System; +using System.Collections.Generic; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// Builds and finds the run-level warning about calls left running into retired added members, + /// for tests that pin whether a run reports it. + /// + internal static class HotReloadStaleAddedMemberCallsWarnings + { + internal static string Expected(params string[] callerCallsMemberPairs) + { + return string.Format( + HotReloadConstants.StaleAddedMemberCallsWarningFormat, + string.Join(", ", callerCallsMemberPairs)); + } + + internal static string Pair(string callerLabel, string memberLabel) + { + return callerLabel + " calls " + memberLabel; + } + + /// + /// Fails when any warning is this one, whatever calls it names; the deactivation filters + /// other tests use would let it through. + /// + internal static void AssertNone(IReadOnlyList warnings) + { + string format = HotReloadConstants.StaleAddedMemberCallsWarningFormat; + string prefix = format.Substring(0, format.IndexOf("{0}", StringComparison.Ordinal)); + foreach (string warning in warnings) + { + Assert.That(warning, Does.Not.StartWith(prefix), string.Join("\n", warnings)); + } + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallsWarnings.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallsWarnings.cs.meta new file mode 100644 index 000000000..cb828ea89 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadStaleAddedMemberCallsWarnings.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 7a9e05510490749ff9b92dba089f411b +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadAddedCalleeIndex.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadAddedCalleeIndex.cs new file mode 100644 index 000000000..45af15de3 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadAddedCalleeIndex.cs @@ -0,0 +1,61 @@ +using System; +using System.Collections.Generic; + +using UnityEngine; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// The added members one group's worker output declares, keyed by the wire key its entries use + /// in calledAddedMethodKeys, so each entry's calls are recorded in the ledger's labels. + /// + internal sealed class HotReloadAddedCalleeIndex + { + private readonly Dictionary _calleesByWireKey = + new Dictionary(StringComparer.Ordinal); + + internal HotReloadAddedCalleeIndex(TransformWorkerEntryDto[] groupEntries) + { + Debug.Assert(groupEntries != null, "groupEntries must not be null."); + + foreach (TransformWorkerEntryDto entry in groupEntries) + { + // Why only added entries: an entry that patches an existing method shares the key + // shape, but a call to it is a call to compiled code, not to an added member. + if (entry.patchKind != HotReloadConstants.PatchKindAddedMethod) + { + continue; + } + + _calleesByWireKey[HotReloadMethodKeys.BuildMethodKey(entry)] = + new HotReloadCalledAddedMember( + HotReloadEntryResolution.FormatEntryLabel(entry), + entry.sourceProjectRelativePath); + } + } + + /// + /// The added members calls, or the error naming the first call + /// this group declares no added member for. + /// + internal (IReadOnlyList Callees, string ErrorMessage) Resolve( + TransformWorkerEntryDto entry) + { + Debug.Assert(entry != null, "entry must not be null."); + + string[] calledKeys = entry.calledAddedMethodKeys ?? Array.Empty(); + List callees = new List(calledKeys.Length); + foreach (string calledKey in calledKeys) + { + if (!_calleesByWireKey.TryGetValue(calledKey ?? string.Empty, out HotReloadCalledAddedMember callee)) + { + return (null, "Called added member not found among this reload's added members: " + calledKey); + } + + callees.Add(callee); + } + + return (callees, null); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadAddedCalleeIndex.cs.meta b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadAddedCalleeIndex.cs.meta new file mode 100644 index 000000000..050a9a99a --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadAddedCalleeIndex.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 0c8585e7ea88e4b91b4b727efd248e60 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEntryResolution.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEntryResolution.cs index 152e90315..b8607228a 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEntryResolution.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEntryResolution.cs @@ -16,13 +16,16 @@ internal static class HotReloadEntryResolution { // Why bindFailures is passed in: one shim assembly serves every file of a group, so its // accessor binders run once for the group instead of once per file. + // Why addedCallees is passed in: a body can call an added member another file of the group + // declares, so one file's entries alone cannot name every call. internal static Result ResolveEntries( HotReloadTypeHome fileHome, HotReloadEntryHomeResolver homeResolver, string filePath, Assembly shimAssembly, TransformWorkerEntryDto[] entriesToPatch, - Dictionary bindFailures) + Dictionary bindFailures, + HotReloadAddedCalleeIndex addedCallees) { Debug.Assert(fileHome != null, "fileHome must not be null."); Debug.Assert(homeResolver != null, "homeResolver must not be null."); @@ -30,6 +33,7 @@ internal static Result ResolveEntries( Debug.Assert(shimAssembly != null, "shimAssembly must not be null."); Debug.Assert(entriesToPatch != null, "entriesToPatch must not be null."); Debug.Assert(bindFailures != null, "bindFailures must not be null."); + Debug.Assert(addedCallees != null, "addedCallees must not be null."); List resolvedEntries = new List(); for (int index = 0; index < entriesToPatch.Length; index++) @@ -40,6 +44,7 @@ internal static Result ResolveEntries( homeResolver, shimAssembly, bindFailures, + addedCallees, filePath); if (entryOutcome.IsFailure) { @@ -111,9 +116,22 @@ private static ResolvedEntryOutcome TryResolveEntry( HotReloadEntryHomeResolver homeResolver, Assembly shimAssembly, IReadOnlyDictionary bindFailures, + HotReloadAddedCalleeIndex addedCallees, string filePath) { string methodLabel = FormatEntryLabel(entry); + // Why a call the group declares no added member for fails the file like a missing shim + // does: every added member an applied body calls comes from the same worker output, so + // such a call means the worker and this Editor disagree, and recording the entry + // without it would hide the call from the check for retired callees. + (IReadOnlyList calledAddedMembers, string calleeError) = + addedCallees.Resolve(entry); + if (calleeError != null) + { + return ResolvedEntryOutcome.Failed( + HotReloadMethodOutcome.Failed(methodLabel, calleeError, filePath)); + } + if (entry.patchKind == HotReloadConstants.PatchKindAddedMethod) { return TryResolveAddedMethod( @@ -121,6 +139,7 @@ private static ResolvedEntryOutcome TryResolveEntry( methodLabel, shimAssembly, bindFailures, + calledAddedMembers, filePath); } @@ -131,6 +150,7 @@ private static ResolvedEntryOutcome TryResolveEntry( homeResolver, shimAssembly, bindFailures, + calledAddedMembers, filePath); } @@ -139,6 +159,7 @@ private static ResolvedEntryOutcome TryResolveAddedMethod( string methodLabel, Assembly shimAssembly, IReadOnlyDictionary bindFailures, + IReadOnlyList calledAddedMembers, string filePath) { if (bindFailures.TryGetValue(entry.shimTypeName ?? string.Empty, out string bindFailureReason)) @@ -170,7 +191,8 @@ private static ResolvedEntryOutcome TryResolveAddedMethod( originalMethod: null, shimMethod, isAddedMethod: true, - invocationCounter)); + invocationCounter, + calledAddedMembers)); } // Why a missing counter fails the file like a missing shim does: the counter is what @@ -204,6 +226,7 @@ private static ResolvedEntryOutcome TryResolveExistingMethod( HotReloadEntryHomeResolver homeResolver, Assembly shimAssembly, IReadOnlyDictionary bindFailures, + IReadOnlyList calledAddedMembers, string filePath) { HotReloadPatchShape patchShape = entry.patchKind == HotReloadConstants.PatchKindDelegation @@ -257,7 +280,8 @@ private static ResolvedEntryOutcome TryResolveExistingMethod( matchResult.Method, shimMethod, isAddedMethod: false, - invocationCounter: null)); + invocationCounter: null, + calledAddedMembers)); } private static (MethodInfo ShimMethod, string ErrorMessage) FindShimMethod( @@ -312,7 +336,9 @@ private static Type FindShimType(Assembly shimAssembly, string shimTypeName) return null; } - private static string FormatEntryLabel(TransformWorkerEntryDto entry) + // Why internal: the added-callee index records calls in this same label, the one an added + // member is registered under, so the two cannot drift apart. + internal static string FormatEntryLabel(TransformWorkerEntryDto entry) { return HotReloadMethodKeys.FormatMethodLabelParts( new HotReloadMetadataTypeName(entry.typeMetadataName), @@ -340,6 +366,9 @@ internal sealed class ResolvedEntry /// public FieldInfo InvocationCounter { get; } + /// The added members this entry's body calls; empty when it calls none. + public IReadOnlyList CalledAddedMembers { get; } + public ResolvedEntry( TransformWorkerEntryDto entry, string methodLabel, @@ -348,8 +377,11 @@ public ResolvedEntry( MethodBase originalMethod, MethodInfo shimMethod, bool isAddedMethod, - FieldInfo invocationCounter) + FieldInfo invocationCounter, + IReadOnlyList calledAddedMembers) { + Debug.Assert(calledAddedMembers != null, "calledAddedMembers must not be null."); + Entry = entry; MethodLabel = methodLabel; FilePath = filePath; @@ -358,6 +390,7 @@ public ResolvedEntry( ShimMethod = shimMethod; IsAddedMethod = isAddedMethod; InvocationCounter = invocationCounter; + CalledAddedMembers = calledAddedMembers; } } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadFileEntryApplier.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadFileEntryApplier.cs index b9e474964..7f02b3d0e 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadFileEntryApplier.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadFileEntryApplier.cs @@ -410,7 +410,8 @@ private HotReloadMethodOutcome ApplyResolvedEntry( resolved.Entry.typeMetadataName, resolved.InvocationCounter, resolved.Entry.sourceStartLine, - resolved.Entry.sourceEndLine); + resolved.Entry.sourceEndLine, + resolved.CalledAddedMembers); return HotReloadMethodOutcome.Added( resolved.MethodLabel, resolved.FilePath, @@ -430,7 +431,8 @@ private HotReloadMethodOutcome ApplyResolvedEntry( resolved.OriginalMethod, resolved.ShimMethod, resolved.PatchShape, - projectRelativePath); + projectRelativePath, + resolved.CalledAddedMembers); if (!patchResult.Success) { generation.RemoveShimMethod(resolved.OriginalMethod); diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupEntryPreparation.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupEntryPreparation.cs index b1fb5862c..d2c29c6f8 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupEntryPreparation.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupEntryPreparation.cs @@ -40,12 +40,15 @@ internal static IReadOnlyList PrepareGroup( // same retained home no matter which file it came from. HotReloadEntryHomeResolver homeResolver = new HotReloadEntryHomeResolver(collaborators.Domain, context.ProjectRoot); + // Why once for the group: a body can call an added member another file of the group + // declares, and only the whole group's entries name every added member it may call. + HotReloadAddedCalleeIndex addedCallees = new HotReloadAddedCalleeIndex(entriesToPatch); List prepared = new List(context.Files.Count); foreach (HotReloadGroupFile file in context.Files) { prepared.Add( - PrepareFile(compileResult, homeResolver, file, entriesByFile, bindFailures)); + PrepareFile(compileResult, homeResolver, file, entriesByFile, bindFailures, addedCallees)); } return prepared; @@ -56,7 +59,8 @@ private static HotReloadPreparedGroupFile PrepareFile( HotReloadEntryHomeResolver homeResolver, HotReloadGroupFile file, Dictionary> entriesByFile, - Dictionary bindFailures) + Dictionary bindFailures, + HotReloadAddedCalleeIndex addedCallees) { if (file.SkipApply) { @@ -76,7 +80,8 @@ private static HotReloadPreparedGroupFile PrepareFile( file.AssemblyResolvePath, compileResult.Assembly, entries, - bindFailures); + bindFailures, + addedCallees); if (!resolution.AllResolved) { return HotReloadPreparedGroupFile.ResolutionFailed(file, entries, resolution); diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunAccumulator.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunAccumulator.cs index 133b68306..a5aa9dbbf 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunAccumulator.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunAccumulator.cs @@ -26,6 +26,9 @@ internal sealed class HotReloadRunAccumulator private readonly List _addedConsts = new List(); private readonly List _siblingDerivedWarnings = new List(); private readonly List _reappliedSiblingPaths = new List(); + private readonly List _pathsInRun = new List(); + private readonly HotReloadStaleAddedMemberCallers _staleAddedMemberCallers = + new HotReloadStaleAddedMemberCallers(); private readonly HotReloadSiblingBaselineNotices _siblingBaselineNotices = new HotReloadSiblingBaselineNotices(); // Why appended without deduplication: one row per declaration is what the report means, @@ -119,6 +122,14 @@ public void Add(string projectRelativePath, HotReloadFileProcessResult fileResul _outcomes.AddRange(fileResult.Outcomes); _warnings.AddRange(fileResult.Warnings); + // Why every merged file, short-circuited inputs and siblings included: a call left + // running into a retired added member is reported on every run that touches either end. + // Why the empty check: the run-wide path set skips an input that resolved to no path. + if (!string.IsNullOrEmpty(projectRelativePath)) + { + _pathsInRun.Add(projectRelativePath); + } + HotReloadOutcomeAggregation.AppendDistinct(_suppressedPausePointIds, fileResult.SuppressedPausePointIds); HotReloadOutcomeAggregation.AppendDistinct(_retargetedPausePointIds, fileResult.RetargetedPausePointIds); HotReloadOutcomeAggregation.AppendDistinct(_inlineRiskMethodLabels, fileResult.InlineRiskMethodLabels); @@ -232,6 +243,7 @@ public HotReloadOrchestratorResult BuildResult(string correlationId) AppendAddedFieldsLifetimeWarning(); AppendSerializedAddedFieldWarning(); AppendUnforwardedUnityMessageWarning(); + AppendStaleAddedMemberCallsWarning(); // Why at the end of the run and on the main thread: the added methods this run brought // in are in the domain by now, and building a proxy type touches Unity APIs that only // answer on the main thread. A type whose proxy cannot be built reports here, so the @@ -301,6 +313,19 @@ private HotReloadCarriedInState DescribeCarriedInState(string projectRelativePat return _siblingLedgerUpdates.DescribeAfterApply(new HotReloadDomainCarriedInLookup(_domain), projectRelativePath); } + // Why after every group: a call is stale only once no generation registers its member, and + // a later group of the same run can register it again or retire it. + private void AppendStaleAddedMemberCallsWarning() + { + List calls = new List(); + _domain.CollectAddedMemberCalls(calls); + string warning = _staleAddedMemberCallers.DescribeOrNull(calls, _domain.DescribeAddedMembers(), _pathsInRun); + if (warning != null) + { + _warnings.Add(warning); + } + } + private void AppendInlineRiskWarning() { if (_inlineRiskMethodLabels.Count == 0) diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStaleAddedMemberCallers.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStaleAddedMemberCallers.cs new file mode 100644 index 000000000..809ad20d7 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStaleAddedMemberCallers.cs @@ -0,0 +1,72 @@ +using System; +using System.Collections.Generic; + +using UnityEngine; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Words the run-level warning for calls that live patches and registered added members still + /// make into added members no generation registers any more. + /// + /// + /// Why from the state at the end of the run rather than from what the run changed: such a call + /// is left behind by a caller that did not apply, in this run or an earlier one, and it keeps + /// running until the caller applies again. + /// + internal sealed class HotReloadStaleAddedMemberCallers + { + /// + /// The warning naming each call into a member does not + /// hold, whose caller's file or member's declaring file is in ; + /// null when there is none. + /// + /// + /// Why only calls with an end in this run: a reload of an unrelated file would otherwise + /// repeat the warning on every run until the call is gone. + /// + internal string DescribeOrNull( + IReadOnlyList calls, + IReadOnlyList registeredMembers, + IEnumerable pathsInRun) + { + Debug.Assert(calls != null, "calls must not be null."); + Debug.Assert(registeredMembers != null, "registeredMembers must not be null."); + Debug.Assert(pathsInRun != null, "pathsInRun must not be null."); + + HashSet registeredLabels = new HashSet(StringComparer.Ordinal); + foreach (HotReloadAddedMemberInfo member in registeredMembers) + { + registeredLabels.Add(member.MethodKey); + } + + HashSet touchedPaths = new HashSet( + pathsInRun, + HotReloadSourcePathNormalizer.ProjectRelativePathComparer()); + SortedSet staleCalls = new SortedSet(StringComparer.Ordinal); + foreach (HotReloadAddedMemberCall call in calls) + { + if (registeredLabels.Contains(call.Callee.Label)) + { + continue; + } + + if (!touchedPaths.Contains(call.CallerFilePath) && !touchedPaths.Contains(call.Callee.DeclaringFilePath)) + { + continue; + } + + staleCalls.Add(call.CallerLabel + " calls " + call.Callee.Label); + } + + if (staleCalls.Count == 0) + { + return null; + } + + return string.Format( + HotReloadConstants.StaleAddedMemberCallsWarningFormat, + string.Join(", ", staleCalls)); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStaleAddedMemberCallers.cs.meta b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStaleAddedMemberCallers.cs.meta new file mode 100644 index 000000000..9f3ebcce9 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStaleAddedMemberCallers.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: c8e4dd9d79c8440238e06bdda68e1c16 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadActivePatchEntry.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadActivePatchEntry.cs index e4285ae1f..52f13ef50 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadActivePatchEntry.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadActivePatchEntry.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Diagnostics; using System.Reflection; @@ -7,8 +8,9 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { /// /// One method's hot-reload patch as the owning file generation holds it: the shim whose call - /// replaced the body, whether Harmony has accepted the patch yet, the transplant LocalBuilders - /// in shim slot order, and how many instructions the latest rebuild prepended. + /// replaced the body, the added members that body calls, whether Harmony has accepted the patch + /// yet, the transplant LocalBuilders in shim slot order, and how many instructions the latest + /// rebuild prepended. /// /// /// Why the transplant state is written before the patch is committed: the transpiler runs @@ -19,19 +21,29 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// internal sealed class HotReloadActivePatchEntry { - internal HotReloadActivePatchEntry(MethodBase method, MethodInfo shim) + internal HotReloadActivePatchEntry( + MethodBase method, + MethodInfo shim, + IReadOnlyList calledAddedMembers) { Debug.Assert(method != null, "method must not be null."); Debug.Assert(shim != null, "shim must not be null."); Method = method; Shim = shim; + CalledAddedMembers = calledAddedMembers ?? Array.Empty(); } internal MethodBase Method { get; } internal MethodInfo Shim { get; } + /// + /// The added members the shim's body calls, fixed when the patch opens: the body is compiled + /// by then, and a later reload that changes the calls opens a new patch. + /// + internal IReadOnlyList CalledAddedMembers { get; } + /// False while Harmony has not yet accepted the patch this entry describes. internal bool IsActive { get; private set; } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadAddedMemberCall.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadAddedMemberCall.cs new file mode 100644 index 000000000..66700fffa --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadAddedMemberCall.cs @@ -0,0 +1,36 @@ +using System; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// One call a live patch or a registered added member makes into an added member: who calls, + /// from which file, and the member it was applied against. + /// + internal sealed class HotReloadAddedMemberCall + { + internal HotReloadAddedMemberCall(string callerLabel, string callerFilePath, HotReloadCalledAddedMember callee) + { + if (string.IsNullOrEmpty(callerLabel)) + { + throw new ArgumentException("A call is reported with its caller's label.", nameof(callerLabel)); + } + + if (string.IsNullOrEmpty(callerFilePath)) + { + throw new ArgumentException( + "A call is reported with the project-relative path of its caller's file.", + nameof(callerFilePath)); + } + + CallerLabel = callerLabel; + CallerFilePath = callerFilePath; + Callee = callee ?? throw new ArgumentNullException(nameof(callee)); + } + + internal string CallerLabel { get; } + + internal string CallerFilePath { get; } + + internal HotReloadCalledAddedMember Callee { get; } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadAddedMemberCall.cs.meta b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadAddedMemberCall.cs.meta new file mode 100644 index 000000000..c82dad6d2 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadAddedMemberCall.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: e8deda464d8c946d7ae71d98c0196317 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadAddedMemberInfo.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadAddedMemberInfo.cs index 2ee312480..737c46c44 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadAddedMemberInfo.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadAddedMemberInfo.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Reflection; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -38,6 +39,9 @@ internal sealed class HotReloadAddedMemberInfo /// public FieldInfo InvocationCounter { get; } + /// The other added members this member's body calls; empty when it calls none. + public IReadOnlyList CalledAddedMembers { get; } + public HotReloadAddedMemberInfo( string methodKey, string filePath, @@ -46,7 +50,8 @@ public HotReloadAddedMemberInfo( int sourceEndLine = 0, string methodName = null, string declaringTypeMetadataName = null, - FieldInfo invocationCounter = null) + FieldInfo invocationCounter = null, + IReadOnlyList calledAddedMembers = null) { MethodKey = methodKey ?? string.Empty; FilePath = filePath ?? string.Empty; @@ -56,6 +61,7 @@ public HotReloadAddedMemberInfo( MethodName = methodName ?? string.Empty; DeclaringTypeMetadataName = declaringTypeMetadataName ?? string.Empty; InvocationCounter = invocationCounter; + CalledAddedMembers = calledAddedMembers ?? Array.Empty(); } /// diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadCalledAddedMember.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadCalledAddedMember.cs new file mode 100644 index 000000000..afd129a46 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadCalledAddedMember.cs @@ -0,0 +1,39 @@ +using System; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// One added member a reloaded body calls, as the reload that applied the body resolved it: + /// the label the added-member ledger keys it by, and the project-relative path of the file that + /// declared it then. + /// + /// + /// Why the declaring file is kept: once a later reload retires the member, the ledger no longer + /// knows where it came from, and a run that touches that file is one that has to say the call + /// was left behind. + /// + internal sealed class HotReloadCalledAddedMember + { + internal HotReloadCalledAddedMember(string label, string declaringFilePath) + { + if (string.IsNullOrEmpty(label)) + { + throw new ArgumentException("A called added member is recorded with its ledger label.", nameof(label)); + } + + if (string.IsNullOrEmpty(declaringFilePath)) + { + throw new ArgumentException( + "A called added member is recorded with the project-relative path of its declaring file.", + nameof(declaringFilePath)); + } + + Label = label; + DeclaringFilePath = declaringFilePath; + } + + internal string Label { get; } + + internal string DeclaringFilePath { get; } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadCalledAddedMember.cs.meta b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadCalledAddedMember.cs.meta new file mode 100644 index 000000000..6a0726d0b --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadCalledAddedMember.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: d157eec1d91634784a3de31f6393d639 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadDomain.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadDomain.cs index 40399db34..da4bbd260 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadDomain.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadDomain.cs @@ -385,6 +385,19 @@ internal IReadOnlyList DescribeActivePatches() return patches; } + /// + /// Adds each call a live patch or a registered added member of any file makes into an added + /// member, in no particular order. + /// + internal void CollectAddedMemberCalls(List calls) + { + Debug.Assert(calls != null, "calls must not be null."); + foreach (KeyValuePair pair in _generationsByPath) + { + pair.Value.CollectAddedMemberCalls(calls); + } + } + /// Active added members of one file, in no particular order. internal IReadOnlyList DescribeAddedMembersOfFile(string projectRelativePath) { diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadFileGeneration.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadFileGeneration.cs index 4d4cc3c47..5092c06c5 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadFileGeneration.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadFileGeneration.cs @@ -140,7 +140,8 @@ internal void RegisterAddedMethod( string declaringTypeMetadataName, FieldInfo invocationCounter, int sourceStartLine = 0, - int sourceEndLine = 0) + int sourceEndLine = 0, + IReadOnlyList calledAddedMembers = null) { Debug.Assert(!string.IsNullOrEmpty(methodKey), "methodKey must not be empty."); Debug.Assert(shimMethod != null, "shimMethod must not be null."); @@ -174,7 +175,8 @@ internal void RegisterAddedMethod( sourceEndLine, methodName, declaringTypeMetadataName, - invocationCounter); + invocationCounter, + calledAddedMembers); } /// @@ -244,10 +246,14 @@ internal bool TryGetAddedFieldDeclaration( /// /// Opens a patch of with , pending until - /// Harmony accepts it. The shim has to be registered first: that is what makes every patch - /// this generation holds a patch of a method the edited source still declares. + /// Harmony accepts it, calling . The shim has to be + /// registered first: that is what makes every patch this generation holds a patch of a + /// method the edited source still declares. /// - internal void BeginPatch(MethodBase method, MethodInfo shim) + internal void BeginPatch( + MethodBase method, + MethodInfo shim, + IReadOnlyList calledAddedMembers = null) { Debug.Assert(method != null, "method must not be null."); Debug.Assert(shim != null, "shim must not be null."); @@ -263,7 +269,7 @@ internal void BeginPatch(MethodBase method, MethodInfo shim) "This method already holds a pending or active patch."); } - _patchesByMethod[method] = new HotReloadActivePatchEntry(method, shim); + _patchesByMethod[method] = new HotReloadActivePatchEntry(method, shim, calledAddedMembers); } /// Turns this method's pending patch into the live one, keeping what the rebuild recorded. @@ -499,6 +505,43 @@ internal HotReloadAddedMethodAtLine FindAddedMethodContainingLine(int line) return null; } + /// + /// Adds each call this generation's live patches and registered added members make into an + /// added member, named by the caller's label and this generation's path. + /// + /// + /// Why a pending patch is left out: Harmony has not accepted it, so nothing runs its calls. + /// + internal void CollectAddedMemberCalls(List calls) + { + Debug.Assert(calls != null, "calls must not be null."); + foreach (KeyValuePair pair in _patchesByMethod) + { + if (!pair.Value.IsActive) + { + continue; + } + + AddCalls(HotReloadMethodKeys.FormatMethodLabel(pair.Key), pair.Value.CalledAddedMembers, calls); + } + + foreach (KeyValuePair pair in _addedMembersByMethodKey) + { + AddCalls(pair.Key, pair.Value.CalledAddedMembers, calls); + } + } + + private void AddCalls( + string callerLabel, + IReadOnlyList callees, + List calls) + { + foreach (HotReloadCalledAddedMember callee in callees) + { + calls.Add(new HotReloadAddedMemberCall(callerLabel, Path, callee)); + } + } + internal void DescribeAddedMembers(List members) { Debug.Assert(members != null, "members must not be null."); diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadPatcher.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadPatcher.cs index c1b21f237..fd3c25011 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadPatcher.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadPatcher.cs @@ -47,7 +47,8 @@ internal HotReloadPatcher(HotReloadDomain domain, IHotReloadHarmony harmony) /// /// Patches with using - /// . Re-applying the same method Unpatches the previous + /// , recording as the + /// added members the shim's body calls. Re-applying the same method Unpatches the previous /// transpiler first so patches do not stack. /// Engine failures during apply never throw; they are contained as an /// result for that method only. @@ -56,7 +57,8 @@ public HotReloadPatchResult Apply( MethodBase method, MethodInfo shimMethodInfo, HotReloadPatchShape patchShape, - string filePath) + string filePath, + IReadOnlyList calledAddedMembers = null) { if (method == null) { @@ -113,7 +115,7 @@ public HotReloadPatchResult Apply( // The patch is committed only after Patch succeeds. During Patch the transpiler reads // the pending entry because Harmony resolves transpilers statically (no MethodInfo arg). - generation.BeginPatch(method, shimMethodInfo); + generation.BeginPatch(method, shimMethodInfo, calledAddedMembers); try { // Why Priority.First: same numeric priority sorts by registration index, and diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadConstants.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadConstants.cs index b46c7ddf4..de4ce7785 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadConstants.cs @@ -239,6 +239,16 @@ internal static class HotReloadConstants + "The next reload of this assembly retries their file once while it stays unchanged, so " + "change what their Methods[].Reason names and reload, or run 'uloop compile'."; + // Format: each stale call as "caller calls member", joined with ", " in ordinal order. + // Why it names the pair: the member is gone from the ledger and from --status, and the + // caller that did not apply may have done so in an earlier run, so nothing else in the + // response ties the body that keeps running to the call that still reaches it. + public const string StaleAddedMemberCallsWarningFormat = + "Methods that earlier hot reloads patched or added still call added members that are no " + + "longer registered: {0}. Those calls still run the members' earlier bodies, which match " + + "neither the compiled assembly nor the source on disk. Reload until the calling methods " + + "apply again, or run 'uloop compile'."; + // Wire value for TransformWorkerRemovedMemberDto.kind. // Keep in sync with RemovedMemberKinds in TransformWorker~/RemovedMemberKinds.cs. public const string RemovedMemberKindMethod = "method"; diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md index 6185e28a8..b91e2d5b8 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` and `Added` rows of `--status` and on `AlreadyActive` and `Stale` apply rows (calls into the patched or added body since it was applied); it is `0` on other apply/revert outcomes, including the `Added` rows of the run that applied them. 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). On `--status`, an `Added` row counts calls into the added member's body since it was applied; while that count is 0, its `Reason` explains that compiled code cannot call an added member, so only a hot-reloaded body that calls it, or the hot-reload proxy delivering a forwarded Unity message in Play Mode, can run it. An added iterator counts when its enumeration starts, whereas a patched iterator, and an async method of either kind, counts when it is called. An `AlreadyActive` row for an added member carries that member's count. `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. `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": "", "FilePath": "Assets/Scripts/Host.cs", "InvocationCount": 3, "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, 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. +- `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 patch or added member that an earlier reload applied still calls an added member that is no longer registered — a later reload changed its signature, deleted it, or skipped it while the caller did not apply again — Warnings includes `Methods that earlier hot reloads patched or added still call added members that are no longer registered: calls , .... Those calls still run the members' earlier bodies, which match neither the compiled assembly nor the source on disk. Reload until the calling methods apply again, or run 'uloop compile'.` on every reload that includes the caller's file or the member's file; see [troubleshooting.md](troubleshooting.md). 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 903a9997f..be2d4032e 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 @@ -213,6 +213,9 @@ method: a copy the JIT inlined into another method before the patch, and a deleg the old method the caller created before it. A call inside a lambda or local function stays listed under its compiler-generated name even when the method declaring it is patched. +An added member that a later reload re-signatures or deletes has no compiled callers, but a +hot-reloaded caller that does not apply again in that reload keeps calling the member's +earlier body; `Warnings` then names that call (see [troubleshooting.md](troubleshooting.md)). Field declarations are stricter: when a compiled field's type — or its `static`/ `const` modifier — differs from the edited source, every edited method that reads diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/troubleshooting.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/troubleshooting.md index fb919684c..7909cf1e8 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/troubleshooting.md +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/troubleshooting.md @@ -75,3 +75,18 @@ the hits with `uloop pause-point-status`: the failing statement sits at or after marked line that records a hit, and before the first one that records none. A line inside an added method cannot hold a pause point until `uloop compile`, so mark the line that calls it instead. + +## When Earlier Patches Still Call an Added Member That Is Gone + +A patched or added body keeps calling the added members it was applied against. When a later +reload changes such a member's signature, deletes it, or reports it `Skipped`, the member is +no longer registered and `--status` stops listing it, but a caller that did not apply again +in that reload — its row is `Failed` or `Skipped`, or its file was not re-applied — still +runs the member's earlier body, which matches neither the compiled assembly nor the source +on disk. `Warnings` then carries one line naming each such call as ` calls `, +with `` in the signature the caller was applied against. + +The line comes back on every reload that includes the caller's file or the member's file, +and stops once the caller applies again: fix what the caller's row reported and reload its +file together with the member's file, or run `uloop compile`. A reload that includes neither +file does not repeat it.