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

Rename the method stop() to pause() in AnimatedSprite. #31168

Closed
Acytra opened this issue Aug 7, 2019 · 9 comments
Closed

Rename the method stop() to pause() in AnimatedSprite. #31168

Acytra opened this issue Aug 7, 2019 · 9 comments

Comments

@Acytra
Copy link

Acytra commented Aug 7, 2019

Usually the word stop is used when you wanna pause something and when you continue with it, it will start from the beginning. Instead the word pause is used when you wanna continue with the thing from where you left it. You can read more here if it's not clear:
https://www.differencebetween.com/difference-between-pause-and-vs-stop/

Right now, the method stop() in AnimatedSprite is working as a pause (it is even stated in the documentation saying it doesn't reset the frame counter), so what i propose is to change the name of the method stop() to pause.

https://docs.godotengine.org/en/3.1/classes/class_animatedsprite.html#class-animatedsprite-method-stop

In case you wanna keep the method stop, then after the pause, you should set the frame of the animation to zero, but for that we could open another issue.

@codecustard
Copy link
Contributor

I agree, stop should set it back to the beginning. Although, I would imagine this would break compatibility. Perhaps a boolean parameter can be added to stop which defaults to false, meaning it will stop animation but not reset the frame. That way it will work the same and not break compatibility. It will still "pause", but if you want to stop the animation and reset to the beginning, you can do something like: stop(true) which would reset the to the beginning.

Or maybe you can supply a frame number to stop and -1 would keep it on the current frame.

@isaacremnant
Copy link
Contributor

This should be consistent with AnimationPlayer. It has void stop ( bool reset=true ), so stop(false) is a pause and continuing is done with play() (with the default arguments).

Worth noting that you can specify the initial position/time in AudioStreamPlayer2D.play(from_position) and Timer.start(time_sec)', so maybe AnimatedSprite.play()andAnimationPlayer.play()` could use a similar argument?

@girng
Copy link

girng commented Aug 9, 2019

the parameter for stop (bool reset = true) makes the stop/pause issue objective
if you need stop functionality, set it to true and it'll reset

this is good i think.

@golddotasksquestions
Copy link

golddotasksquestions commented Aug 25, 2019

Having stop() and pause() is self-explanatory as this is widely spread knowledge in many other areas of living.
stop(bool) is something the user has to learn. "What's that supposed to mean, a stop, but not really?"
4.0 is breaking compatibility in many areas, if we have a chance to make GDscript more intuitive and self-explanatory, this is it.

@codecustard
Copy link
Contributor

Just wanted to comment, the documentation might be wrong (well I guess it is right, but the overall effect is wrong). Stop acts as a pause, but when you play(), the frame resets to zero.

AnimatedSprite::play() calls AnimatedSprite::set_animation(), which in turn resets the frame to zero.

@Costieman
Copy link

Costieman commented Jun 19, 2020

I am still very confused can someone write out an example for a novice. where and how do you set the parameter stop (bool reset = true)

@opl-
Copy link
Contributor

opl- commented Nov 10, 2020

Similarly, AnimatedSprite3D also uses the name stop() and mentions that it doesn't reset the frame counter: https://docs.godotengine.org/en/3.2/classes/class_animatedsprite3d.html#class-animatedsprite3d-method-stop

@Calinou
Copy link
Member

Calinou commented Nov 10, 2020

Closing, as this renaming request is now tracked in #16863.

@golddotasksquestions
Copy link

Note there is also this proposal for further discussion:
godotengine/godot-proposals#287

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

Successfully merging a pull request may close this issue.

9 participants