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

Rename AudioStreamPlayer to AudioPlayer #46997

Closed
wants to merge 1 commit into from
Closed

Rename AudioStreamPlayer to AudioPlayer #46997

wants to merge 1 commit into from

Conversation

madmiraal
Copy link
Contributor

Also renames:

  • AudioStreamPlayer2D to AudioPlayer2D
  • AudioStreamPlayer3D to AudioPlayer3D

Originally suggested by @dalexeev in #16863.

@madmiraal madmiraal added this to the 4.0 milestone Mar 14, 2021
@madmiraal madmiraal requested review from a team as code owners March 14, 2021 13:20
@madmiraal
Rename AudioStreamPlayer to AudioPlayer
f69b081 
Also renames:
- AudioStreamPlayer2D to AudioPlayer2D
- AudioStreamPlayer3D to AudioPlayer3D
@madmiraal madmiraal force-pushed the rename-audio-stream-player branch from ae4d79b to f69b081 Compare March 21, 2021 10:08
@madmiraal
Copy link
Contributor Author

Rebased following merge of #46740.

@mhilbrunner
Copy link
Member

Thanks! Sorry its taking time to review due to the backlog situation.

Discussed this with other team members and there does not seem to be consensus for this change, so I'm closing it for now.
Still, thank you for suggesting it :)

@mhilbrunner mhilbrunner removed this from the 4.0 milestone Nov 30, 2021
@madmiraal
Copy link
Contributor Author

@mhilbrunner what is the argument for not making this change? Has it been decided to rename VideoPlayer to VideoStreamPlayer instead?

@mhilbrunner
Copy link
Member

mhilbrunner commented Dec 2, 2021 •
edited
Loading

There was no clear consensus for the renames either way - some for it, some against. If we are going forward with these renames, it should go through the proposal process to demonstrate there is clear support first. Discussion on the pros/cons can happen in that proposal.

@dalexeev dalexeev mentioned this pull request Dec 2, 2021
Closed
@dalexeev
Copy link
Member

dalexeev commented Dec 2, 2021

I opened a proposal where you can vote: godotengine/godot-proposals#3624.

@madmiraal
Copy link
Contributor Author

Can we reopen this PR to support godotengine/godot-proposals#3624, or do I need to create a new one?

@Calinou
Copy link
Member

Calinou commented Dec 3, 2021 •
edited
Loading

Can we reopen this PR to support godotengine/godot-proposals#3624, or do I need to create a new one?

GitHub prevents reopening this PR since the audio-stream-player branch was force-pushed or recreated.

@madmiraal madmiraal mentioned this pull request Dec 3, 2021
Closed
@madmiraal
Copy link
Contributor Author

I've created a new PR: #55583.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
archived breaks compat enhancement topic:audio
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@madmiraal @mhilbrunner @dalexeev @Calinou