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

Provide instructions for resource change notifications in tools #9283

Merged
merged 29 commits into from
Apr 27, 2024

Conversation

betalars
Copy link
Contributor

I have found this quite counter-intuitive, and was irritated by there being no guidance around this, so I wanted to create this enhancement.

Please double-check my c# Code, I do not use it personally and there will probably be mistakes.

@betalars
Copy link
Contributor Author

Also: I know this is not strictly adhering to the guidelines when it comes to giving easy-to-understand examples, but honestly this is kind of an advanced issue so I feel there is a value in keeping it short?

But feel free to suggest a different framing and I will update it.

Most examples of resources I could think of would add a lot of complexity to the explanation because using a resource in a tool does not make sense that often.

@AThousandShips AThousandShips added enhancement area:manual Issues and PRs related to the Manual/Tutorials section of the documentation labels Apr 27, 2024
tutorials/plugins/running_code_in_the_editor.rst Outdated Show resolved Hide resolved
tutorials/plugins/running_code_in_the_editor.rst Outdated Show resolved Hide resolved
tutorials/plugins/running_code_in_the_editor.rst Outdated Show resolved Hide resolved
tutorials/plugins/running_code_in_the_editor.rst Outdated Show resolved Hide resolved
tutorials/plugins/running_code_in_the_editor.rst Outdated Show resolved Hide resolved
tutorials/plugins/running_code_in_the_editor.rst Outdated Show resolved Hide resolved
tutorials/plugins/running_code_in_the_editor.rst Outdated Show resolved Hide resolved
tutorials/plugins/running_code_in_the_editor.rst Outdated Show resolved Hide resolved
betalars and others added 12 commits April 27, 2024 15:16
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
@betalars
Copy link
Contributor Author

Thanks @AThousandShips !

@AThousandShips
Copy link
Member

You missed several ones, please check all

betalars and others added 5 commits April 27, 2024 15:26
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
@betalars
Copy link
Contributor Author

You missed several ones, please check all

Yea I just realized that as well while checking the diff. I did not expect github to collapse unresolved threads and not resolved ones.

tutorials/plugins/running_code_in_the_editor.rst Outdated Show resolved Hide resolved
tutorials/plugins/running_code_in_the_editor.rst Outdated Show resolved Hide resolved
tutorials/plugins/running_code_in_the_editor.rst Outdated Show resolved Hide resolved
tutorials/plugins/running_code_in_the_editor.rst Outdated Show resolved Hide resolved
tutorials/plugins/running_code_in_the_editor.rst Outdated Show resolved Hide resolved
tutorials/plugins/running_code_in_the_editor.rst Outdated Show resolved Hide resolved
@raulsntos
Copy link
Member

Also note that you can use the Add suggestion to batch button from the Files changed tab instead of applying the suggestions one by one.

Co-authored-by: Raul Santos <raulsntos@gmail.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

Stylewise this look great!

Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Co-authored-by: Raul Santos <raulsntos@gmail.com>
@AThousandShips AThousandShips changed the title providing guidance for being notified about resources changing in tools Provide instructions for resource change notifications in tools Apr 27, 2024
@AThousandShips AThousandShips merged commit e6c8e51 into godotengine:master Apr 27, 2024
1 check passed
@AThousandShips
Copy link
Member

Thanks! And congratulations on your first merged contribution! 🎉

@betalars
Copy link
Contributor Author

Thanks! And congratulations on your first merged contribution! 🎉

I think I did another documentation thing before, but this is the first merge with code :D

@AThousandShips
Copy link
Member

It wasn't registered at least, at least not on here

@betalars
Copy link
Contributor Author

betalars commented Apr 28, 2024

Lastly, you should to disconnect

Oh no, we missed a typo.
I really need to get into a habit of letting things sit for a day and then checking again because I often restructure sentences and then make gramatical errors because of that.

Will fix that soon, but need to run rn

GD.Print("My resource just changed!");
}

Lastly, you should to disconnect the signal as the old resource being used and changed somewhere else
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
Lastly, you should to disconnect the signal as the old resource being used and changed somewhere else
Lastly, remember to disconnect the signal as the old resource being used and changed somewhere else

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay I am afraid this is going to need a new PR

Copy link
Member

Choose a reason for hiding this comment

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

Yes you can't update something already merged, but no worries, missed it too

@mhilbrunner
Copy link
Member

Cherry-picked to 4.2 in #9648.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation cherrypick:4.0 cherrypick:4.1 enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants