diff --git a/.agents/skills/uloop-hot-reload/SKILL.md b/.agents/skills/uloop-hot-reload/SKILL.md index 5a341c1fb..1cc6aa834 100644 --- a/.agents/skills/uloop-hot-reload/SKILL.md +++ b/.agents/skills/uloop-hot-reload/SKILL.md @@ -44,8 +44,8 @@ automatically — pass it with `--files`. `uloop hot-reload --status` lists the currently active changes; it cannot be combined with `--files` or `--revert-all`. Every change is static Editor state, so after a domain reload -it reports zero. Each `Active` row's `InvocationCount` counts calls -into the patched body since it was applied — a reachability signal only while the code is +it reports zero. Each `Active`/`Added` row's `InvocationCount` counts calls +into its body since it was applied — a reachability signal only while the code is being driven. ## How It Works diff --git a/.agents/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md b/.agents/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md index e8e2978e8..60d403d48 100644 --- a/.agents/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md +++ b/.agents/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md @@ -10,8 +10,8 @@ Re-running on the same method after a real edit replaces its previous patch; `ActivePatchTotal` tracks the ledger across runs. Reloading a file whose source is unchanged since the last fully applied reload (a run with no Skipped or Failed -outcomes) is a no-op: each still-active method is reported -as `AlreadyActive`, the existing patch stays in place, and the row carries the live `InvocationCount`. +outcomes) is a no-op: each still-active method or added member is reported +as `AlreadyActive`, the existing patch or added member stays in place, and the row carries its live `InvocationCount`. When another edited file of the same assembly is in the reload, that unchanged file is re-applied with the group instead, and other files of the assembly that hold active patches and are unchanged since they were applied are re-applied too, so every active diff --git a/.agents/skills/uloop-hot-reload/references/output.md b/.agents/skills/uloop-hot-reload/references/output.md index 72a3dec2f..6185e28a8 100644 --- a/.agents/skills/uloop-hot-reload/references/output.md +++ b/.agents/skills/uloop-hot-reload/references/output.md @@ -5,7 +5,7 @@ Returns JSON with: - `Success` (boolean): `false` on parameter validation failure or when any method outcome is `Failed`, or when any `IntroducedTypes` row is `Failed`. `Skipped` outcomes alone never force `false` - `ErrorCode` (string, optional): Present on parameter validation failure. Values are `HOT_RELOAD_FILES_REQUIRED` when an omitted apply has no compile snapshots, `HOT_RELOAD_NO_CHANGED_FILES` when snapshots contain no changed `.cs` files, `HOT_RELOAD_INVALID_FILES` when `--files` contains a null or empty path, and `HOT_RELOAD_STATUS_CONFLICT` when `--status` is combined with `--files` or `--revert-all`. - `NextActions` (array, optional): Ordered recovery steps, present only with `ErrorCode` on a parameter validation failure. Omitted from every other response, including successful apply, plain `--status`, and `--revert-all` runs. -- `Methods` (array): Per-method `{ Kind, Method, Reason, FilePath, InvocationCount, LifecycleNote, ReappliedFromSibling }` where `Kind` is `Patched`, `Skipped`, `Failed`, `Added`, `AlreadyActive`, or `Stale` on apply runs, and `Active`, `Added`, or `AddedField` on `--status` runs; empty on `--revert-all` runs. `AlreadyActive` means this file's source matched the last fully applied reload (a run with no Skipped or Failed outcomes), so the existing patch was left in place and the row carries the live `InvocationCount`. `Stale` means the method was deleted from the edited source while its patch is still installed: compiled callers keep running the patched body until `uloop compile`, `--revert-all`, or a later reload whose source restores the method to the compiled baseline clears it; a reload that declares the method with a different body replaces the patch instead of clearing it. Stale rows keep counting toward `ActivePatchTotal`, and the Message summary includes `Stale=N`. `InvocationCount` is meaningful on `Active` rows and on `AlreadyActive` and `Stale` apply rows (calls since the current patch was applied); it is `0` on other apply/revert outcomes. On `--status`, an `Active` row with `InvocationCount` 0 sets `Reason` to explain that the method has not run since the patch: finished calls do not re-run, the patched body takes effect on the next call, and how to retrigger an initialization-only path. When the edited source later declares a different signature, that `Active` row's `Reason` instead explains it is superseded by a new declaration of that signature and is no longer the entry point for new calls (superseded wins over the never-invoked sentence). Added-member rows always show InvocationCount 0 — added-member calls are not instrumented, and the row's Reason says so. `AddedField` rows list a live added field as `Type.field` with an empty `Reason`; they are not method patches. `LifecycleNote` is set when a patched method is a Unity one-shot lifecycle message (`private void Awake`/`Start`/`OnEnable`/`OnDisable`/`OnDestroy` on a `MonoBehaviour`), or when every compiled call path into the patched method (callers of callers are followed a few levels within the compiled assemblies) starts at such a message; empty otherwise — it does not change `Kind`. On an `Added` row whose method is a Unity message, `LifecycleNote` instead says whether the engine will reach it: a forwarded message (`Start`, `Update`, the collision/trigger/mouse messages, and the rest listed in [scope-and-limits.md](scope-and-limits.md)) carries the note that a hot-reload proxy component delivers it to live instances while Play Mode runs, that the proxy is rebuilt only when a later reload changes which messages the type adds or their signatures, that execution order relative to other components is not guaranteed, and that it is gone on any compile or domain reload (only an added `Start` row also says that it runs once on each existing instance when the proxy attaches, and again when the proxy is rebuilt); a message this feature leaves to the compiler (`Awake`, `OnEnable`, `OnDisable`, `OnDestroy`, the editor-only messages, and any non-void message) carries the note that the engine does not invoke it until `uloop compile`, and the run adds one `Warnings` line naming every such message together. `Added` rows carry the added member's signature and file; their `InvocationCount` is always `0` (added-member calls are not instrumented). `ReappliedFromSibling` is `true` on every apply row, whatever its `Kind`, that belongs to a sibling file the run pulled in to re-apply changes from earlier reloads rather than to a file passed in `--files`; it is `false` on the other rows and on every `--status` and `--revert-all` row. Message's re-applied count covers only the `Patched` and `Added` rows among them. `Method` spells parameter types as .NET metadata does, the same on every method row: a constructed generic as ``System.Collections.Generic.List`1``, a multidimensional array as `System.Int32[0...,0...]`, and a nested type with `+`. Example `--status` row: `{ "Kind": "Added", "Method": "Ns.Host.NewHelper(System.Int32)", "Reason": "Added-member calls are not instrumented, so InvocationCount is always 0 for this row.", "FilePath": "Assets/Scripts/Host.cs", "InvocationCount": 0, "LifecycleNote": "", "ReappliedFromSibling": false }` +- `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. - `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. @@ -20,7 +20,7 @@ Returns JSON with: - `ClearedCount` (number): Patches removed by `--revert-all`, or stale patches reverted because their source matched the compiled baseline again - `IntroducedTypes` (array): Per-type `{ Kind, TypeName, AssemblyName, FilePath, Reason }` rows for the type declarations a reload met, always present and empty when there are none. On apply runs `Kind` is `Introduced` (this reload compiled the declaration into a retained assembly and it is now loaded), `AlreadyActive` (the declaration is bound from an assembly an earlier reload retained, so this reload introduced nothing for it), or `Failed` (the declaration was refused — a redefinition of a type already active, the same type declared in more than one file of the group, or a failed artifact compilation; `Reason` says which, and a `Failed` row alone makes `Success` false). On `--status` every row is `Kind` `Active` and lists a type this domain still holds. Declarations a reload simply cannot introduce are reported as `Warnings`, not rows. These rows are never counted in `PatchedTotal`, `ActivePatchTotal`, `AddedFieldTotal`, or `ClearedCount` - `ActiveIntroducedTypeTotal` (number): Introduced types this domain holds after this run or on `--status`, counted per type rather than per compiled artifact; always present and `0` when there is none. `--revert-all` cannot unload them, so its Message says how many stay loaded until the next Domain Reload, and that Auto Refresh stays held for them until `uloop compile` -- `Message` (string): Short summary. When a run carries `IntroducedTypes` rows, Message reports them: a run that only introduced or only re-bound types says so instead of reporting the methods, a refused declaration is reported as the failure of the run and points at `IntroducedTypes`, and a run the methods decided ends with `IntroducedTypes=N`. On apply runs that pulled in sibling files, Message follows `PatchedTotal` and `Added` with how many of those Patched and Added rows re-applied the siblings' earlier changes (left out when 0). On apply runs, Message counts the patched rows that carry a `LifecycleNote` in one sentence and the added Unity messages a hot-reload proxy delivers in another, both pointing at `Methods[].LifecycleNote`; forwarded `Added` rows are not in the patched count, and each sentence is left out when its count is 0. On `--status`, Message opens with how many changes are currently active — patched methods, added members, and introduced types together, which is why it can exceed `ActivePatchTotal` — and when any `Active` row has `InvocationCount` 0 it also appends how many such rows there are and points at `Methods[].Reason`; added-member rows are not included in that count. `--revert-all` appends how many introduced types stay loaded until the next Domain Reload, and when the hold is still armed for them, that Auto Refresh stays held until `uloop compile` +- `Message` (string): Short summary. When a run carries `IntroducedTypes` rows, Message reports them: a run that only introduced or only re-bound types says so instead of reporting the methods, a refused declaration is reported as the failure of the run and points at `IntroducedTypes`, and a run the methods decided ends with `IntroducedTypes=N`. On apply runs that pulled in sibling files, Message follows `PatchedTotal` and `Added` with how many of those Patched and Added rows re-applied the siblings' earlier changes (left out when 0). On apply runs, Message counts the patched rows that carry a `LifecycleNote` in one sentence and the added Unity messages a hot-reload proxy delivers in another, both pointing at `Methods[].LifecycleNote`; forwarded `Added` rows are not in the patched count, and each sentence is left out when its count is 0. On `--status`, Message opens with how many changes are currently active — patched methods, added members, and introduced types together, which is why it can exceed `ActivePatchTotal` — and when any `Active` or `Added` row has `InvocationCount` 0 it also appends how many such rows there are and points at `Methods[].Reason`. `--revert-all` appends how many introduced types stay loaded until the next Domain Reload, and when the hold is still armed for them, that Auto Refresh stays held until `uloop compile` - `RecommendedNextAction` (string): Present in three cases. (1) Any method or introduced-type outcome is `Failed`: a partial apply (some methods patched or added, or some types left active) says to fix and rerun, run `uloop compile`, or `uloop hot-reload --revert-all`; a failure with nothing applied says to fix and rerun or compile. (2) Every method of the requested files was `Skipped`, which still answers `Success`: it points first at the fix each Skipped row's `Methods[].Reason` names and offers `uloop compile` as the alternative. (3) `CompileFallback` is `HeldForPlayMode` or `BlockedByPlayModeSetting`, whatever the outcomes: the reason no compile ran is appended after any advice from (1) or (2), and it opens by saying to do any fix a `Reason` names that needs no compile before compiling. Omitted otherwise. - `CompileFallback` (string, always present): whether the CLI should run a compile after this run — `NotNeeded`, `Requested`, `HeldForPlayMode` (edits stayed unapplied but the Editor is in Play Mode and `--compile-on-skip` is `auto`), `BlockedByPlayModeSetting` (`--compile-on-skip on` during Play Mode while Unity's "Script Changes While Playing" is "Recompile After Finished Playing", which refuses the compile; `RecommendedNextAction` says to stop Play Mode first), or `Disabled` (`--compile-on-skip off`). `--status`, `--revert-all` and validation failures answer `NotNeeded`. `Skipped` rows of a sibling pulled in to re-bind its active patches do not count as unapplied edits: they are not this run's edits, and their earlier patches stay active. Its `Failed` rows do count, because a failed reload reverts those patches. A sibling retried after an earlier Skip, or brought in as a companion, still counts. - `Compile` (object, present only when the CLI ran the fallback compile): the full `uloop compile` response; the top-level `Success` is then the compile's, and the command's exit code is the compile's. A successful compile drops `RecommendedNextAction` and ends `Message` with a sentence saying the compile succeeded; a failed one sets `RecommendedNextAction` to the compile's own `NextActions` when it reports any, and otherwise to fixing `Compile.Errors`. diff --git a/.agents/skills/uloop-hot-reload/references/troubleshooting.md b/.agents/skills/uloop-hot-reload/references/troubleshooting.md index 512f2b676..fb919684c 100644 --- a/.agents/skills/uloop-hot-reload/references/troubleshooting.md +++ b/.agents/skills/uloop-hot-reload/references/troubleshooting.md @@ -2,9 +2,9 @@ ## Reading `--status` and `InvocationCount` -`uloop hot-reload --status` lists the methods whose bodies are currently replaced, -without applying or reverting anything. It cannot be combined with `--files` or -`--revert-all`. Patches are static Editor state, so the answer is authoritative: after +`uloop hot-reload --status` lists the methods whose bodies are currently replaced and the +members hot reload added, without applying or reverting anything. It cannot be combined +with `--files` or `--revert-all`. Patches are static Editor state, so the answer is authoritative: after a domain reload it reports zero patched methods, which is exactly when an `ActivePatchTotal` remembered from an earlier response has gone stale. @@ -12,9 +12,19 @@ Each `Active` row's `InvocationCount` counts calls into the patched body since t was applied. Reloading the same source with no edits after a fully applied reload (a run with no Skipped or Failed outcomes) reports `AlreadyActive` and the row carries the live `InvocationCount`, unless another edited file of the same assembly is in the -reload — then the unchanged file is re-applied with that group; re-running after a real edit replaces the patch and resets it to zero. When `InvocationCount` is 0 on an `Active` row, `Reason` notes that the method has not run since this patch was applied: calls that already finished do not re-run, and the patched body takes effect the next time this method is called. For initialization-only methods it also names how to trigger that next call. While Unity is -paused — including while a pause-point hit holds the game — the player loop does not -advance, so game-driven calls stop and the count freezes; calls you make yourself (for +reload — then the unchanged file is re-applied with that group; re-running after a real edit replaces the patch and resets it to zero. When `InvocationCount` is 0 on an `Active` row, `Reason` notes that the method has not run since this patch was applied: calls that already finished do not re-run, and the patched body takes effect the next time this method is called. For initialization-only methods it also names how to trigger that next call. + +Each `Added` row's `InvocationCount` counts calls into the added member's body the same +way, from a counter of the member's own: an unchanged reload's `AlreadyActive` row for it +carries that count, and re-applying the member — by editing it, or by re-applying its file +with an edited file of the same assembly — starts the count over at zero. Compiled code +cannot call an added member, so the count stays 0 until a hot-reloaded body that calls it +runs, or, for a forwarded Unity message, until the hot-reload proxy delivers the message in +Play Mode; `Reason` says so while the count is 0. An added iterator counts when its +enumeration starts rather than when it is called. + +While Unity is paused — including while a pause-point hit holds the game — the player loop +does not advance, so game-driven calls stop and the count freezes; calls you make yourself (for example through `uloop execute-dynamic-code`) still increment it. A frozen count during a pause only means game-driven calls are not running; it says nothing about whether call sites reach the patch. Resume first diff --git a/.claude/skills/uloop-hot-reload/SKILL.md b/.claude/skills/uloop-hot-reload/SKILL.md index 5a341c1fb..1cc6aa834 100644 --- a/.claude/skills/uloop-hot-reload/SKILL.md +++ b/.claude/skills/uloop-hot-reload/SKILL.md @@ -44,8 +44,8 @@ automatically — pass it with `--files`. `uloop hot-reload --status` lists the currently active changes; it cannot be combined with `--files` or `--revert-all`. Every change is static Editor state, so after a domain reload -it reports zero. Each `Active` row's `InvocationCount` counts calls -into the patched body since it was applied — a reachability signal only while the code is +it reports zero. Each `Active`/`Added` row's `InvocationCount` counts calls +into its body since it was applied — a reachability signal only while the code is being driven. ## How It Works diff --git a/.claude/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md b/.claude/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md index e8e2978e8..60d403d48 100644 --- a/.claude/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md +++ b/.claude/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md @@ -10,8 +10,8 @@ Re-running on the same method after a real edit replaces its previous patch; `ActivePatchTotal` tracks the ledger across runs. Reloading a file whose source is unchanged since the last fully applied reload (a run with no Skipped or Failed -outcomes) is a no-op: each still-active method is reported -as `AlreadyActive`, the existing patch stays in place, and the row carries the live `InvocationCount`. +outcomes) is a no-op: each still-active method or added member is reported +as `AlreadyActive`, the existing patch or added member stays in place, and the row carries its live `InvocationCount`. When another edited file of the same assembly is in the reload, that unchanged file is re-applied with the group instead, and other files of the assembly that hold active patches and are unchanged since they were applied are re-applied too, so every active diff --git a/.claude/skills/uloop-hot-reload/references/output.md b/.claude/skills/uloop-hot-reload/references/output.md index 72a3dec2f..6185e28a8 100644 --- a/.claude/skills/uloop-hot-reload/references/output.md +++ b/.claude/skills/uloop-hot-reload/references/output.md @@ -5,7 +5,7 @@ Returns JSON with: - `Success` (boolean): `false` on parameter validation failure or when any method outcome is `Failed`, or when any `IntroducedTypes` row is `Failed`. `Skipped` outcomes alone never force `false` - `ErrorCode` (string, optional): Present on parameter validation failure. Values are `HOT_RELOAD_FILES_REQUIRED` when an omitted apply has no compile snapshots, `HOT_RELOAD_NO_CHANGED_FILES` when snapshots contain no changed `.cs` files, `HOT_RELOAD_INVALID_FILES` when `--files` contains a null or empty path, and `HOT_RELOAD_STATUS_CONFLICT` when `--status` is combined with `--files` or `--revert-all`. - `NextActions` (array, optional): Ordered recovery steps, present only with `ErrorCode` on a parameter validation failure. Omitted from every other response, including successful apply, plain `--status`, and `--revert-all` runs. -- `Methods` (array): Per-method `{ Kind, Method, Reason, FilePath, InvocationCount, LifecycleNote, ReappliedFromSibling }` where `Kind` is `Patched`, `Skipped`, `Failed`, `Added`, `AlreadyActive`, or `Stale` on apply runs, and `Active`, `Added`, or `AddedField` on `--status` runs; empty on `--revert-all` runs. `AlreadyActive` means this file's source matched the last fully applied reload (a run with no Skipped or Failed outcomes), so the existing patch was left in place and the row carries the live `InvocationCount`. `Stale` means the method was deleted from the edited source while its patch is still installed: compiled callers keep running the patched body until `uloop compile`, `--revert-all`, or a later reload whose source restores the method to the compiled baseline clears it; a reload that declares the method with a different body replaces the patch instead of clearing it. Stale rows keep counting toward `ActivePatchTotal`, and the Message summary includes `Stale=N`. `InvocationCount` is meaningful on `Active` rows and on `AlreadyActive` and `Stale` apply rows (calls since the current patch was applied); it is `0` on other apply/revert outcomes. On `--status`, an `Active` row with `InvocationCount` 0 sets `Reason` to explain that the method has not run since the patch: finished calls do not re-run, the patched body takes effect on the next call, and how to retrigger an initialization-only path. When the edited source later declares a different signature, that `Active` row's `Reason` instead explains it is superseded by a new declaration of that signature and is no longer the entry point for new calls (superseded wins over the never-invoked sentence). Added-member rows always show InvocationCount 0 — added-member calls are not instrumented, and the row's Reason says so. `AddedField` rows list a live added field as `Type.field` with an empty `Reason`; they are not method patches. `LifecycleNote` is set when a patched method is a Unity one-shot lifecycle message (`private void Awake`/`Start`/`OnEnable`/`OnDisable`/`OnDestroy` on a `MonoBehaviour`), or when every compiled call path into the patched method (callers of callers are followed a few levels within the compiled assemblies) starts at such a message; empty otherwise — it does not change `Kind`. On an `Added` row whose method is a Unity message, `LifecycleNote` instead says whether the engine will reach it: a forwarded message (`Start`, `Update`, the collision/trigger/mouse messages, and the rest listed in [scope-and-limits.md](scope-and-limits.md)) carries the note that a hot-reload proxy component delivers it to live instances while Play Mode runs, that the proxy is rebuilt only when a later reload changes which messages the type adds or their signatures, that execution order relative to other components is not guaranteed, and that it is gone on any compile or domain reload (only an added `Start` row also says that it runs once on each existing instance when the proxy attaches, and again when the proxy is rebuilt); a message this feature leaves to the compiler (`Awake`, `OnEnable`, `OnDisable`, `OnDestroy`, the editor-only messages, and any non-void message) carries the note that the engine does not invoke it until `uloop compile`, and the run adds one `Warnings` line naming every such message together. `Added` rows carry the added member's signature and file; their `InvocationCount` is always `0` (added-member calls are not instrumented). `ReappliedFromSibling` is `true` on every apply row, whatever its `Kind`, that belongs to a sibling file the run pulled in to re-apply changes from earlier reloads rather than to a file passed in `--files`; it is `false` on the other rows and on every `--status` and `--revert-all` row. Message's re-applied count covers only the `Patched` and `Added` rows among them. `Method` spells parameter types as .NET metadata does, the same on every method row: a constructed generic as ``System.Collections.Generic.List`1``, a multidimensional array as `System.Int32[0...,0...]`, and a nested type with `+`. Example `--status` row: `{ "Kind": "Added", "Method": "Ns.Host.NewHelper(System.Int32)", "Reason": "Added-member calls are not instrumented, so InvocationCount is always 0 for this row.", "FilePath": "Assets/Scripts/Host.cs", "InvocationCount": 0, "LifecycleNote": "", "ReappliedFromSibling": false }` +- `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. - `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. @@ -20,7 +20,7 @@ Returns JSON with: - `ClearedCount` (number): Patches removed by `--revert-all`, or stale patches reverted because their source matched the compiled baseline again - `IntroducedTypes` (array): Per-type `{ Kind, TypeName, AssemblyName, FilePath, Reason }` rows for the type declarations a reload met, always present and empty when there are none. On apply runs `Kind` is `Introduced` (this reload compiled the declaration into a retained assembly and it is now loaded), `AlreadyActive` (the declaration is bound from an assembly an earlier reload retained, so this reload introduced nothing for it), or `Failed` (the declaration was refused — a redefinition of a type already active, the same type declared in more than one file of the group, or a failed artifact compilation; `Reason` says which, and a `Failed` row alone makes `Success` false). On `--status` every row is `Kind` `Active` and lists a type this domain still holds. Declarations a reload simply cannot introduce are reported as `Warnings`, not rows. These rows are never counted in `PatchedTotal`, `ActivePatchTotal`, `AddedFieldTotal`, or `ClearedCount` - `ActiveIntroducedTypeTotal` (number): Introduced types this domain holds after this run or on `--status`, counted per type rather than per compiled artifact; always present and `0` when there is none. `--revert-all` cannot unload them, so its Message says how many stay loaded until the next Domain Reload, and that Auto Refresh stays held for them until `uloop compile` -- `Message` (string): Short summary. When a run carries `IntroducedTypes` rows, Message reports them: a run that only introduced or only re-bound types says so instead of reporting the methods, a refused declaration is reported as the failure of the run and points at `IntroducedTypes`, and a run the methods decided ends with `IntroducedTypes=N`. On apply runs that pulled in sibling files, Message follows `PatchedTotal` and `Added` with how many of those Patched and Added rows re-applied the siblings' earlier changes (left out when 0). On apply runs, Message counts the patched rows that carry a `LifecycleNote` in one sentence and the added Unity messages a hot-reload proxy delivers in another, both pointing at `Methods[].LifecycleNote`; forwarded `Added` rows are not in the patched count, and each sentence is left out when its count is 0. On `--status`, Message opens with how many changes are currently active — patched methods, added members, and introduced types together, which is why it can exceed `ActivePatchTotal` — and when any `Active` row has `InvocationCount` 0 it also appends how many such rows there are and points at `Methods[].Reason`; added-member rows are not included in that count. `--revert-all` appends how many introduced types stay loaded until the next Domain Reload, and when the hold is still armed for them, that Auto Refresh stays held until `uloop compile` +- `Message` (string): Short summary. When a run carries `IntroducedTypes` rows, Message reports them: a run that only introduced or only re-bound types says so instead of reporting the methods, a refused declaration is reported as the failure of the run and points at `IntroducedTypes`, and a run the methods decided ends with `IntroducedTypes=N`. On apply runs that pulled in sibling files, Message follows `PatchedTotal` and `Added` with how many of those Patched and Added rows re-applied the siblings' earlier changes (left out when 0). On apply runs, Message counts the patched rows that carry a `LifecycleNote` in one sentence and the added Unity messages a hot-reload proxy delivers in another, both pointing at `Methods[].LifecycleNote`; forwarded `Added` rows are not in the patched count, and each sentence is left out when its count is 0. On `--status`, Message opens with how many changes are currently active — patched methods, added members, and introduced types together, which is why it can exceed `ActivePatchTotal` — and when any `Active` or `Added` row has `InvocationCount` 0 it also appends how many such rows there are and points at `Methods[].Reason`. `--revert-all` appends how many introduced types stay loaded until the next Domain Reload, and when the hold is still armed for them, that Auto Refresh stays held until `uloop compile` - `RecommendedNextAction` (string): Present in three cases. (1) Any method or introduced-type outcome is `Failed`: a partial apply (some methods patched or added, or some types left active) says to fix and rerun, run `uloop compile`, or `uloop hot-reload --revert-all`; a failure with nothing applied says to fix and rerun or compile. (2) Every method of the requested files was `Skipped`, which still answers `Success`: it points first at the fix each Skipped row's `Methods[].Reason` names and offers `uloop compile` as the alternative. (3) `CompileFallback` is `HeldForPlayMode` or `BlockedByPlayModeSetting`, whatever the outcomes: the reason no compile ran is appended after any advice from (1) or (2), and it opens by saying to do any fix a `Reason` names that needs no compile before compiling. Omitted otherwise. - `CompileFallback` (string, always present): whether the CLI should run a compile after this run — `NotNeeded`, `Requested`, `HeldForPlayMode` (edits stayed unapplied but the Editor is in Play Mode and `--compile-on-skip` is `auto`), `BlockedByPlayModeSetting` (`--compile-on-skip on` during Play Mode while Unity's "Script Changes While Playing" is "Recompile After Finished Playing", which refuses the compile; `RecommendedNextAction` says to stop Play Mode first), or `Disabled` (`--compile-on-skip off`). `--status`, `--revert-all` and validation failures answer `NotNeeded`. `Skipped` rows of a sibling pulled in to re-bind its active patches do not count as unapplied edits: they are not this run's edits, and their earlier patches stay active. Its `Failed` rows do count, because a failed reload reverts those patches. A sibling retried after an earlier Skip, or brought in as a companion, still counts. - `Compile` (object, present only when the CLI ran the fallback compile): the full `uloop compile` response; the top-level `Success` is then the compile's, and the command's exit code is the compile's. A successful compile drops `RecommendedNextAction` and ends `Message` with a sentence saying the compile succeeded; a failed one sets `RecommendedNextAction` to the compile's own `NextActions` when it reports any, and otherwise to fixing `Compile.Errors`. diff --git a/.claude/skills/uloop-hot-reload/references/troubleshooting.md b/.claude/skills/uloop-hot-reload/references/troubleshooting.md index 512f2b676..fb919684c 100644 --- a/.claude/skills/uloop-hot-reload/references/troubleshooting.md +++ b/.claude/skills/uloop-hot-reload/references/troubleshooting.md @@ -2,9 +2,9 @@ ## Reading `--status` and `InvocationCount` -`uloop hot-reload --status` lists the methods whose bodies are currently replaced, -without applying or reverting anything. It cannot be combined with `--files` or -`--revert-all`. Patches are static Editor state, so the answer is authoritative: after +`uloop hot-reload --status` lists the methods whose bodies are currently replaced and the +members hot reload added, without applying or reverting anything. It cannot be combined +with `--files` or `--revert-all`. Patches are static Editor state, so the answer is authoritative: after a domain reload it reports zero patched methods, which is exactly when an `ActivePatchTotal` remembered from an earlier response has gone stale. @@ -12,9 +12,19 @@ Each `Active` row's `InvocationCount` counts calls into the patched body since t was applied. Reloading the same source with no edits after a fully applied reload (a run with no Skipped or Failed outcomes) reports `AlreadyActive` and the row carries the live `InvocationCount`, unless another edited file of the same assembly is in the -reload — then the unchanged file is re-applied with that group; re-running after a real edit replaces the patch and resets it to zero. When `InvocationCount` is 0 on an `Active` row, `Reason` notes that the method has not run since this patch was applied: calls that already finished do not re-run, and the patched body takes effect the next time this method is called. For initialization-only methods it also names how to trigger that next call. While Unity is -paused — including while a pause-point hit holds the game — the player loop does not -advance, so game-driven calls stop and the count freezes; calls you make yourself (for +reload — then the unchanged file is re-applied with that group; re-running after a real edit replaces the patch and resets it to zero. When `InvocationCount` is 0 on an `Active` row, `Reason` notes that the method has not run since this patch was applied: calls that already finished do not re-run, and the patched body takes effect the next time this method is called. For initialization-only methods it also names how to trigger that next call. + +Each `Added` row's `InvocationCount` counts calls into the added member's body the same +way, from a counter of the member's own: an unchanged reload's `AlreadyActive` row for it +carries that count, and re-applying the member — by editing it, or by re-applying its file +with an edited file of the same assembly — starts the count over at zero. Compiled code +cannot call an added member, so the count stays 0 until a hot-reloaded body that calls it +runs, or, for a forwarded Unity message, until the hot-reload proxy delivers the message in +Play Mode; `Reason` says so while the count is 0. An added iterator counts when its +enumeration starts rather than when it is called. + +While Unity is paused — including while a pause-point hit holds the game — the player loop +does not advance, so game-driven calls stop and the count freezes; calls you make yourself (for example through `uloop execute-dynamic-code`) still increment it. A frozen count during a pause only means game-driven calls are not running; it says nothing about whether call sites reach the patch. Resume first diff --git a/Assets/Tests/Editor/HotReload/HotReloadAddedMemberInfoTests.cs b/Assets/Tests/Editor/HotReload/HotReloadAddedMemberInfoTests.cs new file mode 100644 index 000000000..1ffe42ad6 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadAddedMemberInfoTests.cs @@ -0,0 +1,57 @@ +using System; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// Pure coverage for how an added member's InvocationCount is read from the counter field its + /// shim increments. + /// + public class HotReloadAddedMemberInfoTests + { + private const string MethodKey = "Fixture.Added()"; + private const string FilePath = "Assets/Fixture.cs"; + + // Stands in for the counter a generated shim declares, which the shim increments on each + // call; SetUp zeroes it so no test sees another's calls. + public static long Calls; + + [SetUp] + public void SetUp() + { + Calls = 0; + } + + /// + /// What: the count is the counter's value when it is read, not when the member was + /// registered, so calls made after the registration are included. + /// + [Test] + public void ReadInvocationCount_ReturnsTheCountersValueWhenRead() + { + HotReloadAddedMemberInfo member = new HotReloadAddedMemberInfo( + MethodKey, + FilePath, + null, + invocationCounter: typeof(HotReloadAddedMemberInfoTests).GetField(nameof(Calls))); + Calls = 3; + + Assert.That(member.ReadInvocationCount(), Is.EqualTo(3)); + } + + /// + /// What: a member built without a counter refuses to report a count, rather than reporting + /// 0, which would read as never invoked. + /// + [Test] + public void ReadInvocationCount_WithoutACounter_Throws() + { + HotReloadAddedMemberInfo member = new HotReloadAddedMemberInfo(MethodKey, FilePath, null); + + Assert.Throws(() => member.ReadInvocationCount()); + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadAddedMemberInfoTests.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadAddedMemberInfoTests.cs.meta new file mode 100644 index 000000000..4d1f162fd --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadAddedMemberInfoTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 5ab6022a61db54ba9804f7f0f1ef1c66 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadAddedMemberInvocationCountE2ETests.cs b/Assets/Tests/Editor/HotReload/HotReloadAddedMemberInvocationCountE2ETests.cs new file mode 100644 index 000000000..bb5d279e2 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadAddedMemberInvocationCountE2ETests.cs @@ -0,0 +1,416 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +using NUnit.Framework; + +using Newtonsoft.Json.Linq; + +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// End-to-end coverage of the InvocationCount a member hot reload added reports: each added + /// member counts the calls that start its shim's body on a counter of its own, so a reapply + /// starts the count over and an unchanged reload keeps it. + /// + public class HotReloadAddedMemberInvocationCountE2ETests + { + private const string HostFileName = "HotReloadCrossFileAddedMemberHost.cs"; + private const string CallerFileName = "HotReloadCrossFileAddedMemberCaller.cs"; + private const string SameFileFixtureName = "HotReloadSignatureChangeSameFileFixture.cs"; + private const string HostValueAnchor = " public int Value()"; + private const string CallerCallBodyAnchor = "return host.Value();"; + private const string AddedMethodMember = + " public int Added()\n {\n return 41;\n }\n\n"; + private const string BodiedPropertyMember = + " public int Seven\n {\n get => 7;\n set { }\n }\n\n"; + private const string AutoPropertyMember = " public int Count { get; set; }\n\n"; + private const string IteratorMember = + " public System.Collections.Generic.IEnumerable Numbers()\n" + + " {\n yield return 5;\n }\n\n"; + private const string AsyncMember = + " public async System.Threading.Tasks.Task AddedAsync()\n" + + " {\n await System.Threading.Tasks.Task.CompletedTask;\n return 5;\n }\n\n"; + private const string SameFileTargetDeclaration = + " public int Target(int value)\n {\n return value;\n }"; + + private HotReloadDomainTestScope _scope; + + [SetUp] + public void SetUp() + { + _scope = new HotReloadDomainTestScope(); + } + + [TearDown] + public void TearDown() + { + _scope.Dispose(); + } + + /// + /// What: an added member of each kind reports InvocationCount 0 with the never-invoked + /// Reason until a patched caller reaches it, then one per call with an empty Reason; a call + /// through a null receiver, which the receiver check refuses before the body starts, is not + /// counted. + /// + [TestCase("Method", AddedMethodMember, "return host.Added();", "Added(", true)] + [TestCase("Expression-bodied method", " public int AddedArrow() => 41;\n\n", "return host.AddedArrow();", "AddedArrow(", true)] + [TestCase("Static method", " public static int AddedStatic()\n {\n return 41;\n }\n\n", "return HotReloadCrossFileAddedMemberHost.AddedStatic();", "AddedStatic(", false)] + [TestCase("Static expression-bodied method", " public static int AddedStaticArrow() => 41;\n\n", "return HotReloadCrossFileAddedMemberHost.AddedStaticArrow();", "AddedStaticArrow(", false)] + [TestCase("Bodied getter", BodiedPropertyMember, "return host.Seven;", "get_Seven(", true)] + [TestCase("Bodied setter", BodiedPropertyMember, "host.Seven = 1;\n return 0;", "set_Seven(", true)] + [TestCase("Auto-property getter", AutoPropertyMember, "return host.Count;", "get_Count(", true)] + [TestCase("Auto-property setter", AutoPropertyMember, "host.Count = 3;\n return 0;", "set_Count(", true)] + public async Task Status_AddedMemberCalledThroughAPatchedCaller_CountsEachCall( + string label, + string hostMember, + string callerBody, + string memberToken, + bool hasReceiver) + { + HotReloadOrchestratorResult result = await RunPairAsync( + "InvocationCount" + label.Replace(" ", string.Empty).Replace("-", string.Empty), + InsertHostMember(hostMember), + ReplaceCallerBody(callerBody)); + AssertNoFailure(result); + HotReloadMethodResult beforeCalls = FindRow(await ExecuteStatusAsync(), "Added", memberToken); + Assert.That(beforeCalls.InvocationCount, Is.EqualTo(0L), label); + Assert.That(beforeCalls.Reason, Is.EqualTo(HotReloadConstants.AddedMemberNeverInvokedReason), label); + + HotReloadCrossFileAddedMemberCaller caller = new HotReloadCrossFileAddedMemberCaller(); + HotReloadCrossFileAddedMemberHost host = new HotReloadCrossFileAddedMemberHost(); + caller.Call(host); + caller.Call(host); + if (hasReceiver) + { + Assert.Throws(() => caller.Call(null), label); + } + + HotReloadMethodResult afterCalls = FindRow(await ExecuteStatusAsync(), "Added", memberToken); + Assert.That(afterCalls.InvocationCount, Is.EqualTo(2L), label); + Assert.That(afterCalls.Reason, Is.EqualTo(string.Empty), label); + } + + /// + /// What: an added member whose expression body is a throw expression counts each call, + /// including those that throw, because the count is taken before the body runs. + /// + [TestCase("Throw method", " public int Stub() => throw new System.InvalidOperationException();\n\n", "return host.Stub();", "Stub(")] + [TestCase("Throw getter", " public int StubValue => throw new System.InvalidOperationException();\n\n", "return host.StubValue;", "get_StubValue(")] + public async Task Status_AddedThrowExpressionMember_CountsTheCallsThatThrew( + string label, + string hostMember, + string callerBody, + string memberToken) + { + HotReloadOrchestratorResult result = await RunPairAsync( + "InvocationCount" + label.Replace(" ", string.Empty), + InsertHostMember(hostMember), + ReplaceCallerBody(callerBody)); + AssertNoFailure(result); + HotReloadCrossFileAddedMemberCaller caller = new HotReloadCrossFileAddedMemberCaller(); + HotReloadCrossFileAddedMemberHost host = new HotReloadCrossFileAddedMemberHost(); + + Assert.Throws(() => caller.Call(host), label); + Assert.Throws(() => caller.Call(host), label); + + HotReloadMethodResult row = FindRow(await ExecuteStatusAsync(), "Added", memberToken); + Assert.That(row.InvocationCount, Is.EqualTo(2L), label); + } + + /// + /// What: an added iterator counts when its enumeration starts, not when it is called, + /// because its whole body, the count included, waits for the first MoveNext; an added async + /// method counts when it is called, because its body runs synchronously up to the first + /// await that has to wait. + /// + [TestCase("Iterator called only", IteratorMember, "host.Numbers();\n return 0;", "Numbers(", 0L)] + [TestCase("Iterator enumerated", IteratorMember, "foreach (int number in host.Numbers())\n {\n return number;\n }\n\n return 0;", "Numbers(", 1L)] + [TestCase("Async called", AsyncMember, "host.AddedAsync();\n return 0;", "AddedAsync(", 1L)] + public async Task Status_AddedIteratorOrAsyncMember_CountsWhenItsBodyStarts( + string label, + string hostMember, + string callerBody, + string memberToken, + long expectedCount) + { + HotReloadOrchestratorResult result = await RunPairAsync( + "InvocationCount" + label.Replace(" ", string.Empty), + InsertHostMember(hostMember), + ReplaceCallerBody(callerBody)); + AssertNoFailure(result); + + new HotReloadCrossFileAddedMemberCaller().Call(new HotReloadCrossFileAddedMemberHost()); + + HotReloadMethodResult row = FindRow(await ExecuteStatusAsync(), "Added", memberToken); + Assert.That(row.InvocationCount, Is.EqualTo(expectedCount), label); + } + + /// + /// What: editing an added member's body and reloading replaces its shim, so its count + /// starts over from 0 with the never-invoked Reason, the way a re-patched method's does. + /// + [Test] + public async Task Status_ReappliedAddedMember_StartsCountingFromZero() + { + string callerSource = ReplaceCallerBody("return host.Added();"); + HotReloadOrchestratorResult first = await RunPairAsync( + "InvocationCountReappliedFirst", + InsertHostMember(AddedMethodMember), + callerSource); + AssertNoFailure(first); + Assert.That( + new HotReloadCrossFileAddedMemberCaller().Call(new HotReloadCrossFileAddedMemberHost()), + Is.EqualTo(41)); + Assert.That( + FindRow(await ExecuteStatusAsync(), "Added", "Added(").InvocationCount, + Is.EqualTo(1L), + "Precondition: the first shim counted the call."); + + HotReloadOrchestratorResult second = await RunPairAsync( + "InvocationCountReappliedSecond", + InsertHostMember(AddedMethodMember.Replace("return 41;", "return 42;")), + callerSource); + AssertNoFailure(second); + AssertHasAdded(second, "Added("); + + HotReloadMethodResult row = FindRow(await ExecuteStatusAsync(), "Added", "Added("); + Assert.That(row.InvocationCount, Is.EqualTo(0L)); + Assert.That(row.Reason, Is.EqualTo(HotReloadConstants.AddedMemberNeverInvokedReason)); + } + + /// + /// What: reloading only the caller re-applies the host file as a sibling, which replaces + /// the added member's shim with the rest of the host, so its count starts over from 0 even + /// though the host's source did not change. + /// + [Test] + public async Task Status_AddedMemberOfAReappliedSibling_StartsCountingFromZero() + { + string hostPath = FixturePath(HostFileName); + string callerPath = FixturePath(CallerFileName); + Dictionary overrides = new Dictionary + { + [hostPath] = HotReloadTestSourceWriter.WriteEditedSource( + "InvocationCountSiblingHost.cs", + InsertHostMember(AddedMethodMember)), + [callerPath] = HotReloadTestSourceWriter.WriteEditedSource( + "InvocationCountSiblingCallerFirst.cs", + ReplaceCallerBody("return host.Added();")) + }; + HotReloadOrchestratorResult first = await RunAsync(new[] { hostPath, callerPath }, overrides); + AssertNoFailure(first); + new HotReloadCrossFileAddedMemberCaller().Call(new HotReloadCrossFileAddedMemberHost()); + Assert.That( + FindRow(await ExecuteStatusAsync(), "Added", "Added(").InvocationCount, + Is.EqualTo(1L), + "Precondition: the first shim counted the call."); + + overrides[callerPath] = HotReloadTestSourceWriter.WriteEditedSource( + "InvocationCountSiblingCallerSecond.cs", + ReplaceCallerBody("return host.Added() + 1;")); + HotReloadOrchestratorResult second = await RunAsync(new[] { callerPath }, overrides); + AssertNoFailure(second); + Assert.That( + second.ReappliedSiblingPaths, + Does.Contain(ProjectRelativePath(HostFileName)), + "Precondition: the host is re-applied as a sibling of the edited caller."); + + HotReloadMethodResult row = FindRow(await ExecuteStatusAsync(), "Added", "Added("); + Assert.That(row.InvocationCount, Is.EqualTo(0L)); + Assert.That(row.Reason, Is.EqualTo(HotReloadConstants.AddedMemberNeverInvokedReason)); + } + + /// + /// What: after a body patch of a method and a later change of its return type, the label + /// names both the patch of the old signature and the added new declaration, and each keeps + /// its own count: --status reports the old patch's calls on the Active row and the new + /// declaration's on the Added row, and an unchanged reload's AlreadyActive row carries the + /// added declaration's count rather than the ledger's. + /// + [Test] + public async Task Status_ReturnTypeChangeAfterABodyPatch_CountsTheOldPatchAndTheAddedMemberApart() + { + string fixturePath = FixturePath(SameFileFixtureName); + string onDisk = File.ReadAllText(fixturePath); + Assert.That(onDisk, Does.Contain(SameFileTargetDeclaration), "Precondition: Target anchor must exist."); + string bodyOnlyEdit = onDisk.Replace( + SameFileTargetDeclaration, + SameFileTargetDeclaration.Replace("return value;", "return value + 7;"), + StringComparison.Ordinal); + AssertNoFailure(await RunSingleAsync(fixturePath, "InvocationCountBodyPatch.cs", bodyOnlyEdit)); + HotReloadSignatureChangeSameFileFixture host = new HotReloadSignatureChangeSameFileFixture(); + Assert.That(host.ExistingCaller(3), Is.EqualTo(10), "Precondition: the compiled caller reaches the patched body."); + + string returnTypeChange = WithReturnTypeChange(onDisk); + HotReloadOrchestratorResult second = await RunSingleAsync( + fixturePath, + "InvocationCountReturnTypeChange.cs", + returnTypeChange); + AssertNoFailure(second); + AssertHasAdded(second, "Target("); + Assert.That(host.ExistingCaller(3), Is.EqualTo(4)); + Assert.That(host.ExistingCaller(3), Is.EqualTo(4)); + + HotReloadResponse status = await ExecuteStatusAsync(); + Assert.That(FindRow(status, "Active", "Target(").InvocationCount, Is.EqualTo(1L)); + Assert.That(FindRow(status, "Added", "Target(").InvocationCount, Is.EqualTo(2L)); + + HotReloadOrchestratorResult unchanged = await RunSingleAsync( + fixturePath, + "InvocationCountUnchanged.cs", + returnTypeChange); + HotReloadMethodResult alreadyActive = FindRow( + HotReloadTool.BuildApplyResponse(unchanged), + nameof(HotReloadMethodOutcomeKind.AlreadyActive), + "Target("); + Assert.That(alreadyActive.Reason, Is.EqualTo(HotReloadConstants.AlreadyActiveAddedMemberReason)); + Assert.That(alreadyActive.InvocationCount, Is.EqualTo(2L)); + } + + private static Task RunPairAsync( + string editedFileNamePrefix, + string editedHostSource, + string editedCallerSource) + { + string hostPath = FixturePath(HostFileName); + string callerPath = FixturePath(CallerFileName); + return RunAsync( + new[] { hostPath, callerPath }, + new Dictionary + { + [hostPath] = HotReloadTestSourceWriter.WriteEditedSource( + editedFileNamePrefix + "Host.cs", + editedHostSource), + [callerPath] = HotReloadTestSourceWriter.WriteEditedSource( + editedFileNamePrefix + "Caller.cs", + editedCallerSource) + }); + } + + private static Task RunSingleAsync( + string fixturePath, + string editedFileName, + string editedSource) + { + return HotReloadCompositionRoot.Services.Orchestrator.RunAsync( + new[] { fixturePath }, + HotReloadTestSourceWriter.WriteEditedSource(editedFileName, editedSource), + CancellationToken.None); + } + + private static Task RunAsync( + string[] files, + Dictionary overrides) + { + return HotReloadCompositionRoot.Services.Orchestrator.RunAsync( + files, + contentPathOverride: null, + CancellationToken.None, + new Dictionary(overrides)); + } + + private static async Task ExecuteStatusAsync() + { + UnityCliLoopToolResponse baseResponse = await new HotReloadTool().ExecuteAsync( + new JObject { ["Status"] = true }, + CancellationToken.None); + HotReloadResponse response = baseResponse as HotReloadResponse; + Assert.That(response, Is.Not.Null); + Assert.That(response.Success, Is.True); + return response; + } + + private static HotReloadMethodResult FindRow(HotReloadResponse response, string kind, string methodToken) + { + List rows = new List(); + foreach (HotReloadMethodResult row in response.Methods) + { + if (row.Kind == kind && row.Method.Contains(methodToken)) + { + return row; + } + + rows.Add(row.Kind + " " + row.Method + " (" + row.InvocationCount + ")"); + } + + Assert.Fail("No " + kind + " row containing '" + methodToken + "' in:\n" + string.Join("\n", rows)); + return null; + } + + 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); + } + + private static string ReplaceCallerBody(string bodyText) + { + string source = File.ReadAllText(FixturePath(CallerFileName)); + Assert.That(source, Does.Contain(CallerCallBodyAnchor), "Precondition: caller anchor must exist."); + return source.Replace(CallerCallBodyAnchor, bodyText, StringComparison.Ordinal); + } + + // The same-file return-type change: Target returns long, and its only caller casts back. + private static string WithReturnTypeChange(string onDisk) + { + string edited = onDisk + .Replace( + SameFileTargetDeclaration, + " public long Target(int value)\n {\n return value + 1L;\n }", + StringComparison.Ordinal) + .Replace( + " return Target(value);\n }", + " return (int)Target(value);\n }", + StringComparison.Ordinal); + Assert.That(edited, Is.Not.EqualTo(onDisk), "Precondition: the return-type edit must apply."); + return edited; + } + + 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 AssertHasAdded(HotReloadOrchestratorResult result, string methodToken) + { + foreach (HotReloadMethodOutcome outcome in result.Methods) + { + if (outcome.Kind == HotReloadMethodOutcomeKind.Added && outcome.Method.Contains(methodToken)) + { + return; + } + } + + Assert.Fail("No Added outcome containing '" + methodToken + "'."); + } + + 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 ProjectRelativePath(string fileName) + { + return "Assets/Tests/Editor/HotReload/" + fileName; + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadAddedMemberInvocationCountE2ETests.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadAddedMemberInvocationCountE2ETests.cs.meta new file mode 100644 index 000000000..7a894f824 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadAddedMemberInvocationCountE2ETests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 2d25143d2c8024e89859646a42365700 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadCoreFixtures.cs b/Assets/Tests/Editor/HotReload/HotReloadCoreFixtures.cs index 68c7251c7..8644f7bff 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadCoreFixtures.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadCoreFixtures.cs @@ -1,3 +1,4 @@ +using System.Reflection; using System.Runtime.CompilerServices; using System.Threading.Tasks; @@ -86,6 +87,11 @@ public string FormatAlignedStaticCount() /// public static class HotReloadHandwrittenShims { + // The invocation counter a generated added-member shim declares beside itself; tests that + // resolve StaticPing__shim0 as an added method need it, because resolution refuses a shim + // without one. + public static long StaticPing__shim0__uloopCalls; + public static string StaticPing__shim0() { return "patched"; @@ -114,5 +120,30 @@ public static async Task ReplaceableComputeAsync__shim0( await Task.Yield(); return instance.PublicSeed + delta + 1; } + + // An added-member shim with no invocation counter beside it, the shape a worker that + // disagreed with the Editor on the counter's name would emit. + public static void AddedWithoutCounter__shim0() + { + } + + // An added-member shim whose invocation counter is not a long, so the Editor cannot read + // it as the member's count. + public static int AddedWithIntCounter__shim0__uloopCalls; + + public static void AddedWithIntCounter__shim0() + { + } + } + + /// + /// An invocation counter for tests that register an added member without ever reading its + /// InvocationCount: registration requires the static long counter a generated shim declares. + /// + internal static class HotReloadUnreadInvocationCounter + { + public static long Calls; + + internal static FieldInfo Field => typeof(HotReloadUnreadInvocationCounter).GetField(nameof(Calls)); } } diff --git a/Assets/Tests/Editor/HotReload/HotReloadDomainTestAccess.cs b/Assets/Tests/Editor/HotReload/HotReloadDomainTestAccess.cs index a4806554b..dd8c69b8e 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadDomainTestAccess.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadDomainTestAccess.cs @@ -90,14 +90,25 @@ internal HotReloadFileGeneration GetOrBeginAddedMemberGeneration(string projectR return Domain.BeginAddedMemberOnlyGeneration(projectRelativePath); } + /// + /// Registers an added member in a fresh added-member generation of the file, counting its + /// calls on invocationCounter, or on a counter no test reads when none is given. + /// internal void RegisterAddedMember( string projectRelativePath, string methodKey, MethodInfo shimMethod, - string filePath) + string filePath, + FieldInfo invocationCounter = null) { Domain.BeginAddedMemberOnlyGeneration(projectRelativePath) - .RegisterAddedMethod(methodKey, shimMethod, filePath, "Added", "Fixture"); + .RegisterAddedMethod( + methodKey, + shimMethod, + filePath, + "Added", + "Fixture", + invocationCounter ?? HotReloadUnreadInvocationCounter.Field); } internal void ReplaceAddedFields( diff --git a/Assets/Tests/Editor/HotReload/HotReloadDomainTests.cs b/Assets/Tests/Editor/HotReload/HotReloadDomainTests.cs index 86ad594ed..d2767321d 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadDomainTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadDomainTests.cs @@ -90,7 +90,13 @@ public void BeginAddedMemberOnlyGeneration_KeepsTheShimGeneration() generation.RegisterShimMethod( GetAddedTarget(), new HotReloadShimMethodEntry(GetAddedTarget(), false, 1, 2)); - generation.RegisterAddedMethod(AddedMethodKey, GetAddedTarget(), FileOne, "AddedPing", "DomainHost"); + generation.RegisterAddedMethod( + AddedMethodKey, + GetAddedTarget(), + FileOne, + "AddedPing", + "DomainHost", + HotReloadUnreadInvocationCounter.Field); Assert.That( _access.Domain.ListActiveAddedMethodKeys(FileOne), Does.Contain(AddedMethodKey), diff --git a/Assets/Tests/Editor/HotReload/HotReloadEntryResolutionTests.cs b/Assets/Tests/Editor/HotReload/HotReloadEntryResolutionTests.cs index 7a9c79701..334b95629 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadEntryResolutionTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadEntryResolutionTests.cs @@ -135,25 +135,60 @@ public void ResolveEntries_WhenShimMethodIsMissing_FailsTheFileAtomically() Does.Contain("ReplaceableCompute__shimAbsent")); } + /// + /// What: an added-method entry naming a shim with no usable invocation counter beside it + /// (none, or one that is not a long) fails the whole file like a missing shim does, and + /// the Failed row names the counter the Editor looked for. + /// + [TestCase(nameof(HotReloadHandwrittenShims.AddedWithoutCounter__shim0))] + [TestCase(nameof(HotReloadHandwrittenShims.AddedWithIntCounter__shim0))] + public void ResolveEntries_WhenAddedMethodHasNoUsableCounter_FailsTheFileAtomically(string shimMethodName) + { + TransformWorkerEntryDto[] entries = + { + BuildExistingMethodEntry( + nameof(HotReloadCoreFixture.StaticPing), + new string[0], + "StaticPing__shim0"), + BuildAddedMethodEntry(shimMethodName) + }; + + HotReloadEntryResolution.Result result = HotReloadEntryResolution.ResolveEntries( + TestAssemblyHome, + FileHomeResolver, + FilePath, + ShimAssembly, + entries, + new Dictionary()); + + 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].Kind, + Is.EqualTo(HotReloadMethodOutcomeKind.Skipped)); + 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(shimMethodName + "__uloopCalls")); + } + /// /// What: an added-method entry resolves through the shim lookup alone — it needs no - /// compiled original method, and the resolved entry is marked as an added method. + /// compiled original method, the resolved entry is marked as an added method, and it + /// carries the invocation counter declared beside its shim. /// [Test] public void ResolveEntries_WhenEntryIsAnAddedMethod_ResolvesWithoutAnOriginalMethod() { TransformWorkerEntryDto[] entries = { - new TransformWorkerEntryDto - { - sourceProjectRelativePath = FilePath, - typeMetadataName = FixtureTypeMetadataName, - methodName = "AddedByThisReload", - parameterTypeFullNames = new string[0], - shimTypeName = ShimTypeName, - shimMethodName = "StaticPing__shim0", - patchKind = HotReloadConstants.PatchKindAddedMethod - } + BuildAddedMethodEntry(nameof(HotReloadHandwrittenShims.StaticPing__shim0)) }; HotReloadEntryResolution.Result result = HotReloadEntryResolution.ResolveEntries( @@ -169,6 +204,10 @@ public void ResolveEntries_WhenEntryIsAnAddedMethod_ResolvesWithoutAnOriginalMet Assert.That(result.ResolvedEntries[0].IsAddedMethod, Is.True); Assert.That(result.ResolvedEntries[0].OriginalMethod, Is.Null); Assert.That(result.ResolvedEntries[0].ShimMethod, Is.Not.Null); + Assert.That( + result.ResolvedEntries[0].InvocationCounter, + Is.EqualTo(typeof(HotReloadHandwrittenShims).GetField( + nameof(HotReloadHandwrittenShims.StaticPing__shim0__uloopCalls)))); } private static string ResolveProjectRoot() @@ -191,5 +230,19 @@ private static TransformWorkerEntryDto BuildExistingMethodEntry( shimMethodName = shimMethodName }; } + + private static TransformWorkerEntryDto BuildAddedMethodEntry(string shimMethodName) + { + return new TransformWorkerEntryDto + { + sourceProjectRelativePath = FilePath, + typeMetadataName = FixtureTypeMetadataName, + methodName = "AddedByThisReload", + parameterTypeFullNames = new string[0], + shimTypeName = ShimTypeName, + shimMethodName = shimMethodName, + patchKind = HotReloadConstants.PatchKindAddedMethod + }; + } } } diff --git a/Assets/Tests/Editor/HotReload/HotReloadFileGenerationContractTests.cs b/Assets/Tests/Editor/HotReload/HotReloadFileGenerationContractTests.cs index 0f1af535a..cc428baa2 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadFileGenerationContractTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadFileGenerationContractTests.cs @@ -24,6 +24,11 @@ public class HotReloadFileGenerationContractTests private const string PlaceholderShimSourcePath = "/placeholder/Source.cs"; private const string PlaceholderShimSourceContentSha256 = "placeholder-sha256"; + // Counters of the two shapes a registration refuses besides a missing one; no test reads + // their values. + public long InstanceLongCounter; + public static int StaticIntCounter; + /// /// What: a patch of a method whose shim this generation never registered is refused, which /// is the invariant that every patch is a patch of a method the edited source declares. @@ -113,7 +118,39 @@ public void RegisterAddedMethod_BeforeTheGenerationIsOpened_Throws() GetAddedTarget(), FixtureProjectRelativePath, "AddedMember", - "FileGenerationContractFixture")); + "FileGenerationContractFixture", + HotReloadUnreadInvocationCounter.Field)); + Assert.That(generation.AddedMemberCount, Is.EqualTo(0)); + } + + /// + /// What: registering an added member without a counter its InvocationCount can be read + /// from (none, an instance field, or a static field that is not a long) is refused, and + /// nothing is registered. + /// + [TestCase(null)] + [TestCase(nameof(InstanceLongCounter))] + [TestCase(nameof(StaticIntCounter))] + public void RegisterAddedMethod_WithoutAUsableCounter_Throws(string counterFieldName) + { + HotReloadFileGeneration generation = new HotReloadFileGeneration(FixtureProjectRelativePath); + generation.BeginAddedMemberGeneration(); + FieldInfo counter = counterFieldName == null + ? null + : typeof(HotReloadFileGenerationContractTests).GetField(counterFieldName); + Assert.That( + counter == null, + Is.EqualTo(counterFieldName == null), + "Precondition: a named counter field must exist, so the case tests its shape."); + + Assert.Throws( + () => generation.RegisterAddedMethod( + AddedMethodKey, + GetAddedTarget(), + FixtureProjectRelativePath, + "AddedMember", + "FileGenerationContractFixture", + counter)); Assert.That(generation.AddedMemberCount, Is.EqualTo(0)); } diff --git a/Assets/Tests/Editor/HotReload/HotReloadFileGenerationTests.cs b/Assets/Tests/Editor/HotReload/HotReloadFileGenerationTests.cs index a4107b2ea..718124c5c 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadFileGenerationTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadFileGenerationTests.cs @@ -86,8 +86,20 @@ public void RegisterAddedMethod_OverwritesSameKey_AndIsDescribed() { HotReloadFileGeneration generation = CreateGeneration(); generation.BeginAddedMemberGeneration(); - generation.RegisterAddedMethod(AddedMethodKey, GetShimTarget(), FixtureProjectRelativePath, "AddedMember", AddedMethodType); - generation.RegisterAddedMethod(AddedMethodKey, GetAddedTarget(), FixtureProjectRelativePath, "AddedMember", AddedMethodType); + generation.RegisterAddedMethod( + AddedMethodKey, + GetShimTarget(), + FixtureProjectRelativePath, + "AddedMember", + AddedMethodType, + HotReloadUnreadInvocationCounter.Field); + generation.RegisterAddedMethod( + AddedMethodKey, + GetAddedTarget(), + FixtureProjectRelativePath, + "AddedMember", + AddedMethodType, + HotReloadUnreadInvocationCounter.Field); List members = new List(); generation.DescribeAddedMembers(members); @@ -115,6 +127,7 @@ public void FindAddedMethodContainingLine_ReportsTheAddedMethodOnlyInsideItsRang FixtureProjectRelativePath, "AddedMember", AddedMethodType, + HotReloadUnreadInvocationCounter.Field, sourceStartLine: 20, sourceEndLine: 24); @@ -142,6 +155,7 @@ public void FindAddedMethodContainingLine_ReportsShortTypeNamesFromTheMetadataNa FixtureProjectRelativePath, "Step", NestedCecilType, + HotReloadUnreadInvocationCounter.Field, sourceStartLine: 10, sourceEndLine: 12); generation.RegisterAddedMethod( @@ -150,6 +164,7 @@ public void FindAddedMethodContainingLine_ReportsShortTypeNamesFromTheMetadataNa FixtureProjectRelativePath, "AddedMember", HostType, + HotReloadUnreadInvocationCounter.Field, sourceStartLine: 20, sourceEndLine: 24); @@ -174,7 +189,13 @@ public void FindAddedMethodContainingLine_AddedMethodWithoutARange_ReportsNothin { HotReloadFileGeneration generation = CreateGeneration(); generation.BeginAddedMemberGeneration(); - generation.RegisterAddedMethod(AddedMethodKey, GetAddedTarget(), FixtureProjectRelativePath, "AddedMember", AddedMethodType); + generation.RegisterAddedMethod( + AddedMethodKey, + GetAddedTarget(), + FixtureProjectRelativePath, + "AddedMember", + AddedMethodType, + HotReloadUnreadInvocationCounter.Field); Assert.That(generation.FindAddedMethodContainingLine(0), Is.Null); Assert.That(generation.FindAddedMethodContainingLine(1), Is.Null); @@ -189,7 +210,13 @@ public void AddedMemberQueries_ReportOnlyRegisteredKeys() { HotReloadFileGeneration generation = CreateGeneration(); generation.BeginAddedMemberGeneration(); - generation.RegisterAddedMethod(AddedMethodKey, GetAddedTarget(), FixtureProjectRelativePath, "AddedMember", AddedMethodType); + generation.RegisterAddedMethod( + AddedMethodKey, + GetAddedTarget(), + FixtureProjectRelativePath, + "AddedMember", + AddedMethodType, + HotReloadUnreadInvocationCounter.Field); Assert.That(generation.IsActiveMember(AddedMethodKey), Is.True); Assert.That(generation.IsActiveMember(OtherAddedMethodKey), Is.False); @@ -205,7 +232,13 @@ public void BeginAddedMemberGeneration_DropsPreviousMembersAndFields() { HotReloadFileGeneration generation = CreateGeneration(); generation.BeginAddedMemberGeneration(); - generation.RegisterAddedMethod(AddedMethodKey, GetAddedTarget(), FixtureProjectRelativePath, "AddedMember", AddedMethodType); + generation.RegisterAddedMethod( + AddedMethodKey, + GetAddedTarget(), + FixtureProjectRelativePath, + "AddedMember", + AddedMethodType, + HotReloadUnreadInvocationCounter.Field); generation.ReplaceAddedFields(new[] { HostType + ".alpha" }, null, null, null); generation.BeginAddedMemberGeneration(); diff --git a/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs b/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs index 0eebbf689..659f7ccd0 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs @@ -4712,11 +4712,12 @@ public async Task Run_ReapplyWorkingAddedMethod_DoesNotWarnDeactivatedPatches() /// /// What: an unchanged reload after a fully applied added-method run reports the added - /// member as AlreadyActive with the added-member Reason and InvocationCount 0, while - /// the patched caller keeps the ordinary AlreadyActive Reason. + /// member as AlreadyActive with the added-member Reason and the calls its own counter + /// holds, while the patched caller keeps the ordinary AlreadyActive Reason and the + /// ledger's count. /// [Test] - public async Task Run_UnchangedReload_AlreadyActiveAddedMember_UsesAddedReasonAndZeroCount() + public async Task Run_UnchangedReload_AlreadyActiveAddedMember_CarriesTheAddedMembersCount() { string fixturePath = ResolveAddedMethodApplyFixturePath(); string onDisk = File.ReadAllText(fixturePath); @@ -4742,13 +4743,14 @@ public async Task Run_UnchangedReload_AlreadyActiveAddedMember_UsesAddedReasonAn Assert.That( addedRow.Reason, Is.EqualTo(HotReloadConstants.AlreadyActiveAddedMemberReason)); - Assert.That(addedRow.InvocationCount, Is.EqualTo(0L)); + Assert.That(addedRow.InvocationCount, Is.EqualTo(1L)); HotReloadMethodResult patchedRow = FindResponseMethod( response, nameof(HotReloadAddedMethodApplyFixture.ExistingCaller)); Assert.That(patchedRow.Kind, Is.EqualTo(nameof(HotReloadMethodOutcomeKind.AlreadyActive))); Assert.That(patchedRow.Reason, Is.EqualTo(HotReloadConstants.AlreadyActiveReason)); + Assert.That(patchedRow.InvocationCount, Is.EqualTo(1L)); } /// diff --git a/Assets/Tests/Editor/HotReload/HotReloadPlayModeEntryDropStatusTests.cs b/Assets/Tests/Editor/HotReload/HotReloadPlayModeEntryDropStatusTests.cs index f093dc34d..25ae25fb8 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadPlayModeEntryDropStatusTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadPlayModeEntryDropStatusTests.cs @@ -101,7 +101,7 @@ public async Task ExecuteAsync_Status_WhenActiveNeverInvokedAndDropsRemain_Keeps Assert.That( response.Message, Is.EqualTo( - "1 change(s) currently active. 1 change(s) have not been invoked since their patch was applied; see Methods[].Reason.")); + "1 change(s) currently active. 1 change(s) have not been invoked since they were applied; see Methods[].Reason.")); Assert.That(response.DroppedByPlayModeEntryCount, Is.EqualTo(1)); Assert.That(response.ShouldSerializeDroppedByPlayModeEntryCount(), Is.True); Assert.That(json.Value("DroppedByPlayModeEntryCount"), Is.EqualTo(1)); diff --git a/Assets/Tests/Editor/HotReload/HotReloadToolTests.cs b/Assets/Tests/Editor/HotReload/HotReloadToolTests.cs index eb1e593b3..60aa71d5e 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadToolTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadToolTests.cs @@ -29,12 +29,17 @@ public class HotReloadToolTests private HotReloadDomainTestScope _scope; + // The counter the added members these tests register report as their InvocationCount. + // SetUp zeroes it, so a test that sets it sees only its own calls. + public static long StatusAddedMemberCalls; + [SetUp] public void SetUp() { _ledgerSessionScope = new HotReloadPlayModeEntryDropLedgerSessionScope(); _scope = new HotReloadDomainTestScope(); HotReloadAutoRefreshHold.SyncToActiveChanges(); + StatusAddedMemberCalls = 0; } [TearDown] @@ -188,11 +193,12 @@ public async Task ExecuteAsync_NoChangedFiles_WhenNoActivePatches_KeepsOriginalM } /// - /// What: --status Added rows explain InvocationCount 0 with the not-instrumented Reason - /// only, not the AlreadyActive source-unchanged sentence. + /// What: a --status Added row whose member has not run since it was applied reports + /// InvocationCount 0 with the added-member never-invoked Reason, which says what can call + /// an added member, and not the AlreadyActive source-unchanged sentence. /// [Test] - public async Task ExecuteAsync_Status_AddedRow_SetsNotInstrumentedReason() + public async Task ExecuteAsync_Status_NeverInvokedAddedRow_SetsNeverInvokedReason() { const string filePath = "Assets/Tests/Editor/HotReload/StatusAddedReason.cs"; const string methodKey = "Host.NewHelper(System.Int32)"; @@ -203,11 +209,35 @@ public async Task ExecuteAsync_Status_AddedRow_SetsNotInstrumentedReason() HotReloadConstants.AddedMemberStatusKind, methodKey); + Assert.That(addedRow.InvocationCount, Is.EqualTo(0L)); Assert.That( addedRow.Reason, Is.EqualTo( - "Added-member calls are not instrumented, so InvocationCount is always 0 for this row.")); - Assert.That(addedRow.InvocationCount, Is.EqualTo(0L)); + "Not invoked since this added member was applied. Compiled code cannot call a member that hot reload added, so it runs only when a hot-reloaded body that calls it runs, or, for a forwarded Unity message, when the hot-reload proxy delivers the message in Play Mode.")); + Assert.That(addedRow.Reason, Is.EqualTo(HotReloadConstants.AddedMemberNeverInvokedReason)); + } + + /// + /// What: a --status Added row whose member has run reports the calls its counter holds + /// with an empty Reason, and Message leaves it out of the never-invoked count. + /// + [Test] + public async Task ExecuteAsync_Status_InvokedAddedRow_LeavesReasonEmptyAndOutOfTheAggregate() + { + const string filePath = "Assets/Tests/Editor/HotReload/StatusAddedReason.cs"; + const string methodKey = "Host.NewHelper(System.Int32)"; + RegisterAddedMemberForStatus(filePath, methodKey); + StatusAddedMemberCalls = 1; + + HotReloadResponse response = await ExecuteStatusAsync(CancellationToken.None); + HotReloadMethodResult addedRow = FindStatusRow( + response, + HotReloadConstants.AddedMemberStatusKind, + methodKey); + + Assert.That(addedRow.InvocationCount, Is.EqualTo(1L)); + Assert.That(addedRow.Reason, Is.EqualTo(string.Empty)); + Assert.That(response.Message, Is.EqualTo("1 change(s) currently active.")); } /// @@ -243,7 +273,7 @@ public async Task ExecuteAsync_Status_NeverInvokedActiveRow_SetsNeverInvokedReas Assert.That( response.Message, Is.EqualTo( - "1 change(s) currently active. 1 change(s) have not been invoked since their patch was applied; see Methods[].Reason.")); + "1 change(s) currently active. 1 change(s) have not been invoked since they were applied; see Methods[].Reason.")); Assert.That(response.AutoRefreshHeld, Is.True); } finally @@ -289,11 +319,11 @@ public async Task ExecuteAsync_Status_InvokedActiveRow_LeavesReasonEmpty() } /// - /// What: --status Message counts never-invoked Active rows only, not added-member - /// rows, when both kinds are present. + /// What: --status Message counts the never-invoked Added rows together with the + /// never-invoked Active rows when both kinds are present. /// [Test] - public async Task ExecuteAsync_Status_MixedActiveAndAdded_CountsOnlyNeverInvokedActiveInAggregate() + public async Task ExecuteAsync_Status_MixedActiveAndAdded_CountsNeverInvokedActiveAndAddedRows() { const string filePath = "Assets/Tests/Editor/HotReload/StatusAddedReason.cs"; const string methodKey = "Host.NewHelper(System.Int32)"; @@ -319,11 +349,10 @@ public async Task ExecuteAsync_Status_MixedActiveAndAdded_CountsOnlyNeverInvoked Assert.That( response.Message, Is.EqualTo( - "3 change(s) currently active. 2 change(s) have not been invoked since their patch was applied; see Methods[].Reason.")); + "3 change(s) currently active. 3 change(s) have not been invoked since they were applied; see Methods[].Reason.")); Assert.That( addedRow.Reason, - Is.EqualTo( - "Added-member calls are not instrumented, so InvocationCount is always 0 for this row.")); + Is.EqualTo(HotReloadConstants.AddedMemberNeverInvokedReason)); } finally { @@ -466,17 +495,17 @@ public async Task ExecuteAsync_Status_RegisteredAddedFields_ListsAddedFieldRowsO } /// - /// What: composing AlreadyActiveAddedMemberReason from the not-instrumented constant - /// keeps the historical AlreadyActive added-member sentence byte-identical. + /// What: an added member's AlreadyActive Reason says the member stays available and keeps + /// its InvocationCount, in the words a patch's AlreadyActive Reason uses for the patch. /// [Test] - public void AlreadyActiveAddedMemberReason_KeepsHistoricalWording() + public void AlreadyActiveAddedMemberReason_SaysTheMemberKeepsItsInvocationCount() { Assert.That( HotReloadConstants.AlreadyActiveAddedMemberReason, Is.EqualTo( - "Source is unchanged since the last applied hot reload; the existing added member stays available. " - + "Added-member calls are not instrumented, so InvocationCount is always 0 for this row.")); + "Source is unchanged since the last applied hot reload; the existing added member stays " + + "available and keeps its InvocationCount. Edit and reload again to apply new changes.")); } /// @@ -2971,14 +3000,19 @@ private static void ApplyCoreFixtureTransplant( Assert.That(applyResult.Success, Is.True, applyResult.ErrorMessage); } - // Registers one added-member ledger row the same way the added-row status test does. + // Registers one added-member ledger row that reports StatusAddedMemberCalls as its count. private static void RegisterAddedMemberForStatus(string filePath, string methodKey) { MethodInfo shim = typeof(HotReloadAddedMemberHost).GetMethod( nameof(HotReloadAddedMemberHost.ExistingCaller), BindingFlags.Instance | BindingFlags.Public); Assert.That(shim, Is.Not.Null); - new HotReloadDomainTestAccess().RegisterAddedMember(filePath, methodKey, shim, filePath); + new HotReloadDomainTestAccess().RegisterAddedMember( + filePath, + methodKey, + shim, + filePath, + typeof(HotReloadToolTests).GetField(nameof(StatusAddedMemberCalls))); } private static async Task ExecuteStatusAsync(CancellationToken ct) diff --git a/Assets/Tests/Editor/HotReload/HotReloadUnityMessageForwardingEditorHooksTests.cs b/Assets/Tests/Editor/HotReload/HotReloadUnityMessageForwardingEditorHooksTests.cs index 72e3b6ca4..c325a68c6 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadUnityMessageForwardingEditorHooksTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadUnityMessageForwardingEditorHooksTests.cs @@ -131,7 +131,13 @@ private HotReloadUnityMessageProxyFixture ArrangeAttachedProxy() BindingFlags.Public | BindingFlags.Static); Assert.That(shim, Is.Not.Null, "The fixture shim method must exist."); _access.GetOrBeginAddedMemberGeneration(FixturePath) - .RegisterAddedMethod("Fixture.Update", shim, FixturePath, "Update", "Fixture"); + .RegisterAddedMethod( + "Fixture.Update", + shim, + FixturePath, + "Update", + "Fixture", + HotReloadUnreadInvocationCounter.Field); _forwarding.Tick(); Assert.That( ProxiesOn(owner).Length, diff --git a/Assets/Tests/Editor/HotReload/HotReloadUnityMessageForwardingTests.cs b/Assets/Tests/Editor/HotReload/HotReloadUnityMessageForwardingTests.cs index 122fd940b..6ef3f5a3e 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadUnityMessageForwardingTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadUnityMessageForwardingTests.cs @@ -209,19 +209,27 @@ private void RegisterUnbuildableFixture() ShimOf(typeof(HotReloadUnityMessageProxyFixtureShims), "Update"), FixturePath, "Update", - "Fixture"); + "Fixture", + HotReloadUnreadInvocationCounter.Field); generation.RegisterAddedMethod( "Fixture.Update.B", ShimOf(typeof(HotReloadUnityMessageDuplicateFixtureShims), "Update"), FixturePath, "Update", - "Fixture"); + "Fixture", + HotReloadUnreadInvocationCounter.Field); } private void RegisterAdded(string projectRelativePath, string methodKey, MethodInfo shim) { _access.GetOrBeginAddedMemberGeneration(projectRelativePath) - .RegisterAddedMethod(methodKey, shim, projectRelativePath, "Update", "Fixture"); + .RegisterAddedMethod( + methodKey, + shim, + projectRelativePath, + "Update", + "Fixture", + HotReloadUnreadInvocationCounter.Field); } private HotReloadUnityMessageProxyFixture CreateFixture() diff --git a/Assets/Tests/Editor/HotReload/TransformWorkerAddedFieldTests.cs b/Assets/Tests/Editor/HotReload/TransformWorkerAddedFieldTests.cs index d42610e25..5d28a0710 100644 --- a/Assets/Tests/Editor/HotReload/TransformWorkerAddedFieldTests.cs +++ b/Assets/Tests/Editor/HotReload/TransformWorkerAddedFieldTests.cs @@ -2524,7 +2524,9 @@ private static string FormatSkipped(TransformWorkerSkippedDto[] skipped) private static string SliceShimMethod(string shimSource, string shimMethodName) { - int nameIndex = shimSource.IndexOf(shimMethodName, StringComparison.Ordinal); + // Why the '(': an added member's shim type also declares its invocation counter, whose + // name starts with the shim method name. + int nameIndex = shimSource.IndexOf(shimMethodName + "(", StringComparison.Ordinal); Assert.That(nameIndex, Is.GreaterThanOrEqualTo(0), "Shim method missing: " + shimMethodName); int declarationStart = shimSource.LastIndexOf("public static", nameIndex, StringComparison.Ordinal); int openBrace = shimSource.IndexOf('{', nameIndex); diff --git a/Assets/Tests/Editor/HotReload/TransformWorkerAddedMemberTests.cs b/Assets/Tests/Editor/HotReload/TransformWorkerAddedMemberTests.cs index c5d62aaa6..e0ae42eec 100644 --- a/Assets/Tests/Editor/HotReload/TransformWorkerAddedMemberTests.cs +++ b/Assets/Tests/Editor/HotReload/TransformWorkerAddedMemberTests.cs @@ -286,6 +286,83 @@ public async Task Emit_AccessorLevelArrowAddedInstanceProperty_ReceiverCheckMaps AssertReceiverCheckMapsTo(result.Output.shimSource, FindLineNumberContaining(edited, "get => 8;")); } + /// + /// What: every added-member shim declares its own static long invocation counter and + /// increments it exactly once before its body runs: after the receiver check for an + /// instance member, with no receiver check for a static one, and the same for expression + /// bodies and for auto-property accessors built against the added-field store. + /// + [TestCase("Instance block method", "public int AddedCountedBlock(int value)\n {\n return value + 1;\n }", "AddedCountedBlock", "return value + 1;", true)] + [TestCase("Instance arrow method", "public int AddedCountedArrow() => 42;", "AddedCountedArrow", "return 42;", true)] + [TestCase("Instance throw arrow method", "public int AddedCountedThrow() => throw new System.InvalidOperationException();", "AddedCountedThrow", "InvalidOperationException", true)] + [TestCase("Static block method", "public static int AddedCountedStatic(int value)\n {\n return value + 2;\n }", "AddedCountedStatic", "return value + 2;", false)] + [TestCase("Static arrow method", "public static int AddedCountedStaticArrow() => 43;", "AddedCountedStaticArrow", "return 43;", false)] + [TestCase("Bodied getter", "public int AddedCountedBodied\n {\n get { return 3; }\n }", "get_AddedCountedBodied", "return 3;", true)] + [TestCase("Instance auto getter", "public int AddedCountedAuto { get; set; }", "get_AddedCountedAuto", "GetOrInit", true)] + [TestCase("Instance auto setter", "public int AddedCountedAuto { get; set; }", "set_AddedCountedAuto", "Set<", true)] + [TestCase("Static auto getter", "public static int AddedCountedStaticAuto { get; set; }", "get_AddedCountedStaticAuto", "GetOrInitStatic", false)] + public async Task Emit_AddedMemberShim_DeclaresItsCounterAndIncrementsItOnceBeforeTheBody( + string label, + string hostMember, + string methodName, + string bodyMarker, + bool expectsReceiverCheck) + { + TransformWorkerClientResult result = await RunHostWithAddedMembersAsync(hostMember); + Assert.That(result.Success, Is.True, result.ErrorMessage); + TransformWorkerEntryDto added = FindEntry(result, methodName); + Assert.That(added, Is.Not.Null, label); + + string counterName = added.shimMethodName + "__uloopCalls"; + string shimSource = result.Output.shimSource; + Assert.That( + shimSource, + Does.Contain("public static long " + counterName + ";"), + label + "\n" + shimSource); + + string method = ExtractShimMethod(shimSource, added.shimMethodName); + string increment = "global::System.Threading.Interlocked.Increment(ref " + counterName + ");"; + int incrementIndex = method.IndexOf(increment, StringComparison.Ordinal); + Assert.That(incrementIndex, Is.GreaterThan(0), label + "\n" + method); + Assert.That( + method.IndexOf(increment, incrementIndex + increment.Length, StringComparison.Ordinal), + Is.EqualTo(-1), + "The counter must be incremented once.\n" + method); + Assert.That( + method.IndexOf(bodyMarker, incrementIndex, StringComparison.Ordinal), + Is.GreaterThan(incrementIndex), + "The increment must run before the body.\n" + method); + + int receiverCheckIndex = method.IndexOf( + "throw new global::System.NullReferenceException", + StringComparison.Ordinal); + if (expectsReceiverCheck) + { + Assert.That(receiverCheckIndex, Is.GreaterThan(0), label + "\n" + method); + Assert.That( + receiverCheckIndex, + Is.LessThan(incrementIndex), + "A call the receiver check refuses must not count.\n" + method); + } + else + { + Assert.That(receiverCheckIndex, Is.EqualTo(-1), label + "\n" + method); + } + } + + // The text of one emitted shim method: from its declaration to the #line default the + // emitter appends after every method. + private static string ExtractShimMethod(string shimSource, string shimMethodName) + { + Match declaration = Regex.Match( + shimSource, + @"public static [^\n]*\b" + Regex.Escape(shimMethodName) + @"\("); + Assert.That(declaration.Success, Is.True, "Shim method missing: " + shimMethodName + "\n" + shimSource); + int end = shimSource.IndexOf("#line default", declaration.Index, StringComparison.Ordinal); + Assert.That(end, Is.GreaterThan(declaration.Index), shimSource); + return shimSource.Substring(declaration.Index, end - declaration.Index); + } + private static void AssertReceiverCheckMapsTo(string shimSource, int expectedLine) { int guardIndex = shimSource.IndexOf( @@ -2128,7 +2205,9 @@ private static string FormatSkipped(TransformWorkerSkippedDto[] skipped) private static string SliceShimMethod(string shimSource, string shimMethodName) { - int nameIndex = shimSource.IndexOf(shimMethodName, StringComparison.Ordinal); + // Why the '(': an added member's shim type also declares its invocation counter, whose + // name starts with the shim method name. + int nameIndex = shimSource.IndexOf(shimMethodName + "(", StringComparison.Ordinal); Assert.That(nameIndex, Is.GreaterThanOrEqualTo(0), "Shim method missing: " + shimMethodName); int declarationStart = shimSource.LastIndexOf("public static", nameIndex, StringComparison.Ordinal); int openBrace = shimSource.IndexOf('{', nameIndex); diff --git a/Assets/Tests/Editor/HotReload/TransformWorkerAddedPropertyTests.cs b/Assets/Tests/Editor/HotReload/TransformWorkerAddedPropertyTests.cs index 0e39190e1..305f9e7b3 100644 --- a/Assets/Tests/Editor/HotReload/TransformWorkerAddedPropertyTests.cs +++ b/Assets/Tests/Editor/HotReload/TransformWorkerAddedPropertyTests.cs @@ -1179,7 +1179,9 @@ private static int FindShimMethodDeclarationStart(string shimSource, string shim } int openBrace = shimSource.IndexOf('{', declarationStart); - int nameIndex = shimSource.IndexOf(shimMethodName, declarationStart, StringComparison.Ordinal); + // Why the '(': an added member's shim type also declares its invocation counter, + // whose name starts with the shim method name. + int nameIndex = shimSource.IndexOf(shimMethodName + "(", declarationStart, StringComparison.Ordinal); if (nameIndex >= declarationStart && nameIndex < openBrace) { return declarationStart; diff --git a/Assets/Tests/Editor/HotReload/TransformWorkerClientTests.cs b/Assets/Tests/Editor/HotReload/TransformWorkerClientTests.cs index a391d5365..d70e4649f 100644 --- a/Assets/Tests/Editor/HotReload/TransformWorkerClientTests.cs +++ b/Assets/Tests/Editor/HotReload/TransformWorkerClientTests.cs @@ -450,7 +450,9 @@ private static (bool Found, int DeclarationStart, int CloseBraceIndex) FindShimM return (false, -1, -1); } - int nameIndex = shimSource.IndexOf(shimMethodName, StringComparison.Ordinal); + // Why the '(': an added member's shim type also declares its invocation counter, whose + // name starts with the shim method name. + int nameIndex = shimSource.IndexOf(shimMethodName + "(", StringComparison.Ordinal); if (nameIndex < 0) { return (false, -1, -1); diff --git a/Assets/Tests/Editor/HotReload/TransformWorkerEventAccessorTests.cs b/Assets/Tests/Editor/HotReload/TransformWorkerEventAccessorTests.cs index f030be6c1..a021022d1 100644 --- a/Assets/Tests/Editor/HotReload/TransformWorkerEventAccessorTests.cs +++ b/Assets/Tests/Editor/HotReload/TransformWorkerEventAccessorTests.cs @@ -436,7 +436,9 @@ private static string FormatSkipped(TransformWorkerSkippedDto[] skipped) private static string SliceShimMethod(string shimSource, string shimMethodName) { - int nameIndex = shimSource.IndexOf(shimMethodName, StringComparison.Ordinal); + // Why the '(': an added member's shim type also declares its invocation counter, whose + // name starts with the shim method name. + int nameIndex = shimSource.IndexOf(shimMethodName + "(", StringComparison.Ordinal); Assert.That(nameIndex, Is.GreaterThanOrEqualTo(0), "Shim method missing: " + shimMethodName); int declarationStart = shimSource.LastIndexOf("public static", nameIndex, StringComparison.Ordinal); int openBrace = shimSource.IndexOf('{', nameIndex); diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadAppliedSourceLifecycle.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadAppliedSourceLifecycle.cs index 50527c031..c459cd783 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadAppliedSourceLifecycle.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadAppliedSourceLifecycle.cs @@ -56,13 +56,17 @@ internal static HotReloadUnchangedSourceDecision TryShortCircuitUnchangedApplied for (int index = 0; index < sortedLabels.Count; index++) { string label = sortedLabels[index]; - string reason = domain.IsActiveMember( - projectRelativePath, - label) - ? HotReloadConstants.AlreadyActiveAddedMemberReason - : HotReloadConstants.AlreadyActiveReason; + // Why the added member wins when the label also names a patch: a return-type + // change after an earlier body patch leaves both, and the patch then serves + // only the superseded signature, not the declaration this source holds. + HotReloadAddedMemberInfo addedMember = domain.FindAddedMember(projectRelativePath, label); outcomes.Add( - HotReloadMethodOutcome.AlreadyActive(label, assemblyResolvePath, reason)); + addedMember != null + ? HotReloadMethodOutcome.AlreadyActiveAddedMember(addedMember, assemblyResolvePath) + : HotReloadMethodOutcome.AlreadyActive( + label, + assemblyResolvePath, + HotReloadConstants.AlreadyActiveReason)); } return HotReloadUnchangedSourceDecision.ShortCircuited; diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadApplyResponseBuilder.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadApplyResponseBuilder.cs index c15d32224..1fa8c126d 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadApplyResponseBuilder.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadApplyResponseBuilder.cs @@ -49,12 +49,7 @@ public static HotReloadResponse Build( Method = outcome.Method, Reason = outcome.Reason ?? string.Empty, FilePath = outcome.FilePath ?? string.Empty, - // Why these two kinds: both describe a patch that was already installed - // before this run, so the ledger counter is the only invocation figure - // that means anything for them. - InvocationCount = ReadsInvocationCountFromLedger(outcome.Kind) - ? HotReloadInvocationRegistry.GetCount(outcome.Method) - : 0L, + InvocationCount = ReadInvocationCount(outcome), LifecycleNote = outcome.LifecycleNote ?? string.Empty, ReappliedFromSibling = reappliedSiblingFiles.Contains(outcome.FilePath) }); @@ -201,6 +196,22 @@ private static bool DecideAppendCompileResolution( && !HotReloadCompileFallbackDecider.HasUnappliedEdit(result, activePatchSiblingFiles); } + // Why read here and not when the row was made: a run awaits the worker and the compile + // after it decides a file is unchanged, and Play Mode frames keep calling in meanwhile. + private static long ReadInvocationCount(HotReloadMethodOutcome outcome) + { + if (outcome.AddedMember != null) + { + return outcome.AddedMember.ReadInvocationCount(); + } + + return ReadsInvocationCountFromLedger(outcome.Kind) + ? HotReloadInvocationRegistry.GetCount(outcome.Method) + : 0L; + } + + // Why these two kinds: both describe a patch that was already installed before this run, + // so the ledger counter is the only invocation figure that means anything for them. private static bool ReadsInvocationCountFromLedger(HotReloadMethodOutcomeKind kind) { return kind == HotReloadMethodOutcomeKind.AlreadyActive diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEntryResolution.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEntryResolution.cs index d4f59a4bf..152e90315 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEntryResolution.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEntryResolution.cs @@ -154,6 +154,13 @@ private static ResolvedEntryOutcome TryResolveAddedMethod( HotReloadMethodOutcome.Failed(methodLabel, shimError, filePath)); } + (FieldInfo invocationCounter, string counterError) = FindInvocationCounter(shimMethod); + if (invocationCounter == null) + { + return ResolvedEntryOutcome.Failed( + HotReloadMethodOutcome.Failed(methodLabel, counterError, filePath)); + } + return ResolvedEntryOutcome.Succeeded( new ResolvedEntry( entry, @@ -162,7 +169,32 @@ private static ResolvedEntryOutcome TryResolveAddedMethod( HotReloadPatchShape.Transplant, originalMethod: null, shimMethod, - isAddedMethod: true)); + isAddedMethod: true, + invocationCounter)); + } + + // Why a missing counter fails the file like a missing shim does: the counter is what + // --status reports as the member's InvocationCount, and a shim without it means the worker + // and this Editor disagree on the shape of an added-member shim. + private static (FieldInfo InvocationCounter, string ErrorMessage) FindInvocationCounter( + MethodInfo shimMethod) + { + Type shimType = shimMethod.DeclaringType; + string counterName = shimMethod.Name + HotReloadConstants.AddedMemberInvocationCounterSuffix; + FieldInfo counter = shimType.GetField( + counterName, + BindingFlags.Public | BindingFlags.Static | BindingFlags.DeclaredOnly); + if (counter == null) + { + return (null, "Invocation counter not found: " + shimType.Name + "." + counterName); + } + + if (!HotReloadAddedMemberInfo.IsReadableInvocationCounter(counter)) + { + return (null, "Invocation counter is not a static long: " + shimType.Name + "." + counterName); + } + + return (counter, null); } private static ResolvedEntryOutcome TryResolveExistingMethod( @@ -224,7 +256,8 @@ private static ResolvedEntryOutcome TryResolveExistingMethod( patchShape, matchResult.Method, shimMethod, - isAddedMethod: false)); + isAddedMethod: false, + invocationCounter: null)); } private static (MethodInfo ShimMethod, string ErrorMessage) FindShimMethod( @@ -301,6 +334,12 @@ internal sealed class ResolvedEntry public MethodInfo ShimMethod { get; } public bool IsAddedMethod { get; } + /// + /// The counter declared beside an added method's shim, or null for an entry that + /// patches an existing method. + /// + public FieldInfo InvocationCounter { get; } + public ResolvedEntry( TransformWorkerEntryDto entry, string methodLabel, @@ -308,7 +347,8 @@ public ResolvedEntry( HotReloadPatchShape patchShape, MethodBase originalMethod, MethodInfo shimMethod, - bool isAddedMethod) + bool isAddedMethod, + FieldInfo invocationCounter) { Entry = entry; MethodLabel = methodLabel; @@ -317,6 +357,7 @@ public ResolvedEntry( OriginalMethod = originalMethod; ShimMethod = shimMethod; IsAddedMethod = isAddedMethod; + InvocationCounter = invocationCounter; } } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadFileEntryApplier.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadFileEntryApplier.cs index 302397a92..b9e474964 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadFileEntryApplier.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadFileEntryApplier.cs @@ -408,6 +408,7 @@ private HotReloadMethodOutcome ApplyResolvedEntry( resolved.FilePath, resolved.Entry.methodName, resolved.Entry.typeMetadataName, + resolved.InvocationCounter, resolved.Entry.sourceStartLine, resolved.Entry.sourceEndLine); return HotReloadMethodOutcome.Added( diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadMethodOutcome.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadMethodOutcome.cs index b2bc54090..39769db49 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadMethodOutcome.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadMethodOutcome.cs @@ -1,3 +1,5 @@ +using System; + namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { /// @@ -14,13 +16,18 @@ internal sealed class HotReloadMethodOutcome // The facts Reason was built from when a worker reported it. Null for every other row. public HotReloadWorkerReasonFacts WorkerReason { get; } + // The added member an AlreadyActive row left in place, whose own counter is the row's + // InvocationCount. Null for every other row. + public HotReloadAddedMemberInfo AddedMember { get; } + private HotReloadMethodOutcome( HotReloadMethodOutcomeKind kind, string method, string reason, string filePath, string lifecycleNote, - HotReloadWorkerReasonFacts workerReason = null) + HotReloadWorkerReasonFacts workerReason = null, + HotReloadAddedMemberInfo addedMember = null) { Kind = kind; Method = method; @@ -28,6 +35,7 @@ private HotReloadMethodOutcome( FilePath = filePath; LifecycleNote = lifecycleNote ?? string.Empty; WorkerReason = workerReason; + AddedMember = addedMember; } public static HotReloadMethodOutcome Patched( @@ -91,6 +99,27 @@ public static HotReloadMethodOutcome AlreadyActive( string.Empty); } + // An added member left in place because its file's source is unchanged. Why the member and + // not its count: the ledger never counts an added member, and the response reads the + // member's own counter when it is built, as it reads the ledger for a patch's row. + public static HotReloadMethodOutcome AlreadyActiveAddedMember( + HotReloadAddedMemberInfo member, + string filePath) + { + if (member == null) + { + throw new ArgumentNullException(nameof(member)); + } + + return new HotReloadMethodOutcome( + HotReloadMethodOutcomeKind.AlreadyActive, + member.MethodKey, + HotReloadConstants.AlreadyActiveAddedMemberReason, + filePath, + string.Empty, + addedMember: member); + } + // A patch that outlived the method it replaced: the edited source no longer declares it, // so 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 reverts it. @@ -107,17 +136,17 @@ public static HotReloadMethodOutcome Stale(string method, string filePath) public HotReloadMethodOutcome WithLifecycleNote(string lifecycleNote) { - return new HotReloadMethodOutcome(Kind, Method, Reason, FilePath, lifecycleNote, WorkerReason); + return new HotReloadMethodOutcome(Kind, Method, Reason, FilePath, lifecycleNote, WorkerReason, AddedMember); } public HotReloadMethodOutcome WithReason(string reason) { - return new HotReloadMethodOutcome(Kind, Method, reason, FilePath, LifecycleNote, WorkerReason); + return new HotReloadMethodOutcome(Kind, Method, reason, FilePath, LifecycleNote, WorkerReason, AddedMember); } public HotReloadMethodOutcome WithWorkerReason(HotReloadWorkerReasonFacts workerReason) { - return new HotReloadMethodOutcome(Kind, Method, Reason, FilePath, LifecycleNote, workerReason); + return new HotReloadMethodOutcome(Kind, Method, Reason, FilePath, LifecycleNote, workerReason, AddedMember); } } } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStatusExecutor.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStatusExecutor.cs index ca059a2a6..b6fd78480 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStatusExecutor.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStatusExecutor.cs @@ -120,18 +120,13 @@ public HotReloadResponse ExecuteStatus() for (int index = 0; index < addedMembers.Count; index++) { - HotReloadAddedMemberInfo added = addedMembers[index]; - methods.Add( - new HotReloadMethodResult - { - Kind = HotReloadConstants.AddedMemberStatusKind, - Method = added.MethodKey, - FilePath = added.FilePath, - // Why: --status does not compare source, so the AlreadyActive first - // sentence would be a lie after a post-reload edit; only the - // not-instrumented fact is always true. - Reason = HotReloadConstants.AddedMemberNotInstrumentedReason - }); + HotReloadMethodResult row = BuildAddedMemberStatusRow(addedMembers[index]); + if (row.Reason == HotReloadConstants.AddedMemberNeverInvokedReason) + { + neverInvokedCount++; + } + + methods.Add(row); } int count = methods.Count; @@ -146,7 +141,7 @@ public HotReloadResponse ExecuteStatus() if (neverInvokedCount > 0) { message += " " + string.Format( - HotReloadConstants.NeverInvokedActiveAggregatedMessageFormat, + HotReloadConstants.NeverInvokedAggregatedMessageFormat, neverInvokedCount); } @@ -210,6 +205,24 @@ private string ResolveActiveStatusReason(string methodKey, long invocationCount) return string.Empty; } + // Why the count alone decides the Reason: --status does not compare source, so the + // AlreadyActive sentence could be false after a post-reload edit, and an added member has + // no compiled signature that a later declaration could supersede. + private static HotReloadMethodResult BuildAddedMemberStatusRow(HotReloadAddedMemberInfo added) + { + long invocationCount = added.ReadInvocationCount(); + return new HotReloadMethodResult + { + Kind = HotReloadConstants.AddedMemberStatusKind, + Method = added.MethodKey, + FilePath = added.FilePath, + InvocationCount = invocationCount, + Reason = invocationCount == 0L + ? HotReloadConstants.AddedMemberNeverInvokedReason + : string.Empty + }; + } + private void AppendAddedFieldStatusRows( List methods, IReadOnlyList addedFields) diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadTools.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadTools.cs index 393fa6847..263ef69a5 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadTools.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadTools.cs @@ -69,10 +69,10 @@ public class HotReloadMethodResult public string FilePath { get; set; } = string.Empty; /// - /// How many times this patched method body has run since the current patch was applied. - /// Populated on --status Active rows and AlreadyActive apply rows; 0 for other - /// apply/revert outcomes. Added-member AlreadyActive rows are always 0 because - /// added-member calls are not instrumented. + /// How many times this method's hot-reloaded body has run since it was applied. Populated + /// on --status Active and Added rows and on AlreadyActive and Stale apply rows, where an + /// added member's AlreadyActive row keeps that member's own count; 0 for other + /// apply/revert outcomes, including the Added rows of the run that applied them. /// public long InvocationCount { get; set; } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadAddedMemberInfo.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadAddedMemberInfo.cs index 42e1af287..2ee312480 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadAddedMemberInfo.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadAddedMemberInfo.cs @@ -1,3 +1,4 @@ +using System; using System.Reflection; using io.github.hatayama.UnityCliLoop.ToolContracts; @@ -31,6 +32,12 @@ internal sealed class HotReloadAddedMemberInfo public string DeclaringTypeMetadataName { get; } + /// + /// The static long field the shim increments each time its body starts, or null for a + /// member a test built without one. + /// + public FieldInfo InvocationCounter { get; } + public HotReloadAddedMemberInfo( string methodKey, string filePath, @@ -38,7 +45,8 @@ public HotReloadAddedMemberInfo( int sourceStartLine = 0, int sourceEndLine = 0, string methodName = null, - string declaringTypeMetadataName = null) + string declaringTypeMetadataName = null, + FieldInfo invocationCounter = null) { MethodKey = methodKey ?? string.Empty; FilePath = filePath ?? string.Empty; @@ -47,6 +55,33 @@ public HotReloadAddedMemberInfo( SourceEndLine = sourceEndLine; MethodName = methodName ?? string.Empty; DeclaringTypeMetadataName = declaringTypeMetadataName ?? string.Empty; + InvocationCounter = invocationCounter; + } + + /// + /// Whether a field can serve as an added member's invocation counter: a static long, the + /// shape the worker declares beside every added-member shim. + /// + internal static bool IsReadableInvocationCounter(FieldInfo field) + { + return field != null && field.IsStatic && field.FieldType == typeof(long); + } + + /// + /// Calls that started the shim's body since this member was registered, read when asked so + /// calls made after the registration are included. + /// + internal long ReadInvocationCount() + { + if (InvocationCounter == null) + { + throw new InvalidOperationException( + "The added member " + MethodKey + " was built without an invocation counter."); + } + + // Why a plain read of a field the shim increments with Interlocked: the Editor is + // 64-bit, where reading an aligned long never observes half of an increment. + return (long)InvocationCounter.GetValue(null); } /// diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadDomain.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadDomain.cs index 5387b011b..40399db34 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadDomain.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadDomain.cs @@ -310,6 +310,18 @@ internal bool IsActiveMember(string projectRelativePath, string methodKey) return generation != null && generation.IsActiveMember(methodKey); } + /// + /// The added member the file's generation registered under the key, or null when the file + /// has no generation or its generation registered none. Keyed by path for the reason + /// IsActiveMember is. + /// + internal HotReloadAddedMemberInfo FindAddedMember(string projectRelativePath, string methodKey) + { + Debug.Assert(!string.IsNullOrEmpty(projectRelativePath), "projectRelativePath must not be empty."); + Debug.Assert(!string.IsNullOrEmpty(methodKey), "methodKey must not be empty."); + return FindGeneration(projectRelativePath)?.FindAddedMember(methodKey); + } + /// /// The live patches this domain holds on the methods one assembly declares on one type, /// which is what a run peels when an edited body matches that assembly's own code again. diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadFileGeneration.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadFileGeneration.cs index d7837b28a..4d4cc3c47 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadFileGeneration.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadFileGeneration.cs @@ -138,6 +138,7 @@ internal void RegisterAddedMethod( string filePath, string methodName, string declaringTypeMetadataName, + FieldInfo invocationCounter, int sourceStartLine = 0, int sourceEndLine = 0) { @@ -149,6 +150,15 @@ internal void RegisterAddedMethod( "An added method is registered with its own name and its declaring type's metadata name."); } + // Why checked in every build: --status reads this counter as the member's + // InvocationCount, so a member registered without one would fail there instead. + if (!HotReloadAddedMemberInfo.IsReadableInvocationCounter(invocationCounter)) + { + throw new ArgumentException( + "An added method is registered with the static long invocation counter beside its shim.", + nameof(invocationCounter)); + } + if (!HasAddedMemberGeneration) { throw new InvalidOperationException( @@ -163,7 +173,8 @@ internal void RegisterAddedMethod( sourceStartLine, sourceEndLine, methodName, - declaringTypeMetadataName); + declaringTypeMetadataName, + invocationCounter); } /// @@ -459,6 +470,18 @@ internal IReadOnlyList ListActiveAddedMethodKeys() return new List(_addedMembersByMethodKey.Keys); } + /// + /// The added member this generation registered under the key, or null when it registered + /// none. + /// + internal HotReloadAddedMemberInfo FindAddedMember(string methodKey) + { + Debug.Assert(!string.IsNullOrEmpty(methodKey), "methodKey must not be empty."); + return _addedMembersByMethodKey.TryGetValue(methodKey, out HotReloadAddedMemberInfo member) + ? member + : null; + } + /// /// The added method whose source range holds the 1-based line, or null when no added method /// of this file covers it. diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadConstants.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadConstants.cs index 4c98d8c82..b46c7ddf4 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadConstants.cs @@ -157,6 +157,12 @@ internal static class HotReloadConstants // in TransformWorker~/PatchKinds.cs. public const string PatchKindAddedMethod = "addedMethod"; + // Appended to an added-method entry's shimMethodName to name the static long counter the + // worker declares beside the shim and increments each time the shim's body starts. Keep in + // sync with TransformWorkerProgramMarker.AddedMemberInvocationCounterSuffix in + // TransformWorker~/TransformWorkerProgramMarker.cs. + public const string AddedMemberInvocationCounterSuffix = "__uloopCalls"; + // --status Kind for rows sourced from a generation's added members (no compiled MethodBase). public const string AddedMemberStatusKind = "Added"; @@ -509,12 +515,15 @@ public static bool IsPublicizableProjectAssemblyFileName(string fileNameWithoutE "This declaration is bound from an assembly an earlier hot reload retained, so this " + "reload introduced nothing for it. It stays loaded until the next Domain Reload."; - public const string AddedMemberNotInstrumentedReason = - "Added-member calls are not instrumented, so InvocationCount is always 0 for this row."; - public const string AlreadyActiveAddedMemberReason = - "Source is unchanged since the last applied hot reload; the existing added member stays available. " - + AddedMemberNotInstrumentedReason; + "Source is unchanged since the last applied hot reload; the existing added member stays " + + "available and keeps its InvocationCount. Edit and reload again to apply new changes."; + + // Why the Reason names the callers: no compiled call site can reach an added member, so an + // InvocationCount of 0 is expected until a hot-reloaded body calls it, and the row has to + // say where a call can come from. + public const string AddedMemberNeverInvokedReason = + "Not invoked since this added member was applied. Compiled code cannot call a member that hot reload added, so it runs only when a hot-reloaded body that calls it runs, or, for a forwarded Unity message, when the hot-reload proxy delivers the message in Play Mode."; // Why: patching does not re-run calls that already finished (e.g. one-time // initialization); InvocationCount 0 on --status is the only runtime signal, @@ -529,9 +538,10 @@ public static bool IsPublicizableProjectAssemblyFileName(string fileNameWithoutE public const string ActivePatchSupersededReasonFormat = "Superseded by a new declaration of {0}: the edited source now declares a different signature. This compiled signature stays patched so existing callers keep working; it is not the entry point for new calls."; - // Format: count of Active rows whose InvocationCount is 0. - public const string NeverInvokedActiveAggregatedMessageFormat = - "{0} change(s) have not been invoked since their patch was applied; see Methods[].Reason."; + // Format: count of Active and Added rows whose Reason says they have not run since they + // were applied. + public const string NeverInvokedAggregatedMessageFormat = + "{0} change(s) have not been invoked since they were applied; see Methods[].Reason."; public const string MultiWarningSingleCompileResolutionMessage = "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."; diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/SKILL.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/SKILL.md index 5a341c1fb..1cc6aa834 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/SKILL.md +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/SKILL.md @@ -44,8 +44,8 @@ automatically — pass it with `--files`. `uloop hot-reload --status` lists the currently active changes; it cannot be combined with `--files` or `--revert-all`. Every change is static Editor state, so after a domain reload -it reports zero. Each `Active` row's `InvocationCount` counts calls -into the patched body since it was applied — a reachability signal only while the code is +it reports zero. Each `Active`/`Added` row's `InvocationCount` counts calls +into its body since it was applied — a reachability signal only while the code is being driven. ## How It Works diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/mechanism-and-lifecycle.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/mechanism-and-lifecycle.md index e8e2978e8..60d403d48 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/mechanism-and-lifecycle.md +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/mechanism-and-lifecycle.md @@ -10,8 +10,8 @@ Re-running on the same method after a real edit replaces its previous patch; `ActivePatchTotal` tracks the ledger across runs. Reloading a file whose source is unchanged since the last fully applied reload (a run with no Skipped or Failed -outcomes) is a no-op: each still-active method is reported -as `AlreadyActive`, the existing patch stays in place, and the row carries the live `InvocationCount`. +outcomes) is a no-op: each still-active method or added member is reported +as `AlreadyActive`, the existing patch or added member stays in place, and the row carries its live `InvocationCount`. When another edited file of the same assembly is in the reload, that unchanged file is re-applied with the group instead, and other files of the assembly that hold active patches and are unchanged since they were applied are re-applied too, so every active diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md index 72a3dec2f..6185e28a8 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md @@ -5,7 +5,7 @@ Returns JSON with: - `Success` (boolean): `false` on parameter validation failure or when any method outcome is `Failed`, or when any `IntroducedTypes` row is `Failed`. `Skipped` outcomes alone never force `false` - `ErrorCode` (string, optional): Present on parameter validation failure. Values are `HOT_RELOAD_FILES_REQUIRED` when an omitted apply has no compile snapshots, `HOT_RELOAD_NO_CHANGED_FILES` when snapshots contain no changed `.cs` files, `HOT_RELOAD_INVALID_FILES` when `--files` contains a null or empty path, and `HOT_RELOAD_STATUS_CONFLICT` when `--status` is combined with `--files` or `--revert-all`. - `NextActions` (array, optional): Ordered recovery steps, present only with `ErrorCode` on a parameter validation failure. Omitted from every other response, including successful apply, plain `--status`, and `--revert-all` runs. -- `Methods` (array): Per-method `{ Kind, Method, Reason, FilePath, InvocationCount, LifecycleNote, ReappliedFromSibling }` where `Kind` is `Patched`, `Skipped`, `Failed`, `Added`, `AlreadyActive`, or `Stale` on apply runs, and `Active`, `Added`, or `AddedField` on `--status` runs; empty on `--revert-all` runs. `AlreadyActive` means this file's source matched the last fully applied reload (a run with no Skipped or Failed outcomes), so the existing patch was left in place and the row carries the live `InvocationCount`. `Stale` means the method was deleted from the edited source while its patch is still installed: compiled callers keep running the patched body until `uloop compile`, `--revert-all`, or a later reload whose source restores the method to the compiled baseline clears it; a reload that declares the method with a different body replaces the patch instead of clearing it. Stale rows keep counting toward `ActivePatchTotal`, and the Message summary includes `Stale=N`. `InvocationCount` is meaningful on `Active` rows and on `AlreadyActive` and `Stale` apply rows (calls since the current patch was applied); it is `0` on other apply/revert outcomes. On `--status`, an `Active` row with `InvocationCount` 0 sets `Reason` to explain that the method has not run since the patch: finished calls do not re-run, the patched body takes effect on the next call, and how to retrigger an initialization-only path. When the edited source later declares a different signature, that `Active` row's `Reason` instead explains it is superseded by a new declaration of that signature and is no longer the entry point for new calls (superseded wins over the never-invoked sentence). Added-member rows always show InvocationCount 0 — added-member calls are not instrumented, and the row's Reason says so. `AddedField` rows list a live added field as `Type.field` with an empty `Reason`; they are not method patches. `LifecycleNote` is set when a patched method is a Unity one-shot lifecycle message (`private void Awake`/`Start`/`OnEnable`/`OnDisable`/`OnDestroy` on a `MonoBehaviour`), or when every compiled call path into the patched method (callers of callers are followed a few levels within the compiled assemblies) starts at such a message; empty otherwise — it does not change `Kind`. On an `Added` row whose method is a Unity message, `LifecycleNote` instead says whether the engine will reach it: a forwarded message (`Start`, `Update`, the collision/trigger/mouse messages, and the rest listed in [scope-and-limits.md](scope-and-limits.md)) carries the note that a hot-reload proxy component delivers it to live instances while Play Mode runs, that the proxy is rebuilt only when a later reload changes which messages the type adds or their signatures, that execution order relative to other components is not guaranteed, and that it is gone on any compile or domain reload (only an added `Start` row also says that it runs once on each existing instance when the proxy attaches, and again when the proxy is rebuilt); a message this feature leaves to the compiler (`Awake`, `OnEnable`, `OnDisable`, `OnDestroy`, the editor-only messages, and any non-void message) carries the note that the engine does not invoke it until `uloop compile`, and the run adds one `Warnings` line naming every such message together. `Added` rows carry the added member's signature and file; their `InvocationCount` is always `0` (added-member calls are not instrumented). `ReappliedFromSibling` is `true` on every apply row, whatever its `Kind`, that belongs to a sibling file the run pulled in to re-apply changes from earlier reloads rather than to a file passed in `--files`; it is `false` on the other rows and on every `--status` and `--revert-all` row. Message's re-applied count covers only the `Patched` and `Added` rows among them. `Method` spells parameter types as .NET metadata does, the same on every method row: a constructed generic as ``System.Collections.Generic.List`1``, a multidimensional array as `System.Int32[0...,0...]`, and a nested type with `+`. Example `--status` row: `{ "Kind": "Added", "Method": "Ns.Host.NewHelper(System.Int32)", "Reason": "Added-member calls are not instrumented, so InvocationCount is always 0 for this row.", "FilePath": "Assets/Scripts/Host.cs", "InvocationCount": 0, "LifecycleNote": "", "ReappliedFromSibling": false }` +- `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. - `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. @@ -20,7 +20,7 @@ Returns JSON with: - `ClearedCount` (number): Patches removed by `--revert-all`, or stale patches reverted because their source matched the compiled baseline again - `IntroducedTypes` (array): Per-type `{ Kind, TypeName, AssemblyName, FilePath, Reason }` rows for the type declarations a reload met, always present and empty when there are none. On apply runs `Kind` is `Introduced` (this reload compiled the declaration into a retained assembly and it is now loaded), `AlreadyActive` (the declaration is bound from an assembly an earlier reload retained, so this reload introduced nothing for it), or `Failed` (the declaration was refused — a redefinition of a type already active, the same type declared in more than one file of the group, or a failed artifact compilation; `Reason` says which, and a `Failed` row alone makes `Success` false). On `--status` every row is `Kind` `Active` and lists a type this domain still holds. Declarations a reload simply cannot introduce are reported as `Warnings`, not rows. These rows are never counted in `PatchedTotal`, `ActivePatchTotal`, `AddedFieldTotal`, or `ClearedCount` - `ActiveIntroducedTypeTotal` (number): Introduced types this domain holds after this run or on `--status`, counted per type rather than per compiled artifact; always present and `0` when there is none. `--revert-all` cannot unload them, so its Message says how many stay loaded until the next Domain Reload, and that Auto Refresh stays held for them until `uloop compile` -- `Message` (string): Short summary. When a run carries `IntroducedTypes` rows, Message reports them: a run that only introduced or only re-bound types says so instead of reporting the methods, a refused declaration is reported as the failure of the run and points at `IntroducedTypes`, and a run the methods decided ends with `IntroducedTypes=N`. On apply runs that pulled in sibling files, Message follows `PatchedTotal` and `Added` with how many of those Patched and Added rows re-applied the siblings' earlier changes (left out when 0). On apply runs, Message counts the patched rows that carry a `LifecycleNote` in one sentence and the added Unity messages a hot-reload proxy delivers in another, both pointing at `Methods[].LifecycleNote`; forwarded `Added` rows are not in the patched count, and each sentence is left out when its count is 0. On `--status`, Message opens with how many changes are currently active — patched methods, added members, and introduced types together, which is why it can exceed `ActivePatchTotal` — and when any `Active` row has `InvocationCount` 0 it also appends how many such rows there are and points at `Methods[].Reason`; added-member rows are not included in that count. `--revert-all` appends how many introduced types stay loaded until the next Domain Reload, and when the hold is still armed for them, that Auto Refresh stays held until `uloop compile` +- `Message` (string): Short summary. When a run carries `IntroducedTypes` rows, Message reports them: a run that only introduced or only re-bound types says so instead of reporting the methods, a refused declaration is reported as the failure of the run and points at `IntroducedTypes`, and a run the methods decided ends with `IntroducedTypes=N`. On apply runs that pulled in sibling files, Message follows `PatchedTotal` and `Added` with how many of those Patched and Added rows re-applied the siblings' earlier changes (left out when 0). On apply runs, Message counts the patched rows that carry a `LifecycleNote` in one sentence and the added Unity messages a hot-reload proxy delivers in another, both pointing at `Methods[].LifecycleNote`; forwarded `Added` rows are not in the patched count, and each sentence is left out when its count is 0. On `--status`, Message opens with how many changes are currently active — patched methods, added members, and introduced types together, which is why it can exceed `ActivePatchTotal` — and when any `Active` or `Added` row has `InvocationCount` 0 it also appends how many such rows there are and points at `Methods[].Reason`. `--revert-all` appends how many introduced types stay loaded until the next Domain Reload, and when the hold is still armed for them, that Auto Refresh stays held until `uloop compile` - `RecommendedNextAction` (string): Present in three cases. (1) Any method or introduced-type outcome is `Failed`: a partial apply (some methods patched or added, or some types left active) says to fix and rerun, run `uloop compile`, or `uloop hot-reload --revert-all`; a failure with nothing applied says to fix and rerun or compile. (2) Every method of the requested files was `Skipped`, which still answers `Success`: it points first at the fix each Skipped row's `Methods[].Reason` names and offers `uloop compile` as the alternative. (3) `CompileFallback` is `HeldForPlayMode` or `BlockedByPlayModeSetting`, whatever the outcomes: the reason no compile ran is appended after any advice from (1) or (2), and it opens by saying to do any fix a `Reason` names that needs no compile before compiling. Omitted otherwise. - `CompileFallback` (string, always present): whether the CLI should run a compile after this run — `NotNeeded`, `Requested`, `HeldForPlayMode` (edits stayed unapplied but the Editor is in Play Mode and `--compile-on-skip` is `auto`), `BlockedByPlayModeSetting` (`--compile-on-skip on` during Play Mode while Unity's "Script Changes While Playing" is "Recompile After Finished Playing", which refuses the compile; `RecommendedNextAction` says to stop Play Mode first), or `Disabled` (`--compile-on-skip off`). `--status`, `--revert-all` and validation failures answer `NotNeeded`. `Skipped` rows of a sibling pulled in to re-bind its active patches do not count as unapplied edits: they are not this run's edits, and their earlier patches stay active. Its `Failed` rows do count, because a failed reload reverts those patches. A sibling retried after an earlier Skip, or brought in as a companion, still counts. - `Compile` (object, present only when the CLI ran the fallback compile): the full `uloop compile` response; the top-level `Success` is then the compile's, and the command's exit code is the compile's. A successful compile drops `RecommendedNextAction` and ends `Message` with a sentence saying the compile succeeded; a failed one sets `RecommendedNextAction` to the compile's own `NextActions` when it reports any, and otherwise to fixing `Compile.Errors`. diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/troubleshooting.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/troubleshooting.md index 512f2b676..fb919684c 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/troubleshooting.md +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/troubleshooting.md @@ -2,9 +2,9 @@ ## Reading `--status` and `InvocationCount` -`uloop hot-reload --status` lists the methods whose bodies are currently replaced, -without applying or reverting anything. It cannot be combined with `--files` or -`--revert-all`. Patches are static Editor state, so the answer is authoritative: after +`uloop hot-reload --status` lists the methods whose bodies are currently replaced and the +members hot reload added, without applying or reverting anything. It cannot be combined +with `--files` or `--revert-all`. Patches are static Editor state, so the answer is authoritative: after a domain reload it reports zero patched methods, which is exactly when an `ActivePatchTotal` remembered from an earlier response has gone stale. @@ -12,9 +12,19 @@ Each `Active` row's `InvocationCount` counts calls into the patched body since t was applied. Reloading the same source with no edits after a fully applied reload (a run with no Skipped or Failed outcomes) reports `AlreadyActive` and the row carries the live `InvocationCount`, unless another edited file of the same assembly is in the -reload — then the unchanged file is re-applied with that group; re-running after a real edit replaces the patch and resets it to zero. When `InvocationCount` is 0 on an `Active` row, `Reason` notes that the method has not run since this patch was applied: calls that already finished do not re-run, and the patched body takes effect the next time this method is called. For initialization-only methods it also names how to trigger that next call. While Unity is -paused — including while a pause-point hit holds the game — the player loop does not -advance, so game-driven calls stop and the count freezes; calls you make yourself (for +reload — then the unchanged file is re-applied with that group; re-running after a real edit replaces the patch and resets it to zero. When `InvocationCount` is 0 on an `Active` row, `Reason` notes that the method has not run since this patch was applied: calls that already finished do not re-run, and the patched body takes effect the next time this method is called. For initialization-only methods it also names how to trigger that next call. + +Each `Added` row's `InvocationCount` counts calls into the added member's body the same +way, from a counter of the member's own: an unchanged reload's `AlreadyActive` row for it +carries that count, and re-applying the member — by editing it, or by re-applying its file +with an edited file of the same assembly — starts the count over at zero. Compiled code +cannot call an added member, so the count stays 0 until a hot-reloaded body that calls it +runs, or, for a forwarded Unity message, until the hot-reload proxy delivers the message in +Play Mode; `Reason` says so while the count is 0. An added iterator counts when its +enumeration starts rather than when it is called. + +While Unity is paused — including while a pause-point hit holds the game — the player loop +does not advance, so game-driven calls stop and the count freezes; calls you make yourself (for example through `uloop execute-dynamic-code`) still increment it. A frozen count during a pause only means game-driven calls are not running; it says nothing about whether call sites reach the patch. Resume first diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/AddedPropertyEmitter.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/AddedPropertyEmitter.cs index f392d86f5..9fe7700b9 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/AddedPropertyEmitter.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/AddedPropertyEmitter.cs @@ -69,10 +69,11 @@ private static void EmitStoreBackedAccessor( MethodDeclarationSyntax method = isGetter ? BuildStoreGetter(binding, accessor) : BuildStoreSetter(binding, accessor); - method = ShimMethodFactory.ToShimMethod( - method, - isGetter ? binding.Symbol.GetMethod : binding.Symbol.SetMethod); - typeState.CurrentShimType.AddMethod(method, accessor.ShimMethodName); + IMethodSymbol accessorSymbol = isGetter ? binding.Symbol.GetMethod : binding.Symbol.SetMethod; + method = ShimMethodFactory.ToShimMethod(method, accessorSymbol); + // Why the receiver check here too, though the store already refuses a null instance: the + // check must come before the counter, or a null-receiver call would count as a run. + typeState.CurrentShimType.AddAddedMemberMethod(method, accessor.ShimMethodName, accessorSymbol); entries.Add(CreateAccessorEntry(typeState, binding, accessor, Array.Empty())); } @@ -171,7 +172,10 @@ private static void EmitAccessor( addedPropertyCatalog, addedMethodCatalog, addedFieldCatalog); - typeState.CurrentShimType.AddMethod(shimMethod, accessor.ShimMethodName); + typeState.CurrentShimType.AddAddedMemberMethod( + shimMethod, + accessor.ShimMethodName, + isGetter ? binding.Symbol.GetMethod : binding.Symbol.SetMethod); entries.Add(CreateAccessorEntry( typeState, binding, @@ -245,9 +249,7 @@ private static MethodDeclarationSyntax BuildAccessorShim( .WithParameterList(isGetter ? SyntaxFactory.ParameterList() : CreateSetterParameterList(binding)); method = ApplyBody(method, rewrittenBody); IMethodSymbol accessorSymbol = isGetter ? binding.Symbol.GetMethod : binding.Symbol.SetMethod; - return ShimMethodFactory.GuardAddedMemberReceiver( - ShimMethodFactory.ToShimMethod(method, accessorSymbol), - accessorSymbol); + return ShimMethodFactory.ToShimMethod(method, accessorSymbol); } private static ParameterListSyntax CreateSetterParameterList(AddedPropertyBinding binding) diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimMethodEmitter.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimMethodEmitter.cs index 5ce9e6e2a..dc71b5f35 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimMethodEmitter.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimMethodEmitter.cs @@ -83,10 +83,12 @@ internal static void EmitQueuedMethods( addedPropertyCatalog); if (queued.IsAddedMethod) { - rewrittenMethod = ShimMethodFactory.GuardAddedMemberReceiver(rewrittenMethod, queued.MethodSymbol); + queued.ShimType.AddAddedMemberMethod(rewrittenMethod, queued.ShimMethodName, queued.MethodSymbol); + } + else + { + queued.ShimType.AddMethod(rewrittenMethod, queued.ShimMethodName); } - - queued.ShimType.AddMethod(rewrittenMethod, queued.ShimMethodName); SyntaxNode bodyNode = (SyntaxNode)queued.MethodDeclaration.Body ?? queued.MethodDeclaration.ExpressionBody; diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimMethodFactory.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimMethodFactory.cs index ff884d179..b988ae978 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimMethodFactory.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimMethodFactory.cs @@ -51,28 +51,27 @@ public static MethodDeclarationSyntax ToShimMethod( } /// - /// Makes the shim of an instance member hot reload added throw NullReferenceException on a - /// null receiver before its body runs, the way a call to a compiled member does. + /// Prepends the statements every shim of a member hot reload added starts with: for an + /// instance member, a check that throws NullReferenceException on a null receiver before the + /// body runs, the way a call to a compiled member does; then the increment of the shim's own + /// invocation counter, which the Editor reports as the member's InvocationCount. /// /// - /// Why in the shim and not at the call site: compiled code evaluates the arguments before the - /// null receiver throws, and a check at the call site would throw before them. Why the object - /// cast: a UnityEngine.Object receiver would otherwise use Unity's == and also refuse a - /// destroyed object, which a compiled call still reaches. An async or iterator shim throws - /// when its body starts rather than at the call, which is still closer than running it. + /// Why the receiver check is in the shim and not at the call site: compiled code evaluates + /// the arguments before the null receiver throws, and a check at the call site would throw + /// before them. Why the object cast: a UnityEngine.Object receiver would otherwise use + /// Unity's == and also refuse a destroyed object, which a compiled call still reaches. An + /// async or iterator shim throws when its body starts rather than at the call, which is still + /// closer than running it. Why the increment follows the check: a call the check refuses + /// never started the body. An iterator shim therefore counts when enumeration starts rather + /// than at the call, because its whole body waits for the first MoveNext. /// - public static MethodDeclarationSyntax GuardAddedMemberReceiver( + public static MethodDeclarationSyntax PrependAddedMemberPreamble( MethodDeclarationSyntax shim, - IMethodSymbol methodSymbol) + IMethodSymbol methodSymbol, + string invocationCounterFieldName) { - if (methodSymbol.IsStatic || methodSymbol.ContainingType.IsValueType) - { - return shim; - } - - StatementSyntax guard = SyntaxFactory.ParseStatement( - "if ((object)" + TransformWorkerProgramMarker.InstanceParameterName - + " == null) throw new global::System.NullReferenceException();"); + bool checksReceiver = !methodSymbol.IsStatic && !methodSymbol.ContainingType.IsValueType; if (shim.Body != null) { // Why the body as a fallback: a block-bodied accessor shim is built without a @@ -81,7 +80,15 @@ public static MethodDeclarationSyntax GuardAddedMemberReceiver( ? (SyntaxNode)shim : shim.Body; return shim.WithBody(shim.Body.WithStatements( - shim.Body.Statements.Insert(0, MapGuardToLine(guard, blockLineSource)))); + shim.Body.Statements.InsertRange( + 0, + BuildPreamble(checksReceiver, invocationCounterFieldName, blockLineSource)))); + } + + if (shim.ExpressionBody == null) + { + throw new InvalidOperationException( + "An added-member shim must have a block or expression body: " + shim.Identifier.ValueText); } ArrowExpressionClauseSyntax arrow = shim.ExpressionBody; @@ -89,15 +96,39 @@ public static MethodDeclarationSyntax GuardAddedMemberReceiver( // Why the annotations move to the statement: the #line mapping is injected from them. An // accessor arrow carries its own, which does not survive the change to a block; an // expression-bodied method carries the expression's line on the declaration instead, - // where it would now map the '{' and guard lines rather than the expression. + // where it would now map the '{' and preamble lines rather than the expression. SyntaxNode lineSource = arrow.HasAnnotations(TransformWorkerProgram.UloopLineAnnotationKind) ? (SyntaxNode)arrow : shim; bodyStatement = (StatementSyntax)PropertyGetterEmitter.TransferUloopLineAnnotations(lineSource, bodyStatement); + List statements = BuildPreamble(checksReceiver, invocationCounterFieldName, lineSource); + statements.Add(bodyStatement); return shim .WithExpressionBody(null) .WithSemicolonToken(default) - .WithBody(SyntaxFactory.Block(MapGuardToLine(guard, lineSource), bodyStatement)); + .WithBody(SyntaxFactory.Block(statements)); + } + + // Why the increment is mapped as well, though it never throws: unmapped, it would continue the + // mapping of the '{' before it and take the line of the body's first statement. + private static List BuildPreamble( + bool checksReceiver, + string invocationCounterFieldName, + SyntaxNode lineSource) + { + List preamble = new List(2); + if (checksReceiver) + { + StatementSyntax guard = SyntaxFactory.ParseStatement( + "if ((object)" + TransformWorkerProgramMarker.InstanceParameterName + + " == null) throw new global::System.NullReferenceException();"); + preamble.Add(MapGuardToLine(guard, lineSource)); + } + + StatementSyntax increment = SyntaxFactory.ParseStatement( + "global::System.Threading.Interlocked.Increment(ref " + invocationCounterFieldName + ");"); + preamble.Add((StatementSyntax)PropertyGetterEmitter.TransferUloopLineAnnotations(lineSource, increment)); + return preamble; } // Why mapped at all: an unannotated guard continues the mapping before it, which is the '{' diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimTypeBuilder.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimTypeBuilder.cs index fa86807d1..c9efbb2f9 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimTypeBuilder.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimTypeBuilder.cs @@ -18,6 +18,7 @@ internal sealed class ShimTypeBuilder { private readonly List _methods = new List(); + private readonly List _invocationCounterFieldNames = new List(); public ShimTypeBuilder( string shimTypeName, @@ -54,6 +55,22 @@ public void AddMethod(MethodDeclarationSyntax shimMethod, string shimMethodName) _methods.Add(named); } + /// + /// Adds the shim of a member hot reload added together with the static field that counts how + /// often its body starts, so no added-member shim can be emitted without its counter. + /// + public void AddAddedMemberMethod( + MethodDeclarationSyntax shimMethod, + string shimMethodName, + IMethodSymbol methodSymbol) + { + string counterFieldName = shimMethodName + TransformWorkerProgramMarker.AddedMemberInvocationCounterSuffix; + AddMethod( + ShimMethodFactory.PrependAddedMemberPreamble(shimMethod, methodSymbol, counterFieldName), + shimMethodName); + _invocationCounterFieldNames.Add(counterFieldName); + } + public IEnumerable EmitMembers() { foreach (AccessorEntry accessor in AccessorPlan.Entries) @@ -61,6 +78,11 @@ public IEnumerable EmitMembers() yield return accessor.EmitFieldDeclaration(); } + foreach (string counterFieldName in _invocationCounterFieldNames) + { + yield return EmitInvocationCounterField(counterFieldName); + } + if (AccessorPlan.Entries.Count > 0) { yield return EmitBindAccessorsMethod(); @@ -89,4 +111,18 @@ private MethodDeclarationSyntax EmitBindAccessorsMethod() SyntaxFactory.Token(SyntaxKind.StaticKeyword))) .WithBody(SyntaxFactory.Block(statements)); } + + private static FieldDeclarationSyntax EmitInvocationCounterField(string counterFieldName) + { + return SyntaxFactory.FieldDeclaration( + SyntaxFactory.VariableDeclaration( + SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.LongKeyword))) + .WithVariables( + SyntaxFactory.SingletonSeparatedList( + SyntaxFactory.VariableDeclarator(counterFieldName)))) + .WithModifiers( + SyntaxFactory.TokenList( + SyntaxFactory.Token(SyntaxKind.PublicKeyword), + SyntaxFactory.Token(SyntaxKind.StaticKeyword))); + } } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/TransformWorkerProgramMarker.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/TransformWorkerProgramMarker.cs index 71a3e0394..f5ab61cac 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/TransformWorkerProgramMarker.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/TransformWorkerProgramMarker.cs @@ -40,4 +40,9 @@ internal static class TransformWorkerProgramMarker // Keep in sync with HotReloadAddedFieldStore.FieldKeySeparator. public const string AddedFieldKeySeparator = "::"; + + // Keep in sync with HotReloadConstants.AddedMemberInvocationCounterSuffix, which the Editor + // appends to an entry's shimMethodName to find the counter. Why a suffix of the shim method + // name: shim method names are unique across a run, so every counter name is too. + public const string AddedMemberInvocationCounterSuffix = "__uloopCalls"; }