diff --git a/tl/src/tl/events.cljs b/tl/src/tl/events.cljs index 6f1c6d4..0651288 100644 --- a/tl/src/tl/events.cljs +++ b/tl/src/tl/events.cljs @@ -177,12 +177,19 @@ (defn- reset-project [db] (merge db (select-keys db/default-db [:project :scene :fps :view :load :save-error :peers]))) +;; Leaving also drops the socket. It isn't just tidiness any more: an open +;; socket keeps us in the project's roster, so peers would go on seeing us +;; viewing something we walked away from — and its deltas would land in the +;; next project's db while that one loads. (rf/reg-event-fx ::nav-list (fn [{:keys [db]} _] {:db (-> (reset-project db) (assoc :page :list)) + :connect-scene nil :fetch-projects true})) (rf/reg-event-db ::set-projects (fn [db [_ ps]] (assoc db :projects ps :projects-error nil))) -(rf/reg-event-db ::nav-create (fn [db _] (-> (reset-project db) - (assoc :page :create :create-error nil)))) +(rf/reg-event-fx ::nav-create (fn [{:keys [db]} _] + {:db (-> (reset-project db) + (assoc :page :create :create-error nil)) + :connect-scene nil})) ;; loading a project is a three-hop chain: detail → OTIO file → scene, each ;; feeding the next, and any hop's failure lands the editor in :error. @@ -192,6 +199,7 @@ (assoc :page :editor) (assoc-in [:load :status] :loading) (assoc-in [:view :route-state] view-state)) + :connect-scene nil ; the previous project's, until ::project-ready :http-xhrio (api/GET (str "/api/projects/" id "/") {:on-success [::project-detail-loaded] :on-failure [::project-load-error]})})) @@ -373,20 +381,27 @@ (= me (party/responder r mine cid))) (assoc :peer/nav (nav-state db)))))) +;; Changing party drops anything parked: it's a position from the party we just +;; left, and replaying it later (on the way out of a form, say) would drag us +;; back to people we're no longer with. +(defn- set-party [db pid] + (-> db (assoc-in [:peers :roster (my-cid db) :party] pid) + (assoc-in [:peers :parked] nil))) + (rf/reg-event-fx ::join (fn [{:keys [db]} [_ cid]] (let [me (my-cid db) r (roster db)] (if (party/joinable? r me cid) - (let [db (assoc-in db [:peers :roster me :party] (party/join-id r cid))] + (let [db (set-party db (party/join-id r cid))] {:db db :peer/state (state-msg db cid)}) {:db db})))) (rf/reg-event-fx ::go-solo (fn [{:keys [db]} _] - (if-let [me (my-cid db)] - (let [db (assoc-in db [:peers :roster me :party] nil)] + (if (my-cid db) + (let [db (set-party db nil)] {:db db :peer/state (state-msg db)}) {:db db}))) @@ -396,14 +411,38 @@ (let [db (assoc-in db [:peers :joinable] (boolean on?))] {:db db :peer/state (state-msg db) :peer/remember-joinable (boolean on?)}))) +;; Nav is the only message that steers another browser, and it arrives relayed +;; from whatever a peer chose to send. Nothing downstream should have to cope +;; with a shape it can't use — a bad frame reaching scene/assert-frame would +;; throw inside the event loop and take the editor down with it. +(defn- nav-stack + "The peer's stack as context keywords, or nil if it isn't one we could stand + in: it has to start at the root and name timelines all the way down." + [stack] + (let [ks (when (sequential? stack) (mapv #(when (string? %) (keyword %)) stack))] + (when (and (seq ks) (= :root (first ks)) (every? some? ks)) ks))) + +(defn- nav-frame [n] + (when (and (number? n) (not (neg? n)) (== n (js/Math.floor n))) n)) + +(defn- standable? [db gid] + (contains? #{:timeline :annotation} (get-in db [:scene :groups gid :type]))) + ;; A party-mate moved. We take their whole view — which timeline, where in it, ;; rolling or not — because in a party anyone drives. (rf/reg-event-fx ::peer-nav - (fn [{:keys [db]} [_ {:keys [cid stack playhead playing] :as msg}]] - (let [stack (mapv keyword stack)] + (fn [{:keys [db]} [_ {:keys [cid playing] :as msg}]] + (let [stack (nav-stack (:stack msg)) + playhead (nav-frame (:playhead msg))] (cond - (not (party/mirrors? (roster db) (my-cid db) cid)) {:db db} + (not (party/mirrors? (roster db) (my-cid db) cid)) + ;; not (or no longer) one of ours — and if we were holding their move + ;; from when they were, it's stale now + {:db (cond-> db (= cid (get-in db [:peers :parked :cid])) + (assoc-in [:peers :parked] nil))} + + (not (and stack playhead)) {:db db} ; nothing we can act on ;; Two reasons to hold a move rather than take it, and one answer to ;; both — park the latest one and replay it when the reason clears: @@ -412,8 +451,7 @@ ;; they made it a moment ago and its scene delta is still in flight ;; (::peer-delta replays). Truncating to an ancestor instead would ;; strand us somewhere they aren't. - (or (authoring? db) - (not (and (seq stack) (every? #(get-in db [:scene :groups %]) stack)))) + (or (authoring? db) (not (every? #(standable? db %) stack))) {:db (assoc-in db [:peers :parked] msg)} :else @@ -424,8 +462,7 @@ (assoc-in [:peers :parked] nil) (assoc-in [:peers :echo] transport) (assoc-in [:view :stack] stack) - (assoc-in [:view :playheads ctx] - (scene/assert-frame "peer playhead" playhead)))] + (assoc-in [:view :playheads ctx] playhead))] (cond-> (merge (sync-route {:db db} db) (seek-fx db)) (and transport playing) (assoc :player/play true) (and transport (not playing)) (assoc :player/pause true))))))) diff --git a/tl/src/tl/views.cljs b/tl/src/tl/views.cljs index 28c8d27..8c5ba0f 100644 --- a/tl/src/tl/views.cljs +++ b/tl/src/tl/views.cljs @@ -188,11 +188,15 @@ ;; effects the stack events use to keep the player on the current context (rf/reg-fx :player/pause (fn [_] (when-let [v @video-el] (.pause v)))) -;; a party-mate started playback. .play() can be refused (autoplay policy) — -;; swallow it rather than leaving an unhandled rejection; we stay paused. -(rf/reg-fx :player/play (fn [_] (when-let [v @video-el] - (when (.-paused v) - (some-> (.play v) (.catch (fn [_] nil))))))) +;; A party-mate started playback. .play() can be refused (autoplay policy, if +;; this tab has had no interaction yet) — say so rather than leaving an +;; unhandled rejection, because ::set-playing is also what clears the flag that +;; stops us echoing a mirrored play back at them. +(rf/reg-fx :player/play (fn [_] (when-let [v @video-el] + (when (.-paused v) + (some-> (.play v) + (.catch (fn [_] + (rf/dispatch [::events/set-playing false])))))))) (rf/reg-fx :player/seek (fn [secs] (when (and @video-el secs) (set! (.-currentTime @video-el) secs)))) (defn- ann-scroll-to? diff --git a/tl/test/tl/flow_test.cljs b/tl/test/tl/flow_test.cljs index 2b54202..162f5dc 100644 --- a/tl/test/tl/flow_test.cljs +++ b/tl/test/tl/flow_test.cljs @@ -373,3 +373,47 @@ (rf/dispatch [::ev/edit-here :a1]) ; pops to :root to expose marks (is (= [:root] @(rf/subscribe [::subs/stack]))) (is (= [] @sent)))) + +(deftest a-malformed-nav-is-ignored-not-obeyed-and-never-throws + ;; nav is the only message that steers another browser, and it arrives + ;; relayed from whatever a peer chose to send + (rf-test/run-test-sync + (setup! (seed) [:root :a1]) + (peers! "me" ["you"]) + (doseq [bad [{:stack ["root" "a1"] :playhead 10.5} ; not a frame + {:stack ["root" "a1"] :playhead -4} + {:stack ["root" "a1"] :playhead nil} + {:stack ["a1"] :playhead 0} ; doesn't start at the root + {:stack [] :playhead 0} + {:stack nil :playhead 0} + {:stack ["root" 7] :playhead 0}]] + (rf/dispatch [::ev/peer-nav (assoc bad :cid "you" :playing false)]) + (is (= [:root :a1] @(rf/subscribe [::subs/stack])) (pr-str bad)) + (is (nil? (get-in @rdb/app-db [:peers :parked])) (pr-str bad))) + (testing "and a context we could never stand in is held, not entered" + ;; :a is a clip — it exists, so it is not a not-yet-synced group + (rf/dispatch [::ev/peer-nav {:cid "you" :stack ["root" "a"] :playhead 0 :playing false}]) + (is (= [:root :a1] @(rf/subscribe [::subs/stack])))))) + +(deftest leaving-the-party-drops-a-move-we-were-holding + (rf-test/run-test-sync + (setup! (seed) [:root]) + (peers! "me" ["you"]) + (rf/dispatch [::ev/edit-annotation :a1]) ; detached: their move parks + (rf/dispatch [::ev/peer-nav {:cid "you" :stack ["root" "b1"] :playhead 3 :playing false}]) + (is (some? (get-in @rdb/app-db [:peers :parked]))) + (rf/dispatch [::ev/go-solo]) ; …but then we leave + (is (nil? (get-in @rdb/app-db [:peers :parked]))) + (rf/dispatch [::ev/save-group :a1 (group :a1) (group :a1)]) + (rf/dispatch [::ev/finish-edit]) + (is (= [:root] @(rf/subscribe [::subs/stack])) "not dragged back to people we left"))) + +(deftest a-held-move-from-someone-who-left-the-party-is-dropped-on-replay + (rf-test/run-test-sync + (setup! (seed) [:root]) + (peers! "me" ["you"]) + (rf/dispatch [::ev/edit-annotation :a1]) + (rf/dispatch [::ev/peer-nav {:cid "you" :stack ["root" "b1"] :playhead 3 :playing false}]) + (swap! rdb/app-db assoc-in [:peers :roster "you" :party] nil) ; they went solo + (rf/dispatch [::ev/peer-delta {}]) ; triggers the replay + (is (nil? (get-in @rdb/app-db [:peers :parked])))))