Skip to content

[UUM-136930] Fix Arch field not readable#674

Open
lopezt-unity wants to merge 4 commits into
masterfrom
bugfix/uum-136930-fix-arch-field-not-readable
Open

[UUM-136930] Fix Arch field not readable#674
lopezt-unity wants to merge 4 commits into
masterfrom
bugfix/uum-136930-fix-arch-field-not-readable

Conversation

@lopezt-unity
Copy link
Copy Markdown
Collaborator

Purpose of this PR

This PR shorten the label "Arch circumference" to "Arch circ." to avoid a cut in the Editor UIs.
This choice has been done to align with the Torus shape that is already doing this.

Links

Jira: https://jira.unity3d.com/browse/UUM-136930

Comments to Reviewers

[List known issues, planned work, provide any extra context for your code.]

Copy link
Copy Markdown
Contributor

@u-pr u-pr Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May require changes

The changes update shape editing capabilities but leave behind some diagnostic profiling code and modify undo/redo behavior that needs verification. A quick cleanup and review of the undo logic is recommended.

🤖 Helpful? 👍/👎

if(comp is ProBuilderShape shapeComponent && shapeComponent.isEditable)
{
UndoUtility.RecordComponents<Transform, ProBuilderMesh, ProBuilderShape>(shapeComponent.GetComponents(typeof(Component)),"Resize Shape");
Stopwatch stopWatch = new Stopwatch();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

It looks like some temporary diagnostic and profiling code (including Stopwatch instantiation, TimeSpan measurements, commented-out lines, and Debug.Log) was left in this block. To keep the codebase clean and avoid logging overhead, would you consider removing the performance-tracking logic and the commented-out lines?

For example, the block inside the if statement could be simplified to:

Undo.RegisterCompleteObjectUndo(shapeComponent.mesh, "Edit Shape");
shapeComponent.UpdateShape();
if(tool != null)
{
    tool.SetBounds(shapeComponent.size);
    tool.SaveShapeParams(shapeComponent);
}
ProBuilderEditor.Refresh();

🤖 Helpful? 👍/👎 by guardian

//Undo.RecordObject(shapeComponent.transform, "Edit Shape");
//Undo.RecordObject(shapeComponent, "Edit Shape");

Undo.RegisterCompleteObjectUndo(shapeComponent.mesh, "Edit Shape");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

It appears that WIP profiling and debugging code was accidentally committed here.

Commenting out UndoUtility.RecordComponents and replacing it with Undo.RegisterCompleteObjectUndo(shapeComponent.mesh, ...) skips registering Undo operations for Transform and ProBuilderShape changes. This will break Undo/Redo functionality when editing shape properties in the scene.

🤖 Helpful? 👍/👎 by bug_hunter

stopWatch.Stop();
// Get the elapsed time as a TimeSpan value.
TimeSpan ts3 = stopWatch.Elapsed;
Debug.Log($"Time spend = ({ts1.TotalMilliseconds}, {ts2.TotalMilliseconds}, {ts3.TotalMilliseconds}) ms");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This Debug.Log and the associated Stopwatch tracking appear to be accidental leftovers from local profiling. Leaving this in will spam the Editor console with timing information whenever shape properties are modified.

🤖 Helpful? 👍/👎 by bug_hunter

@codecov-github-com
Copy link
Copy Markdown

codecov-github-com Bot commented Jun 1, 2026

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
Editor/EditorCore/ProBuilderShapeEditor.cs 0.00% 1 Missing ⚠️
Runtime/Shapes/Arch.cs 0.00% 1 Missing ⚠️
@@            Coverage Diff             @@
##           master     #674      +/-   ##
==========================================
+ Coverage   36.05%   38.02%   +1.96%     
==========================================
  Files         277      278       +1     
  Lines       34909    38859    +3950     
==========================================
+ Hits        12588    14775    +2187     
- Misses      22321    24084    +1763     
Flag Coverage Δ
probuilder_MacOS_6000.0 35.64% <0.00%> (+1.06%) ⬆️
probuilder_MacOS_6000.3 35.64% <0.00%> (+1.06%) ⬆️
probuilder_MacOS_6000.4 35.64% <0.00%> (+1.06%) ⬆️
probuilder_MacOS_6000.5 35.64% <0.00%> (+1.07%) ⬆️
probuilder_MacOS_6000.6 35.64% <0.00%> (?)
probuilder_Windows_6000.0 35.56% <0.00%> (+0.32%) ⬆️
probuilder_Windows_6000.3 35.56% <0.00%> (+0.32%) ⬆️
probuilder_Windows_6000.4 35.56% <0.00%> (+0.31%) ⬆️
probuilder_Windows_6000.5 35.56% <0.00%> (+0.32%) ⬆️
probuilder_Windows_6000.6 35.56% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
Editor/EditorCore/ProBuilderShapeEditor.cs 6.58% <0.00%> (+1.70%) ⬆️
Runtime/Shapes/Arch.cs 62.69% <0.00%> (ø)

... and 90 files with indirect coverage changes

ℹ️ Need help interpreting these results?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant