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

Audio playback on AnimationPlayer is not stopping when reaching end of track [FIX INCLUDED] #26559

Open
TypeOverride opened this issue Mar 4, 2019 · 11 comments

Comments

@TypeOverride
Copy link

TypeOverride commented Mar 4, 2019

Godot version:
Godot 3.1 Beta 10

OS/device including version:
Windows 7

Issue description:
Audio is not stopping on AnimationPlayer when reached end or starting repeatedly by 0.

Steps to reproduce:
create a Audio on Animation Player
let the Audio playing reach over the end of the total length of the track
-> it will not stop to play (not in repeat track mode and not on stopping on reaching the track end)

FIX:
Change on animation_player.cpp::_animation_process_animation(...) :

if (!loop && p_time < nc->audio_star) {
   stop = true;
} else if (nc->audio_len > 0) {
      float len = nc->audio_start > p_time ? (a->get_length() - nc->audio_start) + p_time : p_time - nc->audio_start;
      if (len > nc->audio_len) {
           stop = true;
      }
}

to :

if (!loop && (p_time < nc->audio_start || p_time >= a->get_length())) {
     stop = true;
} else if (nc->audio_len > 0) {
     float len = (p_time < nc->audio_start) ? ((a->get_length() - nc->audio_start) + p_time) - nc->audio_len : (nc->audio_start + nc->audio_len) - p_time;
     if (len < 0) {
          stop = true;
}

I would hard change it more when i knew that i not produce more bugs. I would rewrite much of the code to get a better view on it. But i dont try it to make not much difference so the owner of the code not get irritated and i can give fixes here more easily. I only fix the thinks on the places where i can fix it without changing much.
But i think you can replace much of the code and make it much more nicer. And i have a problem that there are no strategy-patterns for the TrackTypes. Could me much more elegent. It could be much easier to implement new TrackTypes.

@TypeOverride TypeOverride changed the title Audio on AnimationPlayer is not stopping on reached end of track [FIX INCLUDED] Audio playback on AnimationPlayer is not stopping when reaching end of track [FIX INCLUDED] Mar 4, 2019
@akien-mga
Copy link
Member

Thanks for opening issues with fix proposals, but it's usually more efficient to open a PR directly, so that we can merge the fix if we agree with it. (Or open an issue, and open a PR that fixes it, so that even if for some reason we reject the PR, the bug is still open.)

@akien-mga akien-mga added this to the 3.1 milestone Mar 4, 2019
@TypeOverride
Copy link
Author

Thanks for opening issues with fix proposals, but it's usually more efficient to open a PR directly, so that we can merge the fix if we agree with it. (Or open an issue, and open a PR that fixes it, so that even if for some reason we reject the PR, the bug is still open.)

i guess it is... but i have no clue how to do it....

@akien-mga
Copy link
Member

See PR workflow.

@TypeOverride
Copy link
Author

TypeOverride commented Aug 4, 2020

The bug is not solved after over a year.
Will there be a fix someday?

Furthemore i fixed all of the problems + made cool features!
#26582

Will someone implemented it maybe in 4.0? Because it fixes the bugs and this are need need features for making cool cutscenes with the AnimationPlayer which is not possible in the way it is right now.

Its not much changes on code. Not much changes for the features. Small changes with big benefits!

I dont get it why noone is ever noticing all the bugs on animation playback. Make audio and put it in the negative area. It will have problems on playback. Put an audio over the end of the playtime, you will get problems with the playback. Put it fully over the full area of the playtime and it will never be played.

Its all fixed! Tested all cases. But why noone ever noticed all the problems. Live-preview of animations is not possible as well! I fixed it. Why it was disabled with dead code anyway? Because of performance issues? It runs well.

Edit:
I looked a some of the reported bugs here. Yeah my fixes will fix the most of them, i guess.

@Zireael07
Copy link
Contributor

Your PR could not be merged in, so it was closed. If fixing those bugs is so important to you, you could re-do the PR.

@TypeOverride
Copy link
Author

TypeOverride commented Aug 6, 2020

Your PR could not be merged in, so it was closed. If fixing those bugs is so important to you, you could re-do the PR.

But the question is more "Is it now feature-time"? I need a time span.
So i dont get into the problem, that i could leave the time span and i have to do everyting for nothing, again. So i have to do everything again. Like i have to do now than.

In the end i want to push 3 features as well.

I have no clue if i can push them step by step somehow. I did everything more or less in one do. But it
was no feature time. So it was discarded. One year later... no improvements on bugs.

So i need to know the time range of feature time. From date x till date x. Or shouldnt i push the feature ... and only the bug fixes? Than wait until it is merged. And than pushing the feature (in this case the feature where you can make an offset on AnimationTrack like on AudioTrack)

Too much questions.

Because i have to do everthing from start. With a brand new branch. And this takes time. And i dont want to go over the time span of feature time so it will only be wasting of time. Again.

GitHub is very complicated for me. I have to be very carefuly on updating to the last master merge thing thing, else it will look like the one i did. Total mess on merging on to the master branch.

Is it possible to make a second branch of the Godot master. Or do i have to close the one i did somehow? More a GitHub Question. For me Github is a thing where i have to be fast because i ruin ever branch on updating to the last master, when my branch is not up to date anymore.

But i need the features ... and the bug fixes. The AnimationPlayer in his current state is only buggy. I always ask myself why noone recognize it. It has potential. But the bugs are so stupid to work with.

@Zireael07
Copy link
Contributor

Yes, you can make a new fork (branch) of the Godot master.
Godot has no concept of "feature time" and no ETA on when anything lands in master. A PR should only contain bugfixes, as any new features currently have to go through a proposal process on https://github.com/godotengine/godot-proposals/issues

@TypeOverride
Copy link
Author

Yes, you can make a new fork (branch) of the Godot master.
Godot has no concept of "feature time" and no ETA on when anything lands in master. A PR should only contain bugfixes, as any new features currently have to go through a proposal process on https://github.com/godotengine/godot-proposals/issues

But someone told me that he will not merge it because "there is now a feature stop".

What ever. Than i start on weekend with a fully new branch and implement all stuff again. And i hope it will be merged. Else im out. I have fixed sooooo many bugs on your engine in my local branch. But the AnimationPlayer and those bugs fucks me up so hard everytime i dont use my version of Godot.

@Zireael07
Copy link
Contributor

But someone told me that he will not merge it because "there is now a feature stop".

Aaah. I see now what you mean. Yes, a new release of Godot 3.2 is on the horizon, but since you should make all PRs against master either way, the only thing it impacts is developer time/availability (Akien is on a holiday now, too).
Godot, as far as I know, does not have an explicit feature freeze, and if your PR is solely a bugfix PR, it will likely be considered before a new release. (Note the word 'solely' - I mean it, any new features would mean it would have to go through proposals AND be considered only after a new release to avoid breaking things)

@TypeOverride
Copy link
Author

TypeOverride commented Aug 6, 2020

Yes, you can make a new fork (branch) of the Godot master.
Godot has no concept of "feature time" and no ETA on when anything lands in master. A PR should only contain bugfixes, as any new features currently have to go through a proposal process on https://github.com/godotengine/godot-proposals/issues

no i checked it. Its not possible to make a seconds fork of an project when you already has one. You have to make a second account.

maybe i can plug the old one off?

@Zireael07
Copy link
Contributor

Wow, I didn't know. I believe you should be able to delete the existing fork and just fork again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants