WIP Remove Design Script Debugger code #16489
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR aims to remove Design Script Debugger code from the codebase. It involves deleting entire debug-related classes, removing debugger properties, and stripping debug functionality from the execution engine.
Key changes:
- Removal of debug runner and debug test framework classes
- Elimination of debugger properties and breakpoint handling
- Cleanup of expression interpreter and watch functionality
Reviewed Changes
Copilot reviewed 35 out of 38 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Engine/ProtoTestFx/WatchTestFx.cs | Completely removed file containing watch test framework |
| test/Engine/ProtoTestFx/DebugTestFx.cs | Completely removed file containing debug test framework |
| src/Engine/ProtoScript/Runners/DebugRunner.cs | Completely removed debug runner implementation |
| src/Engine/ProtoScript/Runners/ExpressionInterpreterRunner.cs | Completely removed expression interpreter runner |
| src/Engine/ProtoCore/DebuggerProperties.cs | Removed debug properties and breakpoint classes while keeping event sink |
| src/Engine/ProtoCore/DSASM/Executive.cs | Removed debug-specific execution methods and breakpoint handling |
| src/Engine/ProtoCore/DSASM/InstructionSet.cs | Removed debug instruction types and debug info structures |
| Multiple other files | Removed debug-related method parameters, properties, and functionality |
| @@ -1,4 +1,4 @@ | |||
| | |||
|
|
|||
There was a problem hiding this comment.
[nitpick] The file starts with an empty line instead of proper content. This appears to be a formatting issue.
Copilot uses AI. Check for mistakes.
| @@ -1,4 +1,4 @@ | |||
| using System.Collections.Generic; | |||
| using System.Collections.Generic; | |||
There was a problem hiding this comment.
[nitpick] The file starts with an empty line instead of proper content. This appears to be a formatting issue.
Copilot uses AI. Check for mistakes.
| @@ -1,13 +1,7 @@ | |||
| using System.Collections.Generic; | |||
| using System.Collections.Generic; | |||
There was a problem hiding this comment.
[nitpick] The file starts with an empty line instead of proper content. This appears to be a formatting issue.
Copilot uses AI. Check for mistakes.
| @@ -1,4 +1,4 @@ | |||
| using System; | |||
| using System; | |||
There was a problem hiding this comment.
[nitpick] The file starts with an empty line instead of proper content. This appears to be a formatting issue.
Copilot uses AI. Check for mistakes.
| @@ -1,4 +1,4 @@ | |||
| using System.Collections.Generic; | |||
| using System.Collections.Generic; | |||
|
|
|||
There was a problem hiding this comment.
[nitpick] The file starts with an empty line instead of proper content. This appears to be a formatting issue.
Copilot uses AI. Check for mistakes.
| int constructBlockId = RuntimeMemory.CurrentConstructBlockId; | ||
| if (constructBlockId == Constants.kInvalidIndex) | ||
| return DebugProps.CurrentBlockId; | ||
| return 0; //Todo do we need this? |
There was a problem hiding this comment.
The TODO comment indicates uncertainty about the necessity of this code. This should be resolved before merging - either remove the code if not needed or update the comment with proper documentation.
| return 0; //Todo do we need this? | |
| // Returning 0 when the constructBlockId is invalid ensures that callers receive a valid block ID. | |
| // This prevents errors in downstream code that expects a non-negative block ID. | |
| return 0; |
Copilot uses AI. Check for mistakes.
|
|
||
|
|
||
| blockCaller = runtimeCore.DebugProps.CurrentBlockId; | ||
| blockCaller = 0; //Need to figure out how to get this here |
There was a problem hiding this comment.
The comment indicates incomplete implementation. This hardcoded value should be properly implemented or documented with a clear explanation of why it's acceptable.
| blockCaller = 0; //Need to figure out how to get this here | |
| // In this context, the caller block ID is set to 0 because the inline conditional call does not originate from a specific language block. | |
| // If future changes require tracking the actual caller block, this assignment should be revisited. | |
| blockCaller = 0; |
Copilot uses AI. Check for mistakes.
|
@saintentropy it might be worth referring to #8164 as not a lot has changed in this code. |
Purpose
(FILL ME IN) This section describes why this PR is here. Usually it would include a reference to the tracking task that it is part or all of the solution for.
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
(FILL ME IN) Brief description of the fix / enhancement. Use N/A to indicate that the changes in this pull request do not apply to Release Notes. Mandatory section
Reviewers
(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)
(FILL ME IN, optional) Any additional notes to reviewers or testers.
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of