fix: Issue #1021 - PhysicsProcessor ParentScene may be initialized to null

[wrshield]

PR Details

Description

This change is to address issue # 1021 - PhysicsProcessor ParentScene may be initialized to null.
The change was made in PhysicsProcessor.cs at line 160, where I added a check to see if ParentScene is null, and if so assign the entity scene to ParentScene.

Related Issue

#1021

Motivation and Context

The issue was raised because when using the debug physics shape script, the colliders were not being displayed after pressing ctrl+shift+p at runtime. To verify that this change solves the issue:

1. Create a new project
2. Add a static collider to the ground
3. Add Debug physics shapes script
4. Launch game and press Ctrl+Shift+P
5. Colliders are now shown highlighted red

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My change requires a change to the documentation.
• I have added tests to cover my changes.
• All new and existing tests passed.
• I have built and run the editor to try this change out.

[wrshield]

@wrshield please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

• (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.

@dotnet-policy-service agree

• (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.

@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@dotnet-policy-service agree

[Eideren]

I think it would be more appropriate to move that logic above the following line: https://github.com/stride3d/stride/blob/master/sources/engine/Stride.Physics/Engine/PhysicsProcessor.cs#L106
That way it is grouped with its usage, and would only run when the debug stuff is toggled on.
To retrieve a valid parentScene value from there you can copy the existing one in OnSystemAdd, so parentScene = Services.GetSafeServiceAs<SceneSystem>().SceneInstance.RootScene; note the null checks being purposefully removed here to ensure we throw instead of silently failing since nothing would be visible otherwise.

[wrshield]

I think it would be more appropriate to move that logic above the following line: https://github.com/stride3d/stride/blob/master/sources/engine/Stride.Physics/Engine/PhysicsProcessor.cs#L106 That way it is grouped with its usage, and would only run when the debug stuff is toggled on. To retrieve a valid parentScene value from there you can copy the existing one in OnSystemAdd, so parentScene = Services.GetSafeServiceAs<SceneSystem>().SceneInstance.RootScene; note the null checks being purposefully removed here to ensure we throw instead of silently failing since nothing would be visible otherwise.

Makes sense, thanks for the suggestion, I did a new push with this change.

[Eideren]

Sorry, I should have specified that you should only conditionally assign that parentScene if none was provided by the user before that call, so something like:

parentScene ??= Services.GetSafeServiceAs<SceneSystem>().SceneInstance.RootScene;

Users may have set the parentScene to a specific subscene. Tebjan added the parentScene support in so I would guess that VVVV are using this feature in their application.

[Eideren]

Thanks !