Hi Stefan, looks interesting.
I think I would have kept using iview->container plus a few more casts instead
of introducing BstTrackView.song, to ensure that iview->container and
BstTrackView.song cannot get out of sync.
Your approach might be more convenient and I guess the
bst_item_view_set_container() internals ensure that the two members are always
set/reset in sync...
There's one other bit missing though. WIth the old proxy interface, signal
connections *always* come in (connect+disconnect) pairs, I'll quote the
relevant code lines:
Before your PR:
bse_proxy_disconnect (song.proxy_id(),
"any_signal", track_view_repeat_changed, self,
NULL);
[...]
bse_proxy_connect (song.proxy_id(),
"swapped_signal::property-notify::loop-enabled",
track_view_repeat_changed, self,
NULL);
After your PR:
bse_proxy_disconnect (self->song.proxy_id(),
"any_signal", track_view_repeat_changed, self,
NULL);
self->song = NULL; /* disconnect */
[...]
self->song.on ("notify:loop_enabled", [self]() { track_view_repeat_changed
(self); });
bse_proxy_connect (self->song.proxy_id(),
NULL);
I.e. you have replaced proxy_connect() with on(), added an event handler
disconnect via =NULL but forgot to remove the proxy_disconnect line for
track_view_repeat_changed that belongs to the proxy_connect() you removed.
Also please reword "/* disconnect */" to be more explicit: "// disconnects
event handlers"
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/tim-janik/beast/pull/64#issuecomment-415369454_______________________________________________
beast mailing list
[email protected]
https://mail.gnome.org/mailman/listinfo/beast
