search

serversideup / amplitudejs

AmplitudeJS: Open Source HTML5 Web Audio Library. Design your web audio player, the way you want. No dependencies required.
https://521dimensions.com/open-source/amplitudejs
MIT License
4.18k stars 427 forks source link

Can't initialize with `starting_playlist_song: 0` in config #407

Open janlucaklees opened 4 years ago

janlucaklees commented 4 years ago

Issue description

When I initializing amplitude with a list of songs, a playlist, a starting_playlist index and a starting_playlist_song of 0 (Integer literal), it is treated as not having set an starting_playlist_song index at all. Thus there is no initial artwork, song name, artist name, etc. displayed in your players interface. Clicking on play will have no effect as amplitude will not know what to play.

Environment

The website I am developing. If necessary, I can create a fiddle to reproduce the issue. But you can already see the error in the code. See below.

Steps to reproduce the issue

Initialize Amplitude with songs, a playlist, a starting_playlist index and in the config passed to the init function starting_playlist_song set to an integer value of 0.

What is expected?

Amplitude is initialized with the first song of the playlist. And your player is ready to go.

Link to where issue can be reproduced

Will create one on request. I think the code makes this clear.

Additional details / screenshots

This happens due to the use of abstract comparision rather than strict comparision in init.js:528. There we check userConfig.starting_playlist_song != "" but (0 != "") === false so an integer value of 0 is treated as not having set any value. We should use strict comparision !== instead. Or what I would prefer: directly check for an integer or string containing an integer.

I would like to create a PR to fix this issue. Would go for checking if an integer or a string that contains an integer is supplied and remove the abstract comparision with that.

llalundo commented 4 years ago

Before creating an issue on that I checked for existing ones, so here I am. I can confirm that I have that same behaviour when setting song 0 of a playlist as first song with

starting_playlist: "cmnsfavs", starting_playlist_song: 0

Any other index from the playlist (other than 0) works.

I'll add the output from the debug console after Amplitude.init():

Failed to load resource: the server responded with a status of 404 (Not Found) [http://localhost:5500/undefined]

If my next action in the player is to press the 'next' button, exactly this song that threw the error is selected, but does not play automatically (which I found to be the default behaviour in general).

janlucaklees commented 4 years ago

Because there was no instruction on how to proceed, I just opened a PR with the way I would change things around. You can find it here: #429

jaydrogers commented 4 years ago

Thanks for the PR on this. I added this into 5.1.1 for @danpastori to review once he gets a chance.

FuviTeam commented 2 years ago

Got the same problem.

Fixed it by replacing starting_playlist:0 by starting_playlist: '0'