evaluate numiwadlumps after DoMerge() has been called
#1630
Conversation
If a merged PWAD contains sprites, these get mangled into the IWAD sprites, changing the value of `numlumps` and thus invalidating the value of `numiwadlumps` determined earlier. Also, this variable is only used in one occasion, so only evaluate it as needed. This approach makes sure `numiwadlumps` points past the index of the last IWAD lump in the linear `lumpinfo[]` array. Fixes fabiangreffrath#1101
|
|
||
| while (!W_IsIWADLump(lumpinfo[numiwadlumps - 1])) | ||
| { | ||
| numiwadlumps--; |
There was a problem hiding this comment.
What prevents this going negative?
There was a problem hiding this comment.
that we have an IWAD with at least one lump loaded at this point
|
Fix capitalization in commit message. |
Are you joking?! |
|
Let's try to keep the history looking nice. |
I'm all in favour of keeping a good revision history but it's possible to get excessively nitpicky with reviews and I think this is one example. Let's keep in mind that review comments do place a burden onto the author of the PR (no matter how small) and in cases like these it's probably best to just let it go. The Google Testing blog has an article about effective reviews which recommends flagging comments like these with "Nit:" to make clear that they're optional suggestions. |
|
Do you want to write something about this change for NEWS.md in #1623? |
|
I don't think so, this was merely a bug fix. |
If a merged PWAD contains sprites, these get mangled into the IWAD sprites, changing the value of
numlumpsand thus invalidating the value ofnumiwadlumpsdetermined earlier. Also, this variable is only used in one occasion, so only evaluate it as needed.This approach makes sure
numiwadlumpspoints past the index of the last IWAD lump in the linearlumpinfo[]array.Fixes fabiangreffrath#1101