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 crash on expired node #2

Merged
merged 1 commit into from
Jan 8, 2025
Merged

fix crash on expired node #2

merged 1 commit into from
Jan 8, 2025

Conversation

Doprez
Copy link
Contributor

@Doprez Doprez commented Jan 8, 2025

I noticed a crash when the player picks up the rifle then the pistol they can't pick up any of the physics cubes.

This just checks if the joint was expired before trying to change the attachment node. I also saw a couple typos that should now be fixed as well.

@gleblebedev gleblebedev merged commit 5462b62 into rbfx:master Jan 8, 2025
@gleblebedev
Copy link
Collaborator

@Doprez what do you think... Shall we do the same as Unity, i.e. when code checks for equality to null check if the node expired and return true if it is?

@Doprez
Copy link
Contributor Author

Doprez commented Jan 8, 2025

I think that makes sense, I dont see the average user understanding why/when this could be a possibility.

If it isnt in an overridden equality check, then maybe an extension method that allows this in a single line check like IsValid or IsAlive. This would at least make the user think about the lifespan of a Node and it would be easy enough to document.

public static bool IsAlive(this RefCounted reference)
{
    return reference is not null && !reference.IsExpired;
}

Either would work in my eyes but one has the learning benefit.

@Doprez Doprez deleted the crash-fix branch January 9, 2025 03:30
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.

2 participants