DOCS: Move hardcoded code examples to testable files - #2472
ChoshikaBagratee wants to merge 25 commits into
Conversation
|
|
Corrected file path syntax in documentation and improved performance tip.
…ithub.com/Unity-Technologies/InputSystem into docatt-11021-move-hardcoded-code-examples
ChoshikaBagratee
changed the title
![u-pr[bot]](https://avatars.githubusercontent.com/in/1201017?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Harness Review
The migration consistently redirects the affected manual pages to external source regions, but several of the newly added samples cannot be compiled together in the existing documentation-samples assembly.
Reviewed commit 8a42ed2
🤖 Helpful? 👍/👎
| using UnityEngine; | ||
| using UnityEngine.InputSystem; | ||
|
|
||
| class BindingConflictsExample : InputTestFixture |
There was a problem hiding this comment.
InputTestFixture is not provided by this assembly's only direct reference (Unity.InputSystem): its declaration belongs to the separate Unity.InputSystem.TestFramework assembly. Consequently this sample produces an unresolved-type compiler error in Unity.InputSystem.DocCodeSamples. Either put this test-fixture example in a test assembly that references the framework (with the appropriate test constraints) or avoid using the fixture in this compiled sample.
🤖 Helpful? 👍/👎
| using UnityEngine.InputSystem; | ||
| using UnityEngine.InputSystem.Interactions; | ||
|
|
||
| public class ExampleScript : MonoBehaviour |
There was a problem hiding this comment.
These sources are all compiled into one documentation-samples assembly, so this declares DocCodeSamples.Tests.ExampleScript a second time—the same fully qualified type is already declared in ConfigureInputfromCode.cs. The same change also introduces a duplicate global MyPlayerScript between GenerateCsApiFromActions.cs and UsingDirectWorkflow.cs. C# rejects both duplicate declarations, preventing the entire samples assembly from compiling; give the snippets distinct types or namespaces.
🤖 Helpful? 👍/👎
| // you change the default values for the action map, the name of the interface | ||
| // will be different. | ||
|
|
||
| public class MyPlayerScript : MonoBehaviour, IGameplayActions |
There was a problem hiding this comment.
The sample assembly contains neither IGameplayActions nor MyPlayerControls; the latter is only described in the comments as code the user would generate. This makes the relocated sample fail to compile (and generated callback interfaces are nested under their generated wrapper type). Add an appropriate generated test fixture/stub and implement the nested interface, or exclude this intentionally incomplete snippet from the compiled sample assembly.
🤖 Helpful? 👍/👎
| // controls (For example, "<Gamepad>/<Button>"). | ||
| // NOTE: The unconstrained InputStateHistory class can record changes on controls | ||
| // of different value types. | ||
| var history = new InputStateHistory<Vector2>("<Touchscreen>/primaryTouch/position"); |
There was a problem hiding this comment.
InputStateHistory<TValue> is declared in UnityEngine.InputSystem.LowLevel, but this file imports only UnityEngine.InputSystem. The two examples therefore cannot resolve InputStateHistory and make the samples assembly fail to compile. Add the LowLevel using (or fully qualify the type).
🤖 Helpful? 👍/👎
…T/BurstCacheManifest.cm
Updated the changelog to include a fix for the Input Debugger window resizing issue.
|
Used Claude to create https://github.cds.internal.unity3d.com/unity/unity/pull/123975/files |
|
@u-pr review |
PR Review SummaryThis pull request migrates inline code snippets from documentation markdown files into compilable C# test sample files ( Key areas to address:
📚 Additional Context
🤖 Helpful? 👍/👎 |
| } | ||
|
|
||
| ``` | ||
| [!code-cs[customDeviceClass](Packages/com.unity.inputsystem/DocCodeSamples.Tests/HidCreateCustomLayoutClass.cs#customDeviceClass)] |
There was a problem hiding this comment.
Only the first code snippet in this file was replaced with [!code-cs[customDeviceClass]...]. The remaining code blocks in hid-create-custom-layout-class.md (device registration, DualShock4HIDInputReport, and DualShock4GamepadHID definitions) are still inline markdown code blocks, even though corresponding regions (registerMyDevice, dualShock4HidInputReport, dualShock4GamepadHidBasic, dualShock4GamepadHidRegister) have already been set up in HidCreateCustomLayoutClass.cs.
Should the remaining code blocks in this file also be replaced with their DocFX snippet tags?
📚 Additional Context
Packages/com.unity.inputsystem/Documentation~/hid-create-custom-layout-class.md
🤖 Helpful? 👍/👎
| internal class HidCreateCustomLayoutExisting | ||
| { | ||
| const string myDeviceJson = @" | ||
| #region myDeviceJson |
There was a problem hiding this comment.
Because this JSON is defined inside a C# verbatim string literal (@"..."), all internal quotes are escaped as "" (e.g. ""name"" : ""MyDevice""). When DocFX extracts this region, the rendered snippet in hid-create-custom-layout-existing.md will contain doubled quotes and invalid JSON syntax.
Have you considered placing the sample JSON in a standalone .json file and referencing it directly with [!code-json[...](...)]?
🤖 Helpful? 👍/👎
| } | ||
|
|
||
| void Start() | ||
| { |
There was a problem hiding this comment.
Start() is defined inside public class MyExampleInteraction : IInputInteraction, and falls inside #region custominteraction. As a result:
- The custom interaction snippet rendered in
write-custom-interactions.mdshows aStart()method on anIInputInteractionclass (which is not aMonoBehaviour). - The registration and usage samples are nested inside this class rather than shown as external setup code.
Have you considered moving Start() (or a sample method) to an external container class and closing #region custominteraction immediately after Reset()?
🤖 Helpful? 👍/👎
| // Matches all gamepads (also gamepads *based* on the Gamepad layout): | ||
| _ = "<Gamepad>"; | ||
| // Matches the "Submit" control on all devices: | ||
| _ = "*/"; |
There was a problem hiding this comment.
The comment above indicates this should match the Submit control across all devices, but the path is currently "*/".
| _ = "*/"; | |
| // Matches the "Submit" control on all devices: | |
| _ = "*/{Submit}"; |
🤖 Helpful? 👍/👎
| ``` | ||
| InputSystem.actions.FindAction("Move"); | ||
| ``` | ||
| [!code-cs[project-wide actions](Packages/com.unity.inputsystem/DocCodeSamples.Tests/AboutProjectWideActions.cs#about-project-wide-actions)] |
There was a problem hiding this comment.
There are two leading spaces before [!code-cs...]. Removing them ensures DocFX parses the include as a top-level block element:
| [!code-cs[project-wide actions](Packages/com.unity.inputsystem/DocCodeSamples.Tests/AboutProjectWideActions.cs#about-project-wide-actions)] | |
| [!code-cs[project-wide actions](Packages/com.unity.inputsystem/DocCodeSamples.Tests/AboutProjectWideActions.cs#about-project-wide-actions)] |
🤖 Helpful? 👍/👎
| } | ||
| } | ||
| ``` | ||
| [!code-cs[fullexmaple](Packages/com.unity.inputsystem/DocCodeSamples.Tests/UsingActionsWorkflowFullExample.cs)] |
There was a problem hiding this comment.
Small typo in the link text: fullexmaple -> fullexample.
| [!code-cs[fullexmaple](Packages/com.unity.inputsystem/DocCodeSamples.Tests/UsingActionsWorkflowFullExample.cs)] | |
| [!code-cs[fullexample](Packages/com.unity.inputsystem/DocCodeSamples.Tests/UsingActionsWorkflowFullExample.cs)] |
🤖 Helpful? 👍/👎
Review SummaryThe migration of hardcoded documentation samples into compiled and verified C# sample files under
📚 Additional Context
🤖 Helpful? 👍/👎 |
| const string myDeviceJson = @" | ||
| #region myDeviceJson | ||
| { | ||
| ""name"" : ""MyDevice"", |
There was a problem hiding this comment.
Because this JSON snippet is defined inside a C# verbatim string literal (@"..."), every double quote is escaped as "". DocFX extracts the lines between #region and #endregion verbatim without string unescaping, so the rendered documentation will display doubled quotes (e.g., ""name"" : ""MyDevice"").
Have you considered placing this JSON snippet into a standalone .json file and including it via [!code-json[...](...)]?
🤖 Helpful? 👍/👎
| const string json = @" | ||
| #region mydevicejson | ||
| { | ||
| ""name"" : ""MyDevice"", |
There was a problem hiding this comment.
Similar to HidCreateCustomLayoutExisting.cs, the JSON within this verbatim string literal uses escaped double quotes (""). When DocFX extracts #region mydevicejson, the rendered documentation snippet will contain invalid JSON with doubled quotes.
🤖 Helpful? 👍/👎
| ] | ||
| } | ||
| ``` | ||
| [!code-cs[mydevicejson](Packages/com.unity.inputsystem/DocCodeSamples.Tests/ProcessorControls.cs#mydevicejson)] |
There was a problem hiding this comment.
This snippet references a JSON example using [!code-cs[...]] instead of [!code-json[...]]. Consider switching the language tag to code-json once the verbatim string quoting is resolved.
📚 Additional Context
Packages/com.unity.inputsystem/Documentation~/add-processors-controls.md
🤖 Helpful? 👍/👎
| } | ||
|
|
||
| ``` | ||
| [!code-cs[customDeviceClass](Packages/com.unity.inputsystem/DocCodeSamples.Tests/HidCreateCustomLayoutClass.cs#customDeviceClass)] |
There was a problem hiding this comment.
While customDeviceClass has been updated to reference HidCreateCustomLayoutClass.cs, the remaining code blocks in this file are still hardcoded. You can update them to reference the corresponding regions defined in HidCreateCustomLayoutClass.cs:
#registerMyDevice#dualShock4HidInputReport#dualShock4GamepadHidBasic#dualShock4GamepadHidRegister
📚 Additional Context
Packages/com.unity.inputsystem/Documentation~/hid-create-custom-layout-class.md
🤖 Helpful? 👍/👎
| { | ||
| } | ||
|
|
||
| void Start() |
There was a problem hiding this comment.
MyExampleInteraction implements IInputInteraction rather than MonoBehaviour, and #region custominteraction wraps the entire class including void Start(). When write-custom-interactions.md includes #custominteraction, the custom interaction class snippet in the documentation will show void Start() with registration calls inside the class body.
Consider ending #region custominteraction right after Reset() and moving RegisterInteraction / InputAction setup into an example helper method outside MyExampleInteraction.
🤖 Helpful? 👍/👎
4 participants
Description
JIRA: DOCATT-11021
Moves some of the code examples into testable files. I did not have time to move all of them.
Testing status & QA
Overall Product Risks
Please rate the potential complexity and halo effect from low to high for the reviewers. Note down potential risks to specific Editor branches if any.
Comments to reviewers
Checklist
Before review:
Changed,Fixed,Addedsections.Area_CanDoX,Area_CanDoX_EvenIfYIsTheCase,Area_WhenIDoX_AndYHappens_ThisIsTheResult.During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.