grimmdude / MidiPlayerJS

♬ MIDI parser & player engine for browser or Node. As a parser converts MIDI events into JSON. Works well with single or multitrack MIDI files.
https://grimmdude.com/MidiPlayerJS/
MIT License
359 stars 52 forks source link

getCurrentTick() defaults to 0 before Player is played for the first time #53

Closed hardlyfivefeet closed 3 years ago

hardlyfivefeet commented 4 years ago

Hi, my partner and I are using this library to implement a web player, and we realized that getCurrentTick() defaults to 0 before Player is played for the first time. Is there a reason for this or can we just remove the else if clause in the method, as follows?

getCurrentTick() {
    if(!this.startTime) return this.startTick;
    return Math.round(((new Date()).getTime() - this.startTime) / 1000 * (this.division * (this.tempo / 60))) + this.startTick;
}
grimmdude commented 4 years ago

Hi @hardlyfivefeet,

Late reply...I know. The current tick defaults to 0 before playing for the first time so that the file starts from the beginning.

-Garrett

evanmmitchell commented 4 years ago

The problem with using else if(!this.startTime) return 0; is that it makes it impossible to skipToTick() before playing for the first time. For starting from the beginning, it seems to make more sense to just rely on the fact that this.startTick defaults to zero. Is there a situation where both this.startTime and this.tick are zero (so it enters the else if clause) and this.startTick doesn't default to zero?

grimmdude commented 4 years ago

I can't remember exactly why I have the logic that way. But using Player.skipToTick(5555).play() before first play seems to work as expected for me. Am I missing something?

-Garrett

evanmmitchell commented 4 years ago

Oh, I'm sorry, skipToTick() itself wasn't the issue. The issue was that after calling skipToTick() before playing for the first time, getCurrentTick() still returns 0, while it should actually return the new startTick.