-
Notifications
You must be signed in to change notification settings - Fork 40
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
Game 18568 fast note creation ownership #72
base: dynamite-lib
Are you sure you want to change the base?
Conversation
@gomezll Please review, needs cleanup and I'll point out an issue in the comments... |
# Emitted when a new note or reply will be created. The creation request id | ||
# is sent and will match the id sent with the entity_created signal. | ||
# Todo: This has functional overlap with the pre_submit_callback...resolve that... | ||
new_entity_requested = QtCore.Signal(str, int) |
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.
@gomezll I added these extra signals and functionality before I realized there is already a mechanism that supplies similar functionality - the pre_submit_callback
. Imo the existing callback is a bit odd since it passes the NoteInputWidget
object as its parameter which feels a bit arbitrary, and some work would be needed to use that one since the pending request UID would need to be exposed through a function and that also seems a bit random.
I took the approach of adding new signals specific for the exact event type that we're trying to track (shotgun note creations and external user creations (in activity_stream.py), which seems nice and explicit but also makes things a bit busier.
Interested to see what you think.
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.
Your approach with new signals seems justified and appropriate to me Ed. Looks good
SG3DR needs to immediately capture the application state when a note is created, not wait until the shotgun-id is available.
When a note is created by the application, this change adds the ability for the application to pass a userdata object along with the creation request that is returned in the callback for that note creation.
When a note is created by the toolkit, e.g. by the user using the
NoteInputWidget
, new signals have been added to immediately notify the application that a note is being create, and passing note creation task Ids with the creation request and when the note is finally created.These two pathways allow the application to synchronize and match requests for new notes with the creation confirmation callbacks.
This fixes bugs when rapidly creating notes where the application state would change and shotgun notes would become queued and take a long time to create, resulting in state mismatch of when the note was originally created.