Antidote-for-Tox / objcTox

No longer maintained
Mozilla Public License 2.0
37 stars 12 forks source link

Chuongv/additional pause logic #64

Closed Chuongv closed 9 years ago

Chuongv commented 9 years ago

I added 2 extra properties to OCTCall so I can keep track of who initiated the pause of the call. In addition it lets me know when I can move call from OCTCallPause to OCTCallActive. There's probably a better way of improving this so the code could be more readable?

Chuongv commented 9 years ago

Looks like some issue with Slather and Cocoapods https://github.com/venmo/slather/issues/94

dvor commented 9 years ago

Hm, will try to fix that. It seems that for now we won't rely on travis. Don't forget to uncrustify :-)

dvor commented 9 years ago

This is quite messy. I don't like pausedByYou and pausedByFriend statuses, they look quite redundant to me. Also code gets much complicated, thing like that doesn't look clean and nice:

if (wasPaused &&
    call.pausedByFriend &&
    ! call.pausedByYou &&
    (state != OCTToxAVFriendCallStatePaused) ) {
    status = OCTCallStatusActive;
}

Do we really need to know who paused the call? If yes, I'll think about simplifying it tomorrow.

Chuongv commented 9 years ago

Do we really need to know who paused the call? If yes, I'll think about simplifying it tomorrow.

Unfortunately yes :disappointed: . Here's why..

Alice is in a call with Bob.. Alice sends call control pause to Bob, call is now in OCTCallStatusPause. Bob sends call control pause, and call is still in OCTCallStatusPause. Bob sends call control resume, and now OCTSubmanagerCalls needs to know whether or not to move the call to OCTCallStatusActive

I agree, it's a little messy and I'm scratching my head to find out a better way to handle this?

dvor commented 9 years ago

Are you sure this scenario will work? Documentation says:

   * Resume a previously paused call. Only valid if the pause was caused by this
   * client, if not, this control is ignored. Not valid before the call is accepted.
   */
  TOXAV_CALL_CONTROL_RESUME,

It seems that it is possible for Bob to pause call and the unpause it. However I want to make sure it works.

Chuongv commented 9 years ago

If call was Paused by Alice, and Bob sends control Pause and then Resume, the call will still be Paused. Is that what you are asking?

dvor commented 9 years ago

Here is proposal how to simplify this (if you have any thoughts it is better to discuss before start implementing). I think we can

/**
 * This property contains paused status for Active call.
 */
@property (nonatomic, assign) OCTCallPausedStatus pausedStatus;

typedef NS_OPTIONS(NSInteger, OCTCallPausedStatus) {
    OCTCallPausedStatusNone = 0,
    OCTCallPausedStatusByUser = 1 << 0,
    OCTCallPausedStatusByFriend = 1 << 1,
};

This way we will have single one paused status instead of several.

Also maybe you will be able to simplify other code when having only one status. And it would be nice to have some unit tests for that peace of code (you can wait for my approval first so you won't rewrite tests several times).

Chuongv commented 9 years ago

I think that works out well. I can't think of any other way.

Chuongv commented 9 years ago

Will reopen better implementation