4
\$\begingroup\$

Working on a little Sprite Kit game, I had the problem that I wanted a couple of things to wait until a group of sprite nodes had stopped their animation.

I decided to use a GCD dispatch group to monitor the sprites' actions. The nodes that I was concerned about were all instances of the same class, so I gave that class a private method vending a dispatch_group_t that the instances can enter and leave as they started and ended their animations.

The class has two public methods that allow checking the status of the animations: one that blocks until the dispatch group is empty, and one that simply returns the group's busy status.

@interface WSSNode : SKSpriteNode

/** The WSSNode class tracks whether any instances are running animations,
 *  in order to avoid overlapping other actions. 
 *  +waitOnAllNodeMovement blocks when called until all nodes have 
 *  completed their animations.
 */
+ (void)waitOnAllNodeMovement;

/** The WSSNode class tracks whether any instances are running animations,
 *  in order to avoid overlapping other actions. 
 *  +anyNodeMovementInProgress returns a BOOL immediately, indicating 
 *  whether any animations are currently running.
 */
+ (BOOL)anyNodeMovementInProgress;

/* Sample method: make the node do something that requires waiting on. */
- (void)moveToPosition:(CGPoint)destination;

@end

@interface WSSNode ()

/** Exposes the private dispatch group that the class uses to track and
 *  report on running animations. 
 */
+ (dispatch_group_t)movementDispatchGroup;

@end

@implementation WSSNode

+ (void)waitOnAllNodeMovement
{
    dispatch_group_wait([self movementDispatchGroup], 
                        DISPATCH_TIME_FOREVER);
}

+ (BOOL)anyNodeMovementInProgress
{
    // Return immediately regardless of state of group, but indicate 
    // whether group is empty or timeout occurred.
    return (0 != dispatch_group_wait([self movementDispatchGroup], 
                                     DISPATCH_TIME_NOW));
}

+ (dispatch_group_t)movementDispatchGroup
{
    static dispatch_group_t group;

    static dispatch_once_t onceToken;
    dispatch_once(&onceToken, ^{
        group = dispatch_group_create();
    });

    return group;
}

- (void)moveToPosition:(CGPoint)destination
{   
    // No need to actually enqueue anything; simply increment/decrement
    // group counter manually.
    dispatch_group_enter([WSSNode movementDispatchGroup]);
    [self runAction:/* whatever */
         completion:^{ dispatch_group_leave([WSSNode movementDispatchGroup])}];
}

@end

An example of the intended usage of +anyNodeMovementInProgress would be:

- (void)keyDown:(NSEvent *)theEvent
{
    // Don't accept input while movement is taking place.
    if( [WSSNode anyNodeMovementInProgress] ){
        return;
    }
    // ...
}

The three things that I'm specifically interested to have reviewed are:

  • I think that the simplicity of the solution makes concurrency problems unlikely, but I'm far from an expert on the subject. Is there any potential concurrency problem here?
  • Is the interface for waiting/checking status clear and usable? Do the methods do what you expect given their names?
    • Sub-point: are the doc comments clear and thorough enough?
  • The way that the "wait" functionality is exposed makes users of the class have to do a little dispatch queue hop to avoid blocking the main thread:

    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0),
        ^{
            [WSSNode waitOnAllNodeMovement];
            dispatch_async(dispatch_get_main_queue(), ^{
              // Action that needs to wait for animation to finish
         });
    });
    

    Would this be more palatable if the WSSNode class accepted a Block to be run after the group is empty? I didn't like that idea because it seemed like a muddying of responsibility, but I'm interested in contrary thoughts.

I posted this solution on Stack Overflow, including my reasoning for not using a seemingly-more-straightforward mechanism such as a dispatch semaphore or queue with a barrier Block.

If the nodes incremented the semaphore when they were created, decremented when animating, and incremented again when done, the other task would only wait if all created nodes were animating, and wouldn't wait any longer than the first completion. If the nodes didn't increment the semaphore initially, then only one of them could function at a time.

The barrier Block wouldn't work because I didn't know when I could actually enqueue it. At the point where I decided to enqueue the "after" task, no "before" tasks could be added.

Suggestions on that: that is, alternate mechanisms and/or flaws in my reasoning are also welcome.

\$\endgroup\$
2
  • \$\begingroup\$ Upon first reading, the only thing I could think was "why not callback blocks?", and I think I'm still leaning that way. If I think I can make a convincing case, I'll post an answer. Alternatively, if I convince myself that this approach is right, I may similarly post an answer reaffirming this decision of yours. \$\endgroup\$
    – nhgrif
    Mar 8, 2016 at 4:57
  • \$\begingroup\$ I look forward to it either way, @nhgrif. \$\endgroup\$
    – jscs
    Mar 8, 2016 at 5:05

1 Answer 1

1
\$\begingroup\$

I've been chewing on this for a while now. I can't really convince myself one way or the other whether the blocking wait method is better or worse than blocks.

So, my thought is this.. in addition to waitOnAllNodeMovement, which in the end, seems fine, why not provide at least one non-blocking option to allow users a cleaner way to execute code after node movement is completed.

A method that accepts a block to execute when all node movements are completed could work.

typedef void (^AfterNodeMovementBlock)(void);

- (void)performAfterAllNodeMovement:(AfterNodeMovementBlock)block;

Or alternatively, we might declare a constant string that is used as a notification name to subscribe to, and the notification name is posted whenever all node movement is completed.

\$\endgroup\$
2
  • \$\begingroup\$ Yes, I can see this making sense as the more straightforward alternative to the double dispatch I mentioned in the question. Just have to document that the AfterNodeMovementBlock will be run on the main queue. \$\endgroup\$
    – jscs
    Mar 21, 2016 at 18:30
  • 1
    \$\begingroup\$ Indeed. Or take an NSOperationQueue approach and allow the user to pass in a queue to run on. \$\endgroup\$
    – nhgrif
    Mar 21, 2016 at 18:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.