feature/23-voice-room/poc #93
@@ -1,12 +1,14 @@
|
||||
<script lang="ts">
|
||||
import type {Readable} from "svelte/store"
|
||||
import {readable} from "svelte/store"
|
||||
import cx from "classnames"
|
||||
import {removeUndefined} from "@welshman/lib"
|
||||
import {ifLet, removeUndefined} from "@welshman/lib"
|
||||
import {deriveProfile} from "@welshman/app"
|
||||
import UserRounded from "@assets/icons/user-rounded.svg?dataurl"
|
||||
import ImageIcon from "@lib/components/ImageIcon.svelte"
|
||||
|
||||
type Props = {
|
||||
pubkey: string
|
||||
pubkey?: string
|
||||
class?: string
|
||||
size?: number
|
||||
url?: string
|
||||
@@ -14,11 +16,13 @@
|
||||
|
||||
const {pubkey, url, size = 7, ...props}: Props = $props()
|
||||
|
||||
const profile = deriveProfile(pubkey, removeUndefined([url]))
|
||||
const readableProfile = ifLet(pubkey, pk => deriveProfile(pk, removeUndefined([url])))
|
||||
const emptyProfile = readable(undefined)
|
||||
const profile: Readable<{picture?: string} | undefined> = readableProfile ?? emptyProfile
|
||||
|
|
||||
</script>
|
||||
|
||||
<ImageIcon
|
||||
{size}
|
||||
alt=""
|
||||
class={cx(props.class, "rounded-full")}
|
||||
src={$profile?.picture || UserRounded} />
|
||||
src={$profile?.picture ?? UserRounded} />
|
||||
|
||||
@@ -4,10 +4,10 @@
|
||||
import Icon from "@lib/components/Icon.svelte"
|
||||
import SecondaryNavItem from "@lib/components/SecondaryNavItem.svelte"
|
||||
import RoomNameWithImage from "@app/components/RoomNameWithImage.svelte"
|
||||
import VoiceRoomItem from "@app/components/VoiceRoomItem.svelte"
|
||||
import {deriveRoom, deriveShouldNotify, getRoomType, RoomType} from "@app/core/state"
|
||||
import {notifications} from "@app/util/notifications"
|
||||
import {makeRoomPath} from "@app/util/routes"
|
||||
import {joinVoiceRoom, currentVoiceSession} from "@app/voice"
|
||||
|
||||
interface Props {
|
||||
url: any
|
||||
@@ -24,21 +24,18 @@
|
||||
const shouldNotifyForSpace = deriveShouldNotify(url)
|
||||
const shouldNotifyForRoom = deriveShouldNotify(url, h)
|
||||
const showDifferenceIcon = $derived($shouldNotifyForRoom !== $shouldNotifyForSpace)
|
||||
|
||||
const handleClick = () => {
|
||||
if (roomType !== RoomType.Voice) return
|
||||
if ($currentVoiceSession?.url === url && $currentVoiceSession?.h === h) return
|
||||
void joinVoiceRoom(url, h)
|
||||
}
|
||||
</script>
|
||||
|
hodlbod marked this conversation as resolved
Outdated
hodlbod
commented
I can see why this would be confusing, but we should figure out what to do about muted rooms still I can see why this would be confusing, but we should figure out what to do about muted rooms still
mplorentz
commented
Oh yes that's my mistake. How about we use a bell icon for notifications and speaker icon for voice rooms? Oh yes that's my mistake. How about we use a bell icon for notifications and speaker icon for voice rooms?
hodlbod
commented
sure, that makes sense sure, that makes sense
|
||||
|
||||
<SecondaryNavItem
|
||||
href={path}
|
||||
{replaceState}
|
||||
onclick={handleClick}
|
||||
notification={notify ? $notifications.has(path) : false}>
|
||||
<RoomNameWithImage {url} {h} />
|
||||
{#if showDifferenceIcon}
|
||||
<Icon icon={$shouldNotifyForRoom ? Bell : BellOff} size={4} class="opacity-50" />
|
||||
{/if}
|
||||
</SecondaryNavItem>
|
||||
{#if roomType === RoomType.Voice}
|
||||
<VoiceRoomItem {url} {h} />
|
||||
{:else}
|
||||
<SecondaryNavItem
|
||||
href={path}
|
||||
{replaceState}
|
||||
notification={notify ? $notifications.has(path) : false}>
|
||||
<RoomNameWithImage {url} {h} />
|
||||
{#if showDifferenceIcon}
|
||||
<Icon icon={$shouldNotifyForRoom ? Bell : BellOff} size={4} class="opacity-50" />
|
||||
{/if}
|
||||
</SecondaryNavItem>
|
||||
{/if}
|
||||
|
||||
@@ -11,7 +11,9 @@
|
||||
joinVoiceRoom,
|
||||
leaveVoiceRoom,
|
||||
currentVoiceSession,
|
||||
speakingPubkeys,
|
||||
isParticipantSpeaking,
|
||||
participantKey,
|
||||
type VoiceParticipant,
|
||||
} from "@app/voice"
|
||||
|
||||
interface Props {
|
||||
@@ -46,8 +48,8 @@
|
||||
}
|
||||
|
||||
$effect(() => {
|
||||
for (const pk of $participants) {
|
||||
loadProfile(pk)
|
||||
for (const p of $participants) {
|
||||
|
hodlbod marked this conversation as resolved
Outdated
hodlbod
commented
Why do the bots do this? I have seen this cause bugs more than once. Just do Why do the bots do this? I have seen this cause bugs more than once. Just do `e.message || String(e)`. Or, better yet, actually design the error types and don't put anything that gets thrown in a message to the user. Just tell them "Failed to join voice room" and log it if it's not an error we know what to do with.
mplorentz
commented
I am cleaning this up but I want to fully understand you. Can you explain how this causes bugs? As a non-javascript guy this doesn't look pretty but makes sense to me because I understand the current code risks showing raw error messages to the user (which doesn't bother me actually as long as it has a nice prefix like "Failed to join voice room"). Is there something worse that I am missing? I am cleaning this up but I want to fully understand you. Can you explain how this causes bugs? As a non-javascript guy this doesn't look pretty but makes sense to me because `e` could literally be any type and there is no way to write a catch block for a specific type, so it seems reasonably defensive to check the type before just grabbing the `message` property off of whatever might have been thrown. Or in the case where some library throws a String it seems reasonable to surface that.
I understand the current code risks showing raw error messages to the user (which doesn't bother me actually as long as it has a nice prefix like "Failed to join voice room"). Is there something worse that I am missing?
mplorentz
commented
Idk maybe it is stupid to think that some library would throw some random object with a Idk maybe it is stupid to think that some library would throw some random object with a `message` property, but anything seems possible when `node_modules` are involved 😓
mplorentz
commented
Oh interesting, Oh interesting, `e.message || String(e)` actually gives a typescript error: `'e' is of type 'unknown'.`. But AI found the `errorMessage()` function from `@lib/util` so I'll use that.
hodlbod
commented
The problem is when a pojo with a message gets thrown or rejected, which means the object gets cast to a string, which fails to show the message. This is the case when you probably actually want to show the message to the user, whereas showing an error's message is almost always confusing. Errors being completely untyped are a huge flaw in typescript, so I like to save throws for truly exceptional cases (https://effect.website/ does this more formally, and I think it's a good idea), in which case you want to log and recover if you can, and resolve/return everything else (even errors that are deferred to the caller). But I'm certainly not consistent about this.
The problem is when a pojo with a message gets thrown or rejected, which means the object gets cast to a string, which fails to show the message. This is the case when you probably actually want to show the message to the user, whereas showing an error's message is almost always confusing. Errors being completely untyped are a huge flaw in typescript, so I like to save throws for truly exceptional cases (https://effect.website/ does this more formally, and I think it's a good idea), in which case you want to log and recover if you can, and resolve/return everything else (even errors that are deferred to the caller). But I'm certainly not consistent about this.
`errorMessage` was introduced by an LLM too (via Tyson), I don't really believe in it either. I think we should just avoid propagating thrown errors to the user (I don't like the "Failed to do x: [technical jargon]" pattern. It helps to debug some, but not really in that many cases.
|
||||
if (p.pubkey) loadProfile(p.pubkey)
|
||||
}
|
||||
})
|
||||
</script>
|
||||
@@ -65,17 +67,17 @@
|
||||
<RoomName {url} {h} />
|
||||
</div>
|
||||
{#if $participants.length > 0}
|
||||
{#each $participants as pk (pk)}
|
||||
{#each $participants as p (participantKey(p as VoiceParticipant))}
|
||||
<div class="flex items-center gap-2 ml-6">
|
||||
<div
|
||||
class={cx(
|
||||
"inline-flex shrink-0 items-center justify-center rounded-full transition-shadow",
|
||||
isActive && $speakingPubkeys.has(pk) && "ring-2 ring-success",
|
||||
isActive && $isParticipantSpeaking(p) && "ring-2 ring-success",
|
||||
)}>
|
||||
<ProfileCircle pubkey={pk} size={5} class="h-5 w-5" />
|
||||
<ProfileCircle pubkey={p.pubkey} size={5} class="h-5 w-5" />
|
||||
</div>
|
||||
<span class="ellipsize text-xs opacity-70">
|
||||
|
hodlbod marked this conversation as resolved
Outdated
hodlbod
commented
There's a typescript error here because pubkey may be undefined. Is there a reason for this or can we strengthen the type to guarantee it? There's a typescript error here because pubkey may be undefined. Is there a reason for this or can we strengthen the type to guarantee it?
mplorentz
commented
Yes, in the case where there is a voice participant in the room but we can't derive a Yes, in the case where there is a voice participant in the room but we can't derive a `pubkey` from their LiveKit identity string it seems important to still show that there is _someone_ in the room. I just wanted to reuse the default profile icon for that and this was my way of doing it.
|
||||
{displayProfileByPubkey(pk)}
|
||||
{p.pubkey ? displayProfileByPubkey(p.pubkey) : "Unknown"}
|
||||
</span>
|
||||
</div>
|
||||
{/each}
|
||||
|
||||
@@ -55,7 +55,7 @@ import {
|
||||
loadFeedsForPubkey,
|
||||
} from "@app/core/state"
|
||||
import {hasBlossomSupport} from "@app/core/commands"
|
||||
import {ROOM_PRESENCE} from "@app/voice"
|
||||
import {LIVEKIT_PARTICIPANTS} from "@app/voice"
|
||||
|
||||
// Utils
|
||||
|
||||
@@ -320,7 +320,7 @@ const syncSpace = (url: string, rooms: string[]) => {
|
||||
pullAndListen({
|
||||
url,
|
||||
signal: controller.signal,
|
||||
filters: [{kinds: [ROOM_PRESENCE]}],
|
||||
filters: [{kinds: [LIVEKIT_PARTICIPANTS]}],
|
||||
})
|
||||
|
||||
return () => controller.abort()
|
||||
|
||||
@@ -4,21 +4,19 @@
|
||||
*/
|
||||
import {DisconnectReason, Room, RoomEvent, Track} from "livekit-client"
|
||||
import {derived, get, writable} from "svelte/store"
|
||||
import {now} from "@welshman/lib"
|
||||
import {makeEvent, makeHttpAuth, makeHttpAuthHeader, getTagValue} from "@welshman/util"
|
||||
import {signer, publishThunk} from "@welshman/app"
|
||||
import {uniqBy} from "@welshman/lib"
|
||||
import type {TrustedEvent} from "@welshman/util"
|
||||
import {makeHttpAuth, makeHttpAuthHeader, getTags} from "@welshman/util"
|
||||
import {signer} from "@welshman/app"
|
||||
import {getLivekitEndpoint} from "$lib/livekit"
|
||||
import {AbortError, whenAborted, whenTimeout} from "$lib/util"
|
||||
import {deriveEventsForUrl} from "@app/core/state"
|
||||
import {deriveLatestEventForUrl} from "@app/core/state"
|
||||
import {pushToast} from "@app/util/toast"
|
||||
|
||||
export const ROOM_PRESENCE = 10312
|
||||
export const LIVEKIT_PARTICIPANTS = 39004
|
||||
|
||||
export {checkRelayHasLivekit} from "$lib/livekit"
|
||||
|
||||
const PRESENCE_INTERVAL_MS = 60_000
|
||||
const PRESENCE_EXPIRY_S = 300
|
||||
|
||||
export type VoiceSession = {
|
||||
url: string
|
||||
h: string
|
||||
@@ -26,9 +24,49 @@ export type VoiceSession = {
|
||||
muted: boolean
|
||||
}
|
||||
|
||||
export type Pubkey = string
|
||||
|
||||
export type VoiceParticipant = {pubkey?: Pubkey; identity: string}
|
||||
|
||||
export const currentVoiceSession = writable<VoiceSession | undefined>(undefined)
|
||||
|
||||
export const speakingPubkeys = writable(new Set<string>())
|
||||
export const participantPubkeyMap = writable<Map<string, Pubkey>>(new Map())
|
||||
|
||||
const addParticipant = (identity: string) => {
|
||||
participantPubkeyMap.update(m => {
|
||||
const next = new Map(m)
|
||||
next.set(identity, pubkeyFromLiveKitIdentity(identity) ?? "")
|
||||
return next
|
||||
})
|
||||
}
|
||||
|
||||
const deleteParticipant = (identity: string) => {
|
||||
participantPubkeyMap.update(m => {
|
||||
const next = new Map(m)
|
||||
next.delete(identity)
|
||||
return next
|
||||
})
|
||||
}
|
||||
|
hodlbod marked this conversation as resolved
Outdated
hodlbod
commented
[nit] simpler would be [nit] simpler would be `writable(new Set<string>())`
|
||||
|
||||
const currentVoiceRoom = derived(currentVoiceSession, s => (s ? {url: s.url, h: s.h} : undefined))
|
||||
|
||||
export const pubkeyFromLiveKitIdentity = (identity: string): string | undefined =>
|
||||
/^[a-f0-9]{64}$/.test(identity.slice(0, 64)) ? identity.slice(0, 64) : undefined
|
||||
|
||||
export const participantFromLiveKitIdentity = (identity: string): VoiceParticipant => {
|
||||
const pk = pubkeyFromLiveKitIdentity(identity)
|
||||
return pk ? {pubkey: pk, identity} : {identity}
|
||||
}
|
||||
|
||||
export const participantKey = (p: VoiceParticipant) => p.pubkey ?? p.identity
|
||||
|
||||
export const speakingParticipants = writable<VoiceParticipant[]>([])
|
||||
|
hodlbod marked this conversation as resolved
Outdated
hodlbod
commented
Use welshman's version instead (makeHttpAuth/makeHttpAuthHeader) Use welshman's version instead (makeHttpAuth/makeHttpAuthHeader)
|
||||
|
||||
export const isParticipantSpeaking = derived(
|
||||
speakingParticipants,
|
||||
$participants => (p: VoiceParticipant) =>
|
||||
$participants.some(sp => participantKey(sp) === participantKey(p)),
|
||||
)
|
||||
|
||||
const fetchLivekitToken = async (
|
||||
url: string,
|
||||
@@ -60,54 +98,39 @@ const fetchLivekitToken = async (
|
||||
}
|
||||
|
||||
export const deriveVoiceParticipants = (url: string, h: string) =>
|
||||
derived(deriveEventsForUrl(url, [{kinds: [ROOM_PRESENCE]}]), $events => {
|
||||
const cutoff = now() - PRESENCE_EXPIRY_S
|
||||
const pubkeys: string[] = []
|
||||
// We use the livekit identity list while in a call, and fall back to the list in kind 39004.
|
||||
derived(
|
||||
[
|
||||
participantPubkeyMap,
|
||||
currentVoiceRoom,
|
||||
deriveLatestEventForUrl(url, [{kinds: [LIVEKIT_PARTICIPANTS], "#d": [h]}]),
|
||||
],
|
||||
([$participantPubkeyMap, $currentVoiceRoom, $publishedParticipantList]) => {
|
||||
const inCall =
|
||||
$participantPubkeyMap.size > 0 &&
|
||||
$currentVoiceRoom?.url === url &&
|
||||
$currentVoiceRoom?.h === h
|
||||
|
||||
for (const event of $events) {
|
||||
if (event.created_at < cutoff) continue
|
||||
|
||||
if (getTagValue("h", event.tags) === h) {
|
||||
pubkeys.push(event.pubkey)
|
||||
if (inCall) {
|
||||
const participants = [...$participantPubkeyMap.keys()].map(participantFromLiveKitIdentity)
|
||||
return uniqBy((p: VoiceParticipant) => participantKey(p), participants)
|
||||
} else {
|
||||
const latestEvent = $publishedParticipantList as TrustedEvent | undefined
|
||||
if (!latestEvent) return []
|
||||
const participants = getTags("participant", latestEvent.tags).map((tag: string[]) => {
|
||||
const pubkey = tag[1]
|
||||
const identity = tag[2] ?? pubkey
|
||||
return pubkey ? {pubkey, identity} : {identity}
|
||||
})
|
||||
return uniqBy((p: VoiceParticipant) => participantKey(p), participants)
|
||||
}
|
||||
}
|
||||
|
||||
return pubkeys
|
||||
})
|
||||
|
||||
const publishPresence = (url: string, h: string) => {
|
||||
const event = makeEvent(ROOM_PRESENCE, {
|
||||
tags: [["h", h]],
|
||||
})
|
||||
|
||||
return publishThunk({event, relays: [url]})
|
||||
}
|
||||
|
||||
const deletePresence = (url: string) => {
|
||||
const event = makeEvent(ROOM_PRESENCE, {tags: []})
|
||||
|
||||
return publishThunk({event, relays: [url]})
|
||||
}
|
||||
|
||||
let presenceInterval: ReturnType<typeof setInterval> | undefined
|
||||
|
||||
const startPresenceHeartbeat = (url: string, h: string) => {
|
||||
stopPresenceHeartbeat()
|
||||
publishPresence(url, h)
|
||||
presenceInterval = setInterval(() => publishPresence(url, h), PRESENCE_INTERVAL_MS)
|
||||
}
|
||||
|
||||
const stopPresenceHeartbeat = () => {
|
||||
if (presenceInterval) {
|
||||
clearInterval(presenceInterval)
|
||||
presenceInterval = undefined
|
||||
}
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
const onRoomDisconnected = (reason?: DisconnectReason) => {
|
||||
speakingPubkeys.set(new Set())
|
||||
speakingParticipants.set([])
|
||||
participantPubkeyMap.set(new Map())
|
||||
currentVoiceSession.set(undefined)
|
||||
stopPresenceHeartbeat()
|
||||
if (reason !== undefined && reason !== DisconnectReason.CLIENT_INITIATED) {
|
||||
const message =
|
||||
reason === DisconnectReason.JOIN_FAILURE
|
||||
@@ -131,7 +154,7 @@ const onTrackUnsubscribed = (track: Track) => {
|
||||
}
|
||||
|
||||
const onActiveSpeakersChanged = (participants: {identity: string}[]) => {
|
||||
speakingPubkeys.set(new Set(participants.map(p => p.identity)))
|
||||
speakingParticipants.set(participants.map(p => participantFromLiveKitIdentity(p.identity)))
|
||||
}
|
||||
|
||||
const playJoinSound = () => {
|
||||
@@ -139,10 +162,15 @@ const playJoinSound = () => {
|
||||
audio.play().catch(() => {})
|
||||
}
|
||||
|
||||
const onParticipantConnected = () => {
|
||||
const onParticipantConnected = (participant: {identity: string}) => {
|
||||
addParticipant(participant.identity)
|
||||
playJoinSound()
|
||||
}
|
||||
|
||||
const onParticipantDisconnected = (participant: {identity: string}) => {
|
||||
deleteParticipant(participant.identity)
|
||||
}
|
||||
|
||||
export const joinVoiceRoom = async (
|
||||
url: string,
|
||||
h: string,
|
||||
@@ -160,6 +188,7 @@ export const joinVoiceRoom = async (
|
||||
|
||||
room.on(RoomEvent.Disconnected, onRoomDisconnected)
|
||||
room.on(RoomEvent.ParticipantConnected, onParticipantConnected)
|
||||
room.on(RoomEvent.ParticipantDisconnected, onParticipantDisconnected)
|
||||
room.on(RoomEvent.TrackSubscribed, onTrackSubscribed)
|
||||
room.on(RoomEvent.TrackUnsubscribed, onTrackUnsubscribed)
|
||||
room.on(RoomEvent.ActiveSpeakersChanged, onActiveSpeakersChanged)
|
||||
@@ -178,6 +207,12 @@ export const joinVoiceRoom = async (
|
||||
throw e
|
||||
}
|
||||
|
||||
participantPubkeyMap.set(new Map())
|
||||
addParticipant(room.localParticipant.identity)
|
||||
for (const p of room.remoteParticipants.values()) {
|
||||
addParticipant(p.identity)
|
||||
}
|
||||
|
||||
let muted = false
|
||||
try {
|
||||
await room.localParticipant.setMicrophoneEnabled(true)
|
||||
|
hodlbod marked this conversation as resolved
Outdated
hodlbod
commented
too much of this noise too much of this noise
|
||||
@@ -188,8 +223,6 @@ export const joinVoiceRoom = async (
|
||||
|
||||
|
hodlbod marked this conversation as resolved
Outdated
hodlbod
commented
aaaaa aaaaa
mplorentz
commented
haha, yes. Sorry I did do a self-review of this code but clearly I missed this function or my eyes glazed over while reading it. Cleaning it up. haha, yes. Sorry I did do a self-review of this code but clearly I missed this function or my eyes glazed over while reading it. Cleaning it up.
|
||||
currentVoiceSession.set({url, h, room, muted})
|
||||
|
||||
startPresenceHeartbeat(url, h)
|
||||
|
||||
playJoinSound()
|
||||
}
|
||||
|
||||
@@ -200,10 +233,9 @@ export const leaveVoiceRoom = async () => {
|
||||
const audio = new Audio("/leave-voice-room.mp3")
|
||||
audio.play().catch(() => {})
|
||||
|
||||
speakingPubkeys.set(new Set())
|
||||
stopPresenceHeartbeat()
|
||||
speakingParticipants.set([])
|
||||
participantPubkeyMap.set(new Map())
|
||||
session.room.disconnect()
|
||||
deletePresence(session.url)
|
||||
currentVoiceSession.set(undefined)
|
||||
}
|
||||
|
||||
|
||||
deriveProfileis designed to acceptundefinedto avoid this kind of mess