Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

use san rather than chessground callback args for sound #16461

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+23 −23
Open

use san rather than chessground callback args for sound #16461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fc77f1e
remove unnecessary "music only" call to sound.move
schlawg Nov 22, 2024
cd79197
support initialFen and remove filter from soundMove.ts
schlawg Nov 22, 2024
6006b58
use d.game.fen for ply 0 as d.game.initialFen is not always defined
schlawg Nov 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion ui/@types/lichess/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ type SoundMoveOpts = {
san?: string;
uci?: string;
volume?: number;
filter?: 'music' | 'game';
};

type SoundMove = (opts?: SoundMoveOpts) => void;
Expand Down
1 change: 0 additions & 1 deletion ui/bits/src/bits.soundMove.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ export async function initModule(): Promise<SoundMove> {
await Promise.all(promises);

return o => {
if (o?.filter === 'game') return;
const volume = o?.volume ?? 1;
if (o?.san && o.uci) {
const pitch = keyToPitch(o.uci.slice(2));
Expand Down
18 changes: 10 additions & 8 deletions ui/round/src/ctrl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,16 @@ export default class RoundController implements MoveRootCtrl {
};

private onMove = (orig: Key, dest: Key, captured?: Piece) => {
if (captured || this.enpassant(orig, dest)) {
if (this.data.game.variant.key === 'atomic') {
site.sound.play('explosion');
atomic.capture(this, dest);
} else site.sound.move({ name: 'capture', filter: 'game' });
} else site.sound.move({ name: 'move', filter: 'game' });
if ((captured || this.enpassant(orig, dest)) && this.data.game.variant.key === 'atomic') {
site.sound.play('explosion');
atomic.capture(this, dest);
return;
}
const fen = this.ply === 0 ? this.data.game.fen : this.stepAt(this.ply - 1).fen;
const san = sanOf(readFen(fen), orig + dest);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure sanOf(readFen(fen), move) is expensive to run at such a critical moment; that's why the server sends the SAN to the client.
Also I'm not sure that works with variants.

If we want to play a sound before the server sends the SAN back, it should be possible to do so with computing the SAN.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sanOf(readFen) takes around 2-3 microseconds.

pentatonic uses information such as castles and check which are not readily available from chessground. is it worth separate code paths to shave off a computation we could do a thousand times in a loop without dropping a frame?


site.sound.move({ san, uci: orig + dest });
site.sound.saySan(san);
};

private startPromotion = (orig: Key, dest: Key, meta: MoveMetadata) =>
Expand Down Expand Up @@ -490,8 +494,6 @@ export default class RoundController implements MoveRootCtrl {
this.autoScroll();
this.onChange();
this.pluginUpdate(step.fen);
if (!this.opts.local) site.sound.move({ ...o, filter: 'music' });
site.sound.saySan(step.san);
return true; // prevents default socket pubsub
};

Expand Down
26 changes: 13 additions & 13 deletions ui/site/src/sound.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,21 +72,21 @@ export default new (class implements SoundI {

async move(o?: SoundMoveOpts) {
const volume = o?.volume ?? 1;
if (o?.filter !== 'music' && this.theme !== 'music') {
if (o?.name) this.throttled(o.name, volume);
else {
if (o?.san?.includes('x')) this.throttled('capture', volume);
else this.throttled('move', volume);
if (o?.san?.includes('#')) {
this.throttled('checkmate', volume);
} else if (o?.san?.includes('+')) {
this.throttled('check', volume);
}
if (this.theme === 'music') {
this.music ??= await site.asset.loadEsm<SoundMove>('bits.soundMove');
this.music(o);
return;
}
if (o?.name) this.throttled(o.name, volume);
else {
if (o?.san?.includes('x')) this.throttled('capture', volume);
else this.throttled('move', volume);
if (o?.san?.includes('#')) {
this.throttled('checkmate', volume);
} else if (o?.san?.includes('+')) {
this.throttled('check', volume);
}
}
if (o?.filter === 'game' || this.theme !== 'music') return;
this.music ??= await site.asset.loadEsm<SoundMove>('bits.soundMove');
this.music(o);
}

async playAndDelayMateResultIfNecessary(name: Name): Promise<void> {
Expand Down