mod tracker #306
Johann150
Loading…
Reference in a new issue
No description provided.
Delete branch "puniko/FoundKey:feature/mod-tracker"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The scene is dead ;)
Adds a tracked music player (using chiptune2.js and libopenmpt) to the media list in notes
Since we're AGPL, just checking that the libopenmpt.js is the source for the wasm file? Otherwise we'd need the source for that wasm file in the repo, or as a git submodule, or maybe use it as an npm library if available.
@ -0,0 +1,315 @@<template><div class="mod-player-disabled" v-if="hide" @click="hide = false"><div><b><i class="fas fa-exlamation-triangle"></i> {{ $ts.sensitive }}</b>I think you should use
i18n.tsinstead of$tsbecause that is not set globally any more. Probably also need to addimport { i18n } from '@/i18n';done
https://lib.openmpt.org/libopenmpt/license/
iirc the license has to be included somewhere in code and in docs.
sadly no good and usable npm lib for this. chiptune2.js is modified by myself (especially to event handler stuff and the stuff reading pattern data)
WIP: mod trackerto mod tracker@Johann150 please review
this is how it looks like btw
This is still missing. Please at least put the license as a text file somewhere and reference it in the
COPYINGfile. Also I think as I tried to explain that thewasmfile would be considered "object code" under AGPL so we should be able to provide source code for it. Either add that in as a git submodule or its probably also fine if we just link to the original repo also inCOPYING.is this ok?
bffd15daedSeems ok to me.
Are there any other concerns re: quality? I'm ok to merge otherwise.
P.S. We might move all external licenses to THIRD-PARTY.md or something later.
Hiding and then showing the player again via the eye symbol leaves it blank. Maybe it should be reset to the start when displaying it again.
I find the styling of the volume slider confusing. At first I thought it works like the "progress bar" that is next to it but was confused why the colors are reversed. Since the two UI elements are next to each other and are both sliders I would expect them to look and operate similarly.
@ -127,1 +130,4 @@};const isModule = (file: foundkey.entities.DriveFile): boolean => {return FILE_EXT_TRACKER_MODULES.filter((ext) => {I think this should probably use
someinstead offilter(...).length > 0.@ -0,0 +11,4 @@<canvas class="pattern-canvas" ref="displayCanvas"></canvas></div><div class="controls"><button class="play" accesskey="P" @click="playPause()">I'm not sure about the
accesskeyattribute. We didn't use it anywhere else. According to MDN browser support is also flaky. I'm personally not familiar with them so wouldn't even know how to use them.Maybe you would want to use the
v-hotkeydirective instead? (defined inpackages/client/src/directives/hotkey.ts)just removed it. v-hotkey can be added later i think
Sounds good to me. I think hotkeys aren't necessarily required even.
@ -0,0 +13,4 @@<div class="controls"><button class="play" accesskey="P" @click="playPause()"><i class="fas fa-play" v-if="!playing"></i><i class="fas fa-pause" v-if="playing"></i>I think this could use
v-else.ye, i saw that too. can add a toggle function that also reloads the pattern display when it switches from hidden to visible.
not exactly. since chiptune2.js doesn't have a seek function implemented yet (and i don't have extended it yet due to time lack with one), the progress bar is just for display and doesn't let you seek through the track. i could try to implement seeking and replace the progress bar with a slider. might take a bit tho (yay reading up on libopenmpt docs again)
EDIT: oh wait lol, i actually have added a seek function to chiptune2.js. gonna make the progressbar seekable then. give me a bit
Oh, I didn't notice. 🥴
I mean if you want to that would be welcome too, but I had more in mind that the volume thingy should probably look more like a progress bar.
Or at least like it is right now it is confusing since it is not immediately clear that the green block there is a different kind of green block than in the progress bar. I'm not really sure what a better design of the volume slider would be. But "green rectangle" is definitely confusing.
did kinda a blend of the two, see attached image (styling range input is pain tbh, -ms -webkit -moz everywhere). idk how to make this look the same on webkit tho.
also made it seekable ;)
can't manage to get pattern display to reappear on unhide (and on seek when stopped, not paused). chiptune2.js doesn't make it trivial to display patterns on a stopped (not paused) track, since it only ever loads the module into libopenmpt on play.
so i think its fine to let it be like this for now and maybe improve on this point on another PR maybe
webkit is a pain 🥴
made the range slider look the same on webkit based browsers too. so from my side, its ready to go @Johann150
The redesign looks good to me.
@ -0,0 +142,4 @@}function toggleVisible() {hide.value = !hide.value;The issue with hiding and unhiding still persists. I also noticed that if you hide while its playing it will stop but if you show it again it will continue to play. I tried also calling
stop()here but that caused issues.6998cae7e3this works, its just... terrible code. and i can't come up with another solution as of now without having to rewrite big chunks of chiptune2 or somethingalrightly, i think this is ready. also managed to make the patterns display when seeking while track is stopped