Add missing documentation for AudioStream & AudioStreamPlayback #86958
Conversation
Mickeon
force-pushed
the
reduz-adds-stuff,-refuses-to-elaborate,-leaves
branch
from
aa0adcb
to
9c39945
Compare
AThousandShips
added
cherrypick:4.1
Mickeon
force-pushed
the
reduz-adds-stuff,-refuses-to-elaborate,-leaves
branch
from
9c39945
to
3edfb7d
Compare
|
Updated this PR to address the above, and filled the descriptions of both |
doc/classes/AudioStream.xml
Outdated
| @@ -16,31 +16,39 @@ | |||
| <method name="_get_beat_count" qualifiers="virtual const"> | |||
| <return type="int" /> | |||
| <description> | |||
| Should return the total number of beats of this audio stream. Used by the engine to determine the position of every beat. | |||
There was a problem hiding this comment.
I think we should always explicitly tell the user that virtual methods need to be implemented by them. Without this clarification a description starting with "Should return" looks concerning, as if we don't know exactly if this method returns what we think it returns.
Same below.
There was a problem hiding this comment.
The usual "Override this method to[...]" cannot really work here otherwise it would be too verbose.
I would introduce them with "Overridable method." personally. I don't like to say "Virtual method to be implemented by the user" because it feels very redundant, but that's obviously just an opinion. virtual is right there, you can see what it does, and all virtual methods begin with an underscore. Users should be used by this, especially if they're trying to make their own audio streams.
There was a problem hiding this comment.
I'm not sure about the verbosity argument. It's better if the description is explicit, and a bit of redundancy is okay when we don't know where the user will start reading. Use "Overridable method." if you want, that's better than nothing.
Mickeon
force-pushed
the
reduz-adds-stuff,-refuses-to-elaborate,-leaves
branch
from
3edfb7d
to
8f4598b
Compare
|
Updated the PR to add "Overridable method" or similar to the methods that don't explicitly start with "Override" or similar. |
|
Thanks! |
|
Cherry-picked for 4.2.2. |
akien-mga
removed
cherrypick:4.1
Related to #86927
This PR completes the documentation of AudioServer, AudioStream and AudioStreamPlayback, filling all of the undocumented methods.
The existing descriptions where mostly left untouched even though they're quite poor.
Some doubts are still there, but it's still way better than leaving them as the "mystery meat" methods. Actually writing for them has you realise how flawed some of these are.