From 1317dba3e42a6f700e38f930d296f6f4128e97bf Mon Sep 17 00:00:00 2001 From: James R Date: Sat, 18 Jan 2020 22:13:34 -0800 Subject: [PATCH 1/3] Refactor MUSICDEF parsing, actually count lines If you use strtok for (CR)LF, it'll skip the empty lines bruh. --- src/s_sound.c | 305 ++++++++++++++++++++++++-------------------------- 1 file changed, 147 insertions(+), 158 deletions(-) diff --git a/src/s_sound.c b/src/s_sound.c index c4c92ebf5..848fd671a 100644 --- a/src/s_sound.c +++ b/src/s_sound.c @@ -1472,188 +1472,112 @@ static UINT16 W_CheckForMusicDefInPwad(UINT16 wadid) return INT16_MAX; // not found } -void S_LoadMusicDefs(UINT16 wadnum) +static boolean +ReadMusicDefFields (UINT16 wadnum, int line, char *stoken, musicdef_t **defp) { - UINT16 lumpnum; - char *lump, *buf; - char *musdeftext; - char *stoken; + musicdef_t *def; + char *value; char *textline; - size_t size; - INT32 i; - musicdef_t *def = NULL; - UINT16 line = 1; // for better error msgs + int i; - lumpnum = W_CheckForMusicDefInPwad(wadnum); - if (lumpnum == INT16_MAX) - return; - - lump = W_CacheLumpNumPwad(wadnum, lumpnum, PU_CACHE); - size = W_LumpLengthPwad(wadnum, lumpnum); - - // Null-terminated MUSICDEF lump. - musdeftext = malloc(size+1); - if (!musdeftext) - I_Error("S_LoadMusicDefs: No more free memory for the parser\n"); - M_Memcpy(musdeftext, lump, size); - musdeftext[size] = '\0'; - - // for strtok - buf = malloc(size+1); - if (!buf) - I_Error("S_LoadMusicDefs: No more free memory for the parser\n"); - M_Memcpy(buf, musdeftext, size+1); - - stoken = strtok(buf, "\r\n "); - // Find music def - while (stoken) + if (!stricmp(stoken, "lump")) { - /*if ((stoken[0] == '/' && stoken[1] == '/') - || (stoken[0] == '#')) // skip comments + value = strtok(NULL, " "); + if (!value) { - stoken = strtok(NULL, "\r\n"); // skip end of line - if (def) - stoken = strtok(NULL, "\r\n= "); - else - stoken = strtok(NULL, "\r\n "); - line++; - } - else*/ if (!stricmp(stoken, "lump")) - { - value = strtok(NULL, "\r\n "); - - if (!value) - { - CONS_Alert(CONS_WARNING, "MUSICDEF: Lump '%s' is missing name. (file %s, line %d)\n", stoken, wadfiles[wadnum]->filename, line); - stoken = strtok(NULL, "\r\n"); // skip end of line - goto skip_lump; - } - - // No existing musicdefs - /*if (!musicdefstart) - { - musicdefstart = Z_Calloc(sizeof (musicdef_t), PU_STATIC, NULL); - STRBUFCPY(musicdefstart->name, value); - strlwr(musicdefstart->name); - def = musicdefstart; - //CONS_Printf("S_LoadMusicDefs: Initialized musicdef w/ song '%s'\n", def->name); - } - else*/ - { - musicdef_t *prev = NULL; - def = musicdefstart; - - // Search if this is a replacement - //CONS_Printf("S_LoadMusicDefs: Searching for song replacement...\n"); - while (def) - { - if (!stricmp(def->name, value)) - { - //CONS_Printf("S_LoadMusicDefs: Found song replacement '%s'\n", def->name); - break; - } - - prev = def; - def = def->next; - } - - // Nothing found, add to the end. - if (!def) - { - def = Z_Calloc(sizeof (musicdef_t), PU_STATIC, NULL); - STRBUFCPY(def->name, value); - strlwr(def->name); - def->bpm = TICRATE<<(FRACBITS-1); // FixedDiv((60*TICRATE)<next = def; - //CONS_Printf("S_LoadMusicDefs: Added song '%s'\n", def->name); - } - } - -skip_lump: - stoken = strtok(NULL, "\r\n "); - line++; + CONS_Alert(CONS_WARNING, + "MUSICDEF: Field '%s' is missing name. (file %s, line %d)\n", + stoken, wadfiles[wadnum]->filename, line); + return false; } else { - // If this is set true, the line was invalid. - boolean brokenline = false; + musicdef_t *prev = NULL; + def = musicdefstart; - // Delimit only by line break. - value = strtok(NULL, "\r\n"); + // Search if this is a replacement + //CONS_Printf("S_LoadMusicDefs: Searching for song replacement...\n"); + while (def) + { + if (!stricmp(def->name, value)) + { + //CONS_Printf("S_LoadMusicDefs: Found song replacement '%s'\n", def->name); + break; + } + prev = def; + def = def->next; + } + + // Nothing found, add to the end. + if (!def) + { + def = Z_Calloc(sizeof (musicdef_t), PU_STATIC, NULL); + STRBUFCPY(def->name, value); + strlwr(def->name); + def->bpm = TICRATE<<(FRACBITS-1); // FixedDiv((60*TICRATE)<next = def; + //CONS_Printf("S_LoadMusicDefs: Added song '%s'\n", def->name); + } + + (*defp) = def; + } + } + else + { + value = strtok(NULL, ""); + + if (value) + { // Find the equals sign. value = strchr(value, '='); + } - // It's not there?! - if (!value) - brokenline = true; - else - { - // Skip the equals sign. - value++; - - // Now skip funny whitespace. - if (value[0] == '\0') // :NOTHING: - brokenline = true; - else - value += strspn(value, "\t "); - } - - // If the line is valid, copy the text line from the lump data. - if (!brokenline) - { - // strtok returns memory that already belongs to the input string. - value = musdeftext + (value - buf); - - // Find the length of the line. - size = strcspn(value, "\r\n"); - - // Copy the line. - textline = malloc(size+1); - if (!textline) - I_Error("S_LoadMusicDefs: No more free memory for text line\n"); - M_Memcpy(textline, value, size); - textline[size] = '\0'; - } - else - { - CONS_Alert(CONS_WARNING, "MUSICDEF: Field '%s' is missing value. (file %s, line %d)\n", stoken, wadfiles[wadnum]->filename, line); - stoken = strtok(NULL, "\r\n"); // skip end of line - goto skip_field; - } + if (!value) + { + CONS_Alert(CONS_WARNING, + "MUSICDEF: Field '%s' is missing value. (file %s, line %d)\n", + stoken, wadfiles[wadnum]->filename, line); + return false; + } + else + { + def = (*defp); if (!def) { - CONS_Alert(CONS_ERROR, "MUSICDEF: No music definition before field '%s'. (file %s, line %d)\n", stoken, wadfiles[wadnum]->filename, line); - free(textline); - free(buf); - free(musdeftext); - return; + CONS_Alert(CONS_ERROR, + "MUSICDEF: No music definition before field '%s'. (file %s, line %d)\n", + stoken, wadfiles[wadnum]->filename, line); + return false; } - i = atoi(textline); + // Skip the equals sign. + value++; + // Now skip funny whitespace. + value += strspn(value, "\t "); + + textline = value; + i = atoi(value); + + /* based ignored lumps */ if (!stricmp(stoken, "usage")) { #if 0 // Ignore for now STRBUFCPY(def->usage, textline); - //CONS_Printf("S_LoadMusicDefs: Set usage to '%s'\n", def->usage); #endif } else if (!stricmp(stoken, "source")) { #if 0 // Ignore for now STRBUFCPY(def->source, textline); - //CONS_Printf("S_LoadMusicDefs: Set source to '%s'\n", def->usage); #endif } else if (!stricmp(stoken, "title")) { STRBUFCPY(def->title, textline); - //CONS_Printf("S_LoadMusicDefs: Set title to '%s'\n", def->source); } else if (!stricmp(stoken, "alttitle")) { STRBUFCPY(def->alttitle, textline); - //CONS_Printf("S_LoadMusicDefs: Set alttitle to '%s'\n", def->source); } else if (!stricmp(stoken, "authors")) { STRBUFCPY(def->authors, textline); - //CONS_Printf("S_LoadMusicDefs: Set authors to '%s'\n", def->source); } else if (!stricmp(stoken, "soundtestpage")) { def->soundtestpage = (UINT8)i; } else if (!stricmp(stoken, "soundtestcond")) { @@ -1670,21 +1594,86 @@ skip_lump: if (bpmf > 0) def->bpm = FixedDiv((60*TICRATE)<filename, line); + CONS_Alert(CONS_WARNING, + "MUSICDEF: Invalid field '%s'. (file %s, line %d)\n", + stoken, wadfiles[wadnum]->filename, line); } - - // Free the temporary text line from memory. - free(textline); - -skip_field: - stoken = strtok(NULL, "\r\n= "); - line++; } } - free(buf); + return true; +} + +void S_LoadMusicDefs(UINT16 wadnum) +{ + UINT16 lumpnum; + char *lump; + char *musdeftext; + size_t size; + + char *lf; + char *stoken; + + size_t nlf; + size_t ncr; + + musicdef_t *def = NULL; + int line = 1; // for better error msgs + + lumpnum = W_CheckForMusicDefInPwad(wadnum); + if (lumpnum == INT16_MAX) + return; + + lump = W_CacheLumpNumPwad(wadnum, lumpnum, PU_CACHE); + size = W_LumpLengthPwad(wadnum, lumpnum); + + // Null-terminated MUSICDEF lump. + musdeftext = malloc(size+1); + if (!musdeftext) + I_Error("S_LoadMusicDefs: No more free memory for the parser\n"); + M_Memcpy(musdeftext, lump, size); + musdeftext[size] = '\0'; + + // Find music def + stoken = musdeftext; + for (;;) + { + lf = strpbrk(stoken, "\r\n"); + if (lf) + { + if (*lf == '\n') + nlf = 1; + else + nlf = 0; + *lf++ = '\0';/* now we can delimit to here */ + } + + stoken = strtok(stoken, " "); + if (stoken) + { + if (! ReadMusicDefFields(wadnum, line, stoken, &def)) + break; + } + + if (lf) + { + do + { + line += nlf; + ncr = strspn(lf, "\r");/* skip CR */ + lf += ncr; + nlf = strspn(lf, "\n"); + lf += nlf; + } + while (nlf || ncr) ; + + stoken = lf;/* now the next nonempty line */ + } + else + break;/* EOF */ + } + free(musdeftext); - return; } // From 9dbc54284ecdc46080f0e00a5f35040ba94922fc Mon Sep 17 00:00:00 2001 From: James R Date: Sat, 18 Jan 2020 23:12:30 -0800 Subject: [PATCH 2/3] Opt into new MUSICDEF format (2.2.0 compatibility) The "VERSION" directive enables features available in a certain version of SRB2. It may be used as "VERSION 2.2.0". --- src/s_sound.c | 92 +++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 79 insertions(+), 13 deletions(-) diff --git a/src/s_sound.c b/src/s_sound.c index 848fd671a..d11fc4321 100644 --- a/src/s_sound.c +++ b/src/s_sound.c @@ -1441,6 +1441,12 @@ static tic_t pause_starttic; /// Music Definitions /// ------------------------ +enum +{ + MUSICDEF_220, + MUSICDEF_221, +}; + musicdef_t soundtestsfx = { "_STSFX", // prevents exactly one valid track name from being used on the sound test "Sound Effects", @@ -1472,10 +1478,27 @@ static UINT16 W_CheckForMusicDefInPwad(UINT16 wadid) return INT16_MAX; // not found } +static void +MusicDefStrcpy (char *p, const char *s, size_t n, int version) +{ + strlcpy(p, s, n); + if (version == MUSICDEF_220) + { + while (( p = strchr(p, '_') )) + { + n = strspn(p, "_"); + memset(p, ' ', n); // turn _ into spaces. + p += n; + } + } +} + static boolean -ReadMusicDefFields (UINT16 wadnum, int line, char *stoken, musicdef_t **defp) +ReadMusicDefFields (UINT16 wadnum, int line, boolean fields, char *stoken, + musicdef_t **defp, int *versionp) { musicdef_t *def; + int version; char *value; char *textline; @@ -1525,14 +1548,47 @@ ReadMusicDefFields (UINT16 wadnum, int line, char *stoken, musicdef_t **defp) (*defp) = def; } } + else if (!stricmp(stoken, "version")) + { + if (fields)/* is this not the first field? */ + { + CONS_Alert(CONS_WARNING, + "MUSICDEF: Field '%s' must come first. (file %s, line %d)\n", + stoken, wadfiles[wadnum]->filename, line); + return false; + } + else + { + value = strtok(NULL, " "); + if (!value) + { + CONS_Alert(CONS_WARNING, + "MUSICDEF: Field '%s' is missing version. (file %s, line %d)\n", + stoken, wadfiles[wadnum]->filename, line); + return false; + } + else + { + if (strcasecmp(value, "2.2.0")) + (*versionp) = MUSICDEF_221; + } + } + } else { - value = strtok(NULL, ""); + version = (*versionp); - if (value) + if (version == MUSICDEF_220) + value = strtok(NULL, " ="); + else { - // Find the equals sign. - value = strchr(value, '='); + value = strtok(NULL, ""); + + if (value) + { + // Find the equals sign. + value = strchr(value, '='); + } } if (!value) @@ -1554,11 +1610,14 @@ ReadMusicDefFields (UINT16 wadnum, int line, char *stoken, musicdef_t **defp) return false; } - // Skip the equals sign. - value++; + if (version != MUSICDEF_220) + { + // Skip the equals sign. + value++; - // Now skip funny whitespace. - value += strspn(value, "\t "); + // Now skip funny whitespace. + value += strspn(value, "\t "); + } textline = value; i = atoi(value); @@ -1573,11 +1632,14 @@ ReadMusicDefFields (UINT16 wadnum, int line, char *stoken, musicdef_t **defp) STRBUFCPY(def->source, textline); #endif } else if (!stricmp(stoken, "title")) { - STRBUFCPY(def->title, textline); + MusicDefStrcpy(def->title, textline, + sizeof def->title, version); } else if (!stricmp(stoken, "alttitle")) { - STRBUFCPY(def->alttitle, textline); + MusicDefStrcpy(def->alttitle, textline, + sizeof def->alttitle, version); } else if (!stricmp(stoken, "authors")) { - STRBUFCPY(def->authors, textline); + MusicDefStrcpy(def->authors, textline, + sizeof def->authors, version); } else if (!stricmp(stoken, "soundtestpage")) { def->soundtestpage = (UINT8)i; } else if (!stricmp(stoken, "soundtestcond")) { @@ -1618,7 +1680,9 @@ void S_LoadMusicDefs(UINT16 wadnum) size_t ncr; musicdef_t *def = NULL; + int version = MUSICDEF_220; int line = 1; // for better error msgs + boolean fields = false; lumpnum = W_CheckForMusicDefInPwad(wadnum); if (lumpnum == INT16_MAX) @@ -1651,8 +1715,10 @@ void S_LoadMusicDefs(UINT16 wadnum) stoken = strtok(stoken, " "); if (stoken) { - if (! ReadMusicDefFields(wadnum, line, stoken, &def)) + if (! ReadMusicDefFields(wadnum, line, fields, stoken, + &def, &version)) break; + fields = true; } if (lf) From 2d8ea7125c24e1e3c6e8b56bdaabfeaa54354e25 Mon Sep 17 00:00:00 2001 From: James R Date: Mon, 20 Jan 2020 15:36:27 -0800 Subject: [PATCH 3/3] Remove unnecessary optimization --- src/s_sound.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/s_sound.c b/src/s_sound.c index d11fc4321..d84e20ab4 100644 --- a/src/s_sound.c +++ b/src/s_sound.c @@ -1485,11 +1485,7 @@ MusicDefStrcpy (char *p, const char *s, size_t n, int version) if (version == MUSICDEF_220) { while (( p = strchr(p, '_') )) - { - n = strspn(p, "_"); - memset(p, ' ', n); // turn _ into spaces. - p += n; - } + *p++ = ' '; // turn _ into spaces. } }