fix: four real defects in the party plumbing
Leaving a project never closed its socket. That was untidy before; with presence it means peers go on seeing you viewing something you walked away from, and the old project's deltas land in the next one's db while it loads. ::nav-list, ::nav-create and ::open-project now drop it. A parked move belongs to the party it came from. Leaving one — or the sender leaving it — left the park behind, so closing a form afterwards would drag you back to people you were no longer with. Changing party drops it, and a replay from someone who is no longer a mate drops it too. Nav is the only message that steers another browser, and it arrives relayed from whatever a peer chose to send. A non-integer playhead reaching scene/assert-frame would have thrown inside the event loop and taken the editor down; a stack naming a clip would have dropped you into a context you can't stand in. Validate the shape up front: root-first, timelines all the way down, a real frame — and ignore what fails rather than parking it forever. A refused .play() (autoplay policy, in a tab with no interaction yet) left the echo flag set, swallowing the next genuine play we should have broadcast. The effect now reports the refusal, which both clears the flag and keeps :playing? honest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
60c4096d2f
commit
1ae4251a34
3 changed files with 102 additions and 17 deletions
|
|
@ -177,12 +177,19 @@
|
||||||
(defn- reset-project [db]
|
(defn- reset-project [db]
|
||||||
(merge db (select-keys db/default-db [:project :scene :fps :view :load :save-error :peers])))
|
(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
|
(rf/reg-event-fx ::nav-list
|
||||||
(fn [{:keys [db]} _] {:db (-> (reset-project db) (assoc :page :list))
|
(fn [{:keys [db]} _] {:db (-> (reset-project db) (assoc :page :list))
|
||||||
|
:connect-scene nil
|
||||||
:fetch-projects true}))
|
:fetch-projects true}))
|
||||||
(rf/reg-event-db ::set-projects (fn [db [_ ps]] (assoc db :projects ps :projects-error nil)))
|
(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)
|
(rf/reg-event-fx ::nav-create (fn [{:keys [db]} _]
|
||||||
(assoc :page :create :create-error nil))))
|
{: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
|
;; 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.
|
;; feeding the next, and any hop's failure lands the editor in :error.
|
||||||
|
|
@ -192,6 +199,7 @@
|
||||||
(assoc :page :editor)
|
(assoc :page :editor)
|
||||||
(assoc-in [:load :status] :loading)
|
(assoc-in [:load :status] :loading)
|
||||||
(assoc-in [:view :route-state] view-state))
|
(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 "/")
|
:http-xhrio (api/GET (str "/api/projects/" id "/")
|
||||||
{:on-success [::project-detail-loaded]
|
{:on-success [::project-detail-loaded]
|
||||||
:on-failure [::project-load-error]})}))
|
:on-failure [::project-load-error]})}))
|
||||||
|
|
@ -373,20 +381,27 @@
|
||||||
(= me (party/responder r mine cid)))
|
(= me (party/responder r mine cid)))
|
||||||
(assoc :peer/nav (nav-state db))))))
|
(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
|
(rf/reg-event-fx
|
||||||
::join
|
::join
|
||||||
(fn [{:keys [db]} [_ cid]]
|
(fn [{:keys [db]} [_ cid]]
|
||||||
(let [me (my-cid db) r (roster db)]
|
(let [me (my-cid db) r (roster db)]
|
||||||
(if (party/joinable? r me cid)
|
(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 :peer/state (state-msg db cid)})
|
||||||
{:db db}))))
|
{:db db}))))
|
||||||
|
|
||||||
(rf/reg-event-fx
|
(rf/reg-event-fx
|
||||||
::go-solo
|
::go-solo
|
||||||
(fn [{:keys [db]} _]
|
(fn [{:keys [db]} _]
|
||||||
(if-let [me (my-cid db)]
|
(if (my-cid db)
|
||||||
(let [db (assoc-in db [:peers :roster me :party] nil)]
|
(let [db (set-party db nil)]
|
||||||
{:db db :peer/state (state-msg db)})
|
{:db db :peer/state (state-msg db)})
|
||||||
{:db db})))
|
{:db db})))
|
||||||
|
|
||||||
|
|
@ -396,14 +411,38 @@
|
||||||
(let [db (assoc-in db [:peers :joinable] (boolean on?))]
|
(let [db (assoc-in db [:peers :joinable] (boolean on?))]
|
||||||
{:db db :peer/state (state-msg db) :peer/remember-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,
|
;; A party-mate moved. We take their whole view — which timeline, where in it,
|
||||||
;; rolling or not — because in a party anyone drives.
|
;; rolling or not — because in a party anyone drives.
|
||||||
(rf/reg-event-fx
|
(rf/reg-event-fx
|
||||||
::peer-nav
|
::peer-nav
|
||||||
(fn [{:keys [db]} [_ {:keys [cid stack playhead playing] :as msg}]]
|
(fn [{:keys [db]} [_ {:keys [cid playing] :as msg}]]
|
||||||
(let [stack (mapv keyword stack)]
|
(let [stack (nav-stack (:stack msg))
|
||||||
|
playhead (nav-frame (:playhead msg))]
|
||||||
(cond
|
(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
|
;; 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:
|
;; 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
|
;; they made it a moment ago and its scene delta is still in flight
|
||||||
;; (::peer-delta replays). Truncating to an ancestor instead would
|
;; (::peer-delta replays). Truncating to an ancestor instead would
|
||||||
;; strand us somewhere they aren't.
|
;; strand us somewhere they aren't.
|
||||||
(or (authoring? db)
|
(or (authoring? db) (not (every? #(standable? db %) stack)))
|
||||||
(not (and (seq stack) (every? #(get-in db [:scene :groups %]) stack))))
|
|
||||||
{:db (assoc-in db [:peers :parked] msg)}
|
{:db (assoc-in db [:peers :parked] msg)}
|
||||||
|
|
||||||
:else
|
:else
|
||||||
|
|
@ -424,8 +462,7 @@
|
||||||
(assoc-in [:peers :parked] nil)
|
(assoc-in [:peers :parked] nil)
|
||||||
(assoc-in [:peers :echo] transport)
|
(assoc-in [:peers :echo] transport)
|
||||||
(assoc-in [:view :stack] stack)
|
(assoc-in [:view :stack] stack)
|
||||||
(assoc-in [:view :playheads ctx]
|
(assoc-in [:view :playheads ctx] playhead))]
|
||||||
(scene/assert-frame "peer playhead" playhead)))]
|
|
||||||
(cond-> (merge (sync-route {:db db} db) (seek-fx db))
|
(cond-> (merge (sync-route {:db db} db) (seek-fx db))
|
||||||
(and transport playing) (assoc :player/play true)
|
(and transport playing) (assoc :player/play true)
|
||||||
(and transport (not playing)) (assoc :player/pause true)))))))
|
(and transport (not playing)) (assoc :player/pause true)))))))
|
||||||
|
|
|
||||||
|
|
@ -188,11 +188,15 @@
|
||||||
|
|
||||||
;; effects the stack events use to keep the player on the current context
|
;; 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))))
|
(rf/reg-fx :player/pause (fn [_] (when-let [v @video-el] (.pause v))))
|
||||||
;; a party-mate started playback. .play() can be refused (autoplay policy) —
|
;; A party-mate started playback. .play() can be refused (autoplay policy, if
|
||||||
;; swallow it rather than leaving an unhandled rejection; we stay paused.
|
;; 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]
|
(rf/reg-fx :player/play (fn [_] (when-let [v @video-el]
|
||||||
(when (.-paused v)
|
(when (.-paused v)
|
||||||
(some-> (.play v) (.catch (fn [_] nil)))))))
|
(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))))
|
(rf/reg-fx :player/seek (fn [secs] (when (and @video-el secs) (set! (.-currentTime @video-el) secs))))
|
||||||
|
|
||||||
(defn- ann-scroll-to?
|
(defn- ann-scroll-to?
|
||||||
|
|
|
||||||
|
|
@ -373,3 +373,47 @@
|
||||||
(rf/dispatch [::ev/edit-here :a1]) ; pops to :root to expose marks
|
(rf/dispatch [::ev/edit-here :a1]) ; pops to :root to expose marks
|
||||||
(is (= [:root] @(rf/subscribe [::subs/stack])))
|
(is (= [:root] @(rf/subscribe [::subs/stack])))
|
||||||
(is (= [] @sent))))
|
(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])))))
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue