godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.07k stars 21.18k forks source link

'AnimationPlayer.get_autoplay()' returns empty string #12561

Closed Keetz closed 7 years ago

Keetz commented 7 years ago

Operating system or device, Godot version, GPU Model and driver (if graphics related): Godot 2.1.4

Issue description: The method 'get_autoplay()' returns an empty string and not the actual name of the animation set to autoplay on load in the editor.

This doesn't happen if you do 'set_autoplay("some_anim")' in script. So i guess it's "just" the editor button "Autoplay on Load" that doesn't set the autoplay property correct?

Steps to reproduce:

  1. Create an AnimationPlayer
  2. Add a new animation at set it to Autoplay on Load
  3. Try to print out 'AnimationPlayer.get_autoplay()'
akien-mga commented 7 years ago

Probably due to this: https://github.com/godotengine/godot/blob/a0a9363/scene/animation/animation_player.cpp#L204 Added by @reduz in db43e941752590f32e949638e35f5891bc8979c3 to fix "autoplay issue", but no idea what issue that was.

akien-mga commented 7 years ago

Affects the master branch too: https://github.com/godotengine/godot/blob/9b63418/scene/animation/animation_player.cpp#L205

Keetz commented 7 years ago

@akien-mga nice find :)

So i just tried doing:

set_autoplay(autoplay)

It seems to work, so i'll test just a bit more and create a pull request

akien-mga commented 7 years ago

set_autoplay(autoplay)

Well that's a bit pointless, since set_autoplay only sets autoplay to the given value, so that would amount to setting it to its existing value. I'd just remove set_autoplay("); (and the corresponding "fix" in SteamPlayer: https://github.com/godotengine/godot/commit/db43e941752590f32e949638e35f5891bc8979c3#diff-f2891540866368691953e010a51009d8R109 ).

But then the question remains about what those additions were trying to fix in the first place.

akien-mga commented 7 years ago

But then the question remains about what those additions were trying to fix in the first place.

I can only imagine that some users would have complained about the autoplay animation/sound playing again when an AnimationPlayer/StreamPlayer is reparented (and thus NOTIFICATION_READY is emitted again), but one could argue that it's a feature.

akien-mga commented 7 years ago

I guess it was this issue: #1905. Since it's properly fixed in master (ready stuff isn't reemitted on parent change), the "fix" linked above could be reverted. For 2.1, that's up for discussion, but we should keep in mind that it's a compatibility breakage.

Keetz commented 7 years ago

Seems like https://github.com/godotengine/godot/commit/db43e941752590f32e949638e35f5891bc8979c3#diff-f2891540866368691953e010a51009d8R109 has already been fixed/reverted as the StreamPlayer has been removed or kind of merged with the AudioStreamPlayer i guess?

And i can't see similar behavior in AudioStreamPlayer so it should be good, please correct me if i am wrong.

akien-mga commented 7 years ago

Yep, looks like there is no change needed for AudioStreamPlayer.