Skip to content

Conversation

@Yarwin
Copy link
Contributor

@Yarwin Yarwin commented Dec 21, 2025

No description provided.

Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

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

Great example with invulnerability 👍

impl Monster {
fn make_invulnerable(&mut self) {
if let Some(connection) = self.handle.take()
&& connection.is_connected()
Copy link
Member

@Bromeon Bromeon Dec 21, 2025

Choose a reason for hiding this comment

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

Suggested change
&& connection.is_connected()
&& connection.is_connected()

(Rustfmt would indent this)

But I would not recommend is_connected() as general practice; it's needed only if there is no clear ownership of the signal, or if the involved object is expected to die. Maybe there could be a comment about a potential panic instead.

@Bromeon Bromeon added the new-topic New content added to the book label Dec 21, 2025
@Yarwin Yarwin force-pushed the update-signal-docs-add-connect-handle branch 2 times, most recently from d167d67 to 75e418a Compare December 21, 2025 09:24
Comment on lines +287 to +289
if let Some(connection) = self.handle.take()
&& connection.is_connected()
{
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if let Some(connection) = self.handle.take()
&& connection.is_connected()
{
if let Some(connection) = self.handle.take()
/* && connection.is_connected() -- see above */
{

Copy link
Member

Choose a reason for hiding this comment

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

Or even just

if let Some(connection) = self.handle.take() { 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did it on purpose this time, see comment over this section:

Disconnecting a signal will result in panic if given connection does not exist.
Use is_connected() when there is no clear ownership of the signal or connected object is expected to be freed.

alternatively I can remove the whole 2nd section 🤔.

@Yarwin Yarwin force-pushed the update-signal-docs-add-connect-handle branch from 75e418a to dc73ba9 Compare December 21, 2025 10:07
@Bromeon Bromeon merged commit 46bc9bf into godot-rust:master Dec 21, 2025
4 checks passed
@Bromeon
Copy link
Member

Bromeon commented Dec 21, 2025

Thank you!

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

Labels

new-topic New content added to the book

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants