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.

Sign up for GitHub

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

Jump to bottom

Update 04.mob_scene.rst #8978

Merged
merged 1 commit into from
Feb 18, 2024
Merged

Update 04.mob_scene.rst #8978

merged 1 commit into from
Feb 18, 2024

Conversation

MarsAstro
Copy link
Contributor

Removed an unnecessary instruction. The tutorial calls for the user to return to the 3D viewport to select the VisibleOnScreenNotifier3D and connect its screen_exited signal to the mob script. It then says this will return you to the script. Since these actions happen in the scene dock and inspector dock, there is no need to change from Script mode to 3D mode in the viewport.

@AThousandShips
Copy link
Member

Thank you for your contribution!

You should target master not 4.2, changes can then be cherry picked, see here.

If you need more information about the update process please see: here. If wanted I can fix the branch for you.

@AThousandShips AThousandShips added the needs work Needs additional work by the original author, someone else or in another repo. label Feb 16, 2024
@MarsAstro MarsAstro changed the base branch from 4.2 to master February 16, 2024 18:50
@MarsAstro
Copy link
Contributor Author

MarsAstro commented Feb 16, 2024 •
edited
Loading

Thank you for your contribution!

You should target master not 4.2, changes can then be cherry picked, see here.

If you need more information about the update process please see: here. If wanted I can fix the branch for you.

Sorry, first time I try anything like this! I just clicked the "Edit on GitHub" link in docs and followed the prompts, so I'm sure there's a lot wrong with it. I tried to fix it, but I think I broke the pull request. There's a bunch of merge conflicts now. If the branch/pull request is still salvageable, then I'd love some help fixing it. If not I'll delete it and try again from scratch, using your links to guide me :)

@AThousandShips
Copy link
Member

AThousandShips commented Feb 16, 2024 •
edited
Loading

You can follow the instructions above that explains how to do this (check the second link), or you can create a new PR, I can't help with rebasing this one right now as I'm not by my computer

@MarsAstro @AThousandShips
Update 04.mob_scene.rst
cd48dd0 
Removed an unnecessary instruction
@AThousandShips
Copy link
Member

There resolved it for you :)

@AThousandShips AThousandShips added enhancement area:getting started Issues and PRs related to the Getting Started section of the documentation and removed needs work Needs additional work by the original author, someone else or in another repo. labels Feb 16, 2024
@MarsAstro
Copy link
Contributor Author

There resolved it for you :)

Thanks a lot, you're a godsend! I'll make sure I'm going in less blind next time.

Love the Moomin profile pic, btw, reminds me of my childhood! :)

@AThousandShips
Copy link
Member

Thank you! Same here, grew up on those books

@skyace65 skyace65 merged commit a20f86c into godotengine:master Feb 18, 2024
1 check passed
@skyace65
Copy link
Contributor

Thanks! And congrats on your first merged PR!

@MarsAstro
Copy link
Contributor Author

Thanks! And congrats on your first merged PR!

Thank you! Feels exciting, hopefully not the last! :)

@mhilbrunner
Copy link
Member

Cherry-picked to 4.2 in #9647.

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

@skyace65 skyace65 skyace65 approved these changes

Assignees
No one assigned
Labels
area:getting started Issues and PRs related to the Getting Started section of the documentation enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MarsAstro @AThousandShips @skyace65 @mhilbrunner