-
-
Notifications
You must be signed in to change notification settings - Fork 21k
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
Implement record feature for AnimationTree. #77395
base: master
Are you sure you want to change the base?
Implement record feature for AnimationTree. #77395
Conversation
How does it work? |
bbb777e
to
c94c245
Compare
c94c245
to
f041a38
Compare
f041a38
to
5c9cc26
Compare
5cbbce0
to
bef82cf
Compare
scene/animation/animation_tree.cpp
Outdated
new_record.state_last_pass = state.last_pass; | ||
new_record.state_valid = state.valid; | ||
new_record.process_pass = process_pass; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these three variables are not necessary. Instead, I guess you may need some way to avoid doing double processing after the call to load_record()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
state.last_pass
will be overrided in _process_graph
, it can't be removed.
It seem state.last_pass
and process_pass
only be used to get activity in editor plugin,
state.valid
will be overrided in _process_graph
, too, and it is not be expoed to GDScript, It seem only work with state.state_invalid_reasons
in editor.
In conclusion, I remove state.last_pass
, and handel process_pass
, state.valid
and state.state_invalid_reasons
only in tool build.
I guess you may need some way to avoid doing double processing after the call to
load_record()
.
I am not sure about what do you mean of avoiding double processing.
load_record()
just retrieve the state, it not do any processing of animation, it means that skeleton pose or any other animation datas is not be saved as record.
May be we should add signals to notify users that there have a new record be saved (for auto recording).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to say that instead of recording these, it is necessary to set it up again after retrieving the parameter list with load_record()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I still not clearify what do you mean.
I have changed the retrieving order of Record
's members in load_record()
. Please review it.
a1a6e51
to
d152e5b
Compare
2a0b9d8
to
d1d1a70
Compare
d1d1a70
to
e1727b0
Compare
Impelent record featrure to instead of #77347.
AnimRecord.2.mp4
Bugsquad edited: Closes godotengine/godot-proposals#6807
Demo project:
animation_rollback_test.zip
Simple benchmark between
List<Record>
,LocalVector<Record>
andCircularDeque<Record>
for this use case: