Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: PhysicsComponent reattach logic crashing the editor #2465

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

Basewq
Copy link
Contributor

@Basewq Basewq commented Sep 24, 2024

PR Details

Ignore reattach call if the component hasn't been attached yet.
This situation occurs when the editor gizmo wants to show the physics shape.

Not sure if null checking in both places is overkill or not, this shouldn't be a frequent call so any performance hit should be minimal. Maybe change to Debug.Assert in ReAttach()?

I've tested this opening the editor and at run-time and did not encounter any issue, however I did not test whatever the original implementation was for.
Also I couldn't build/run the tests (for an unrelated issue), so someone do a quick run to check this didn't break an existing test.

Related Issue

Fixes #2464
Caused by #2422 and #2439

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.

@Eideren
Copy link
Collaborator

Eideren commented Sep 24, 2024

Ran the physics tests, the test covering the initial issue, topdown and the editor repro and did not notice any issue.

Maybe change to Debug.Assert in ReAttach()?

A throw makes sense, it is called fairly infrequently as you said - the added overhead should be fine.
Thanks !

@Eideren Eideren merged commit 90522b7 into stride3d:master Sep 24, 2024
2 checks passed
@Eideren Eideren changed the title Fix PhysicsComponent reattach logic crashing the editor fix: PhysicsComponent reattach logic crashing the editor Sep 24, 2024
@Basewq Basewq deleted the phys_reattach_fix branch October 12, 2024 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PhysicsComponent ReAttach logic error
2 participants