-
Notifications
You must be signed in to change notification settings - Fork 557
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.
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
Load an unlimited amount of loose music #1171
base: master
Are you sure you want to change the base?
Conversation
7a2e8a5
to
740f3f9
Compare
|
740f3f9
to
629ff23
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've only looked at the code thus far and it doesn't seem to do very strange things. I also still wanna test it, and see how it behaves in edge cases (like when songs have numbers as filenames, or how accessing vvvvvvmusic.vvv
songs by name works or overwriting the same names, and stuff like that). Unless someone wants to beat me to that 😛
{ | ||
SDL_zerop(this); | ||
read_buf = (Uint8*) SDL_malloc(rw->size(rw)); | ||
SDL_RWread(rw, read_buf, rw->size(rw), 1); | ||
int err; | ||
stb_vorbis_info vorbis_info; | ||
stb_vorbis_comment vorbis_comment; | ||
filename = SDL_strdup(name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name
, filename
and id
are used a bit confusingly. In the enumeration code, the file path (with folder and extension) is first written to a buffer name
, then a plain version without folder or extension is written to a buffer id
, this is passed to the MusicTrack
constructor as name
and then written to the filename
attribute, which can be requested with a function called getid
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you suggest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late reply!
I'd say:
char name[256]
(stores e.g.music/mysong.ogg
) ->asset_filename
? It's used as an argument toFILESYSTEM_loadAssetToMemory()
char id[256]
(stores e.g.mysong
) -> probably fine as-ischar* filename
inMusicTrack
->id
to matchconst char* name
in theMusicTrack
constructor ->id
to match
The names of the constructor argument and class attribute will clash - I'm not too used to C++ OO but I've usually seen the solution in other languages be something like this.id = id;
. Otherwise, naming the argument _id
and doing id = SDL_strdup(_id);
would in my opinion be better than working around it with filename
/name
, loose_extra
/looseextra
, etc. (And that kind of _
prefixing is done in other places in the code as well anyway)
Binary blobs can now store 128 tracks, and music can now be read from loose files. Unfortunately, the two cannot be used together -- loose music can still only use 16 tracks. This commit attempts to fix that, allowing for music playback by filename.
629ff23
to
31ef70c
Compare
31ef70c
to
e0e769b
Compare
Changes:
Binary blobs can now store 128 tracks, and music can now be read from loose files. Unfortunately, the two cannot be used together -- loose music can still only use 16 tracks. This commit attempts to fix that, allowing for music playback by filename.
Closes #965.
Legal Stuff:
By submitting this pull request, I confirm that...
CONTRIBUTORS
file and the "GitHub Friends"section of the credits for all of said releases, but will NOT be compensated
for these changes unless there is a prior written agreement