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

Add details about AudioEffectCapture.clear_buffer() and get_buffer() #84584

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

mutantbob
Copy link

This adds a few sentences to the AudioEffectCapture.xml documentation with helpful notes about the clear_buffer() and get_buffer() functions.

It does not fully address issue godotengine/godot-docs#8326 , but should be useful to new users.

@mutantbob mutantbob requested a review from a team as a code owner November 7, 2023 15:56
@AThousandShips AThousandShips changed the title add details about AudioEffectCapture.clear_buffer() and get_buffer() Add details about AudioEffectCapture.clear_buffer() and get_buffer() Nov 7, 2023
@YuriSizov
Copy link
Contributor

When you make further changes, could you please squash your commits into one? Make sure that the final commit has a short but descriptive message (the title of this PR is a good option). See this documentation, if you need help with squashing.

@AThousandShips
Copy link
Member

Your commit seems not to be linked to your GitHub account. See: Why are my commits linked to the wrong user? for more info.

@mutantbob
Copy link
Author

@YuriSizov it started out as a single commit, but several modifications were suggested in the pull request and a I accepted them. The github UI did not offer me a squash option.

@AThousandShips
Copy link
Member

Please see the documentation above, you need to use the command line

I can help you if you are not able to do so yourself

@AThousandShips
Copy link
Member

AThousandShips commented Nov 7, 2023

You changed previous commits incorrectly, you need to remove those, this is because you resigned them with your own user

You should also start your commit with a capital letter

@AThousandShips
Copy link
Member

To drop those commits you need to do git rebase -i master and then put drop in front of the commits you shouldn't include

@fire
Copy link
Member

fire commented Nov 11, 2023

@lyuma if your interested

@Mickeon Mickeon requested a review from a team January 28, 2024 14:33
Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

The details look good on paper but we need feedback from audio-savvy developers.

doc/classes/AudioEffectCapture.xml Outdated Show resolved Hide resolved
doc/classes/AudioEffectCapture.xml Outdated Show resolved Hide resolved
@akien-mga akien-mga modified the milestones: 4.x, 4.3 Jan 29, 2024
@akien-mga akien-mga requested review from fire and lyuma February 14, 2024 09:35
Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

The first section looks ok to me, I haven't checked the math for get_buffer. Will ping @lyuma

@akien-mga
Copy link
Member

I pushed a doc update as suggested.

@mutantbob For the record, your commit seems not to be linked to your GitHub account. See: Why are my commits linked to the wrong user? for more info.

@akien-mga akien-mga merged commit 14a05f0 into godotengine:master Feb 21, 2024
16 checks passed
@akien-mga
Copy link
Member

akien-mga commented Feb 21, 2024

Thanks! And congrats for your first merged Godot contribution 🎉

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.

6 participants