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

#builder#300 animation group issues #303

Merged
merged 14 commits into from
Aug 12, 2024

Conversation

JiepengTan
Copy link
Collaborator

@JiepengTan JiepengTan commented Jul 10, 2024

issue: #300
project file: bug_anim.zip
config:
image

result preview:

animation_group_bug.mp4

@JiepengTan JiepengTan requested a review from nighca July 10, 2024 09:30
@nighca
Copy link
Collaborator

nighca commented Jul 11, 2024

@JiepengTan What is the relationship between these two:

  1. frameFrom, frameTo & frameFps
  2. from, to & fps

When should spx-user configure frameFrom, frameTo & frameFps, when shouldn't?

Copy link
Collaborator

@nighca nighca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The animation default (which is bound as defaultAnimation) is played only once while sprite in default state. It is expecetd to be played repeatedly (see details in spx: Animation binding builder#603 (comment))
  • After animation fight (played by calling animate "fight") finished playing, the sprite does not return to the default state, while it keeps the last csotume of animation fight

Are these two issues expected to be fixed by this PR? It seems that they are not addressed yet.

spgdi.go Outdated
if dirDeg > 180 {
dirDeg -= 360
}
if dirDeg < -45 || dirDeg > 135 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems this change occurs for #301

While as discussed, we should adopt the same logic as Scratch, which is

  • heading: [0, 180] is considered facing right
  • heading: (-180, 0) is considered facing left

I think we should compare dirDeg with 0, instead of -45, 135 here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems this change occurs for #301

While as discussed, we should adopt the same logic as Scratch, which is

  • heading: [0, 180] is considered facing right
  • heading: (-180, 0) is considered facing left

I think we should compare dirDeg with 0, instead of -45, 135 here?

It was fixed in the latest commit. 885578c

@JiepengTan
Copy link
Collaborator Author

@JiepengTan What is the relationship between these two:

  1. frameFrom, frameTo & frameFps
  2. from, to & fps

When should spx-user configure frameFrom, frameTo & frameFps, when shouldn't?

This design is for compatibility with old projects, although it is not very elegant.

Defination of "anitype":
image

If you need the old behavior (only action, no animation playing), you do not need to configure frameFrom, frameTo , frameFps & anitype.

If configured, it will play an additional animation on top of the previous behavior:
frameFrom: The frame where the animation starts
frameTo: The frame where the animation ends
frameFps: Animation playback speed (how many frames are played per second)

The purpose of fps is to adapt to: loop animations, such as step animations, fps can control the speed of animation playback.

heading: [0, 180] is considered facing right
heading: (-180, 0) is considered facing left
@JiepengTan
Copy link
Collaborator Author

JiepengTan commented Jul 18, 2024

fixed : #300

  1. The animation default (which is bound as defaultAnimation) is played only once while sprite in default state. It is expecetd to be played repeatedly (see details in spx: Animation binding builder#603 (comment))

  2. After animation fight (played by calling animate "fight") finished playing, the sprite does not return to the default state, while it keeps the last csotume of animation fight

default_anim_bug.mp4
default.mp4

@nighca

@JiepengTan JiepengTan requested a review from nighca July 18, 2024 04:17
@JiepengTan
Copy link
Collaborator Author

JiepengTan commented Jul 19, 2024

@nighca
nighca.zip

  1. frameFrom: The name of the starting image of the animation.

  2. frameTo: The name of the ending image of the animation.

  3. frameFps: The fps of the animation.

  4. turnToDuration: The duration of the turnTo action (the time for one circle, in seconds), default value is 1.(optional)

  5. stepDuration: The duration of the step action (the time for one step, in seconds), default value is 0.01.(optional)

    "fAnimations": {
        "fight": {
            "turnToDuration": 2,
            "frameFrom": "__animation_fight_attack_1-1",
            "frameTo": "__animation_fight_attack_1-4",
            "frameFps": 8
        },
        "dying": {
            "frameFrom": "__animation_dying_dead-1",
            "frameTo": "__animation_dying_dead-3",
            "frameFps": 8
        },
        "walk": {
            "stepDuration": 0.01,
            "frameFrom": "__animation_walk_walk-1",
            "frameTo": "__animation_walk_walk-8",
            "frameFps": 8
        },
        "default": { 
            "frameFrom": "__animation_default_idle-1",
            "frameTo": "__animation_default_idle-6",
            "frameFps": 16
        }
    },

Copy link
Collaborator

@nighca nighca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nighca
Copy link
Collaborator

nighca commented Aug 9, 2024

@xushiwei Please review this PR; it’s an important fix for the Go+ Builder animation feature.

@nighca nighca merged commit 33d8d71 into goplus:main Aug 12, 2024
3 checks passed
@JiepengTan JiepengTan deleted the #builder#300_animation_group_issues branch September 23, 2024 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants