MantisBT - Zandronum |
| View Issue Details |
|
| ID | Project | Category | View Status | Date Submitted | Last Update |
| 0004204 | Zandronum | [All Projects] Bug | public | 2024-03-08 04:51 | 2024-04-08 20:27 |
|
| Reporter | Trillster | |
| Assigned To | | |
| Priority | normal | Severity | crash | Reproducibility | always |
| Status | resolved | Resolution | fixed | |
| Platform | | OS | | OS Version | |
| Product Version | 3.1 | |
| Target Version | 3.2 | Fixed in Version | 3.2 | |
|
| Summary | 0004204: Non-standard player classes can cause "eaten" skins and crashes |
| Description | When loading files with multiple non-standard (non-DoomPlayer inheriting) player classes alongside skin files which have skins for player classes which are not in the load order, it is possible for valid skins to be improperly removed from the list. This extends to the "Base" skin designated for each player class, which causes a known crash when opening the Player Setup menu with that class selected. Some players have also reported crashes as soon as the game starts up in this scenario.
I've attached a few example files to illustrate this problem as well as a potential patch to fix it. The patch adds an extra validation step before removing any skin. It would probably be better to tackle the logic leading up to the improper removal instead, but I wasn't too familiar with that area of the code to go that route. |
| Steps To Reproduce | First, load freedoom1.wad and NewPlayerClasses.pk3. The latter file implements a FreeDoomMarine1 (FreeDoomPlayer1) and FreeDoomMarine2 (FreeDoomPlayer2) player class, neither of which inherit from DoomPlayer. Neither of these classes should have any skins at this point.
Next, load freedoom1.wad, NewPlayerClasses.pk3, FreeDoomMarine2Skins.pk3, and FreeDoomMarine3Skins.pk3. FreeDoomMarine2Skins.pk3 attempts to define two skins for FreeDoomMarine2 while FreeDoomMarine3Skins.pk3 does the same for the non-existent FreeDoomMarine3. In-game, you'll notice that FreeDoomMarine2 only has one extra skin defined, rather than the two, so one was eaten.
Finally, load freedoom1.wad, NewPlayerClasses.pk3, and FreeDoomMarine3Skins.pk3. Attempting to open the Player Setup menu with FreeDoomMarine2 selected will now crash the game, because the FreeDoomMarine3Skins.pk3 file has eaten the "Base" skin for FreeDoomMarine2. |
| Additional Information | This issue has historically impacted class mods in MM8BDM which want to remove the default Megaman class (which doesn't inherit from DoomPlayer). Because all skin packs there typically use a "class" of Megaman, people with skins in their skins folder would always experience these crashes. |
| Tags | No tags attached. |
| Relationships | |
| Attached Files |  NewPlayerClasses.pk3 (999) 2024-03-08 04:51
https://zandronum.com/tracker/file_download.php?file_id=2917&type=bug
FreeDoomMarine2Skins.pk3 (139,633) 2024-03-08 04:51
https://zandronum.com/tracker/file_download.php?file_id=2918&type=bug
FreeDoomMarine3Skins.pk3 (139,635) 2024-03-08 04:51
https://zandronum.com/tracker/file_download.php?file_id=2919&type=bug
 skin_removal.patch (2,300) 2024-03-08 04:52
https://zandronum.com/tracker/file_download.php?file_id=2920&type=bug
FreeDoomMarine2+3Skins.pk3 (139,645) 2024-03-08 17:22
https://zandronum.com/tracker/file_download.php?file_id=2921&type=bug
FreeDoomMarine3+2Skins.pk3 (139,645) 2024-03-08 17:23
https://zandronum.com/tracker/file_download.php?file_id=2922&type=bug
skin_removal_BOF.patch (594) 2024-03-24 01:05
https://zandronum.com/tracker/file_download.php?file_id=2946&type=bug |
|
| Issue History |
| Date Modified | Username | Field | Change |
| 2024-03-08 04:51 | Trillster | New Issue | |
| 2024-03-08 04:51 | Trillster | File Added: NewPlayerClasses.pk3 | |
| 2024-03-08 04:51 | Trillster | File Added: FreeDoomMarine2Skins.pk3 | |
| 2024-03-08 04:51 | Trillster | File Added: FreeDoomMarine3Skins.pk3 | |
| 2024-03-08 04:52 | Trillster | File Added: skin_removal.patch | |
| 2024-03-08 17:08 | Kaminsky | Status | new => needs review |
| 2024-03-08 17:22 | Trillster | Note Added: 0023316 | |
| 2024-03-08 17:22 | Trillster | File Added: FreeDoomMarine2+3Skins.pk3 | |
| 2024-03-08 17:23 | Trillster | File Added: FreeDoomMarine3+2Skins.pk3 | |
| 2024-03-24 01:04 | BarrelsOFun | Note Added: 0023478 | |
| 2024-03-24 01:05 | BarrelsOFun | File Added: skin_removal_BOF.patch | |
| 2024-03-24 01:05 | BarrelsOFun | Note Edited: 0023478 | bug_revision_view_page.php?bugnote_id=23478#r14153 |
| 2024-03-30 20:53 | Trillster | Note Added: 0023512 | |
| 2024-04-07 20:39 | Ru5tK1ng | Note Added: 0023531 | |
| 2024-04-07 20:39 | Ru5tK1ng | Status | needs review => needs testing |
| 2024-04-07 20:39 | Ru5tK1ng | Target Version | => 3.2 |
| 2024-04-08 20:27 | Ru5tK1ng | Note Added: 0023558 | |
| 2024-04-08 20:27 | Ru5tK1ng | Status | needs testing => resolved |
| 2024-04-08 20:27 | Ru5tK1ng | Resolution | open => fixed |
| 2024-04-08 20:27 | Ru5tK1ng | Fixed in Version | => 3.2 |
|
Notes |
|
|
|
Disregard the previous patch, after some further testing, I've realized that it doesn't solve this issue in all scenarios. I've attached two more testing files to showcase this.
FreeDoomMarine2+3Skins.pk3 combines the two prior testing files, putting 2's skins first then 3's. FreeDoomMarine2's second skin will still be eaten with the patch in play.
FreeDoomMarine3+2Skins.pk3 does the same, except it puts 3's skins first then 2's. The issue doesn't occur with this file, patch or no patch.
This makes me believe the crux of the issue is that whenever all loaded player classes don't inherit from a standard player class (ie. DoomPlayer) and the last skin of a SKININFO file is set to be removed, after that skin is removed, the parsing to finish out the file will end up removing one extra skin than intended. |
|
|
|
(0023478)
|
|
BarrelsOFun
|
2024-03-24 01:04
(edited on: 2024-03-24 01:05) |
|
The issue was caused by the basetype of the last skin being reinitialized from the removal continuing the loop, which leaves it at an uninitialized value.
the initializer after the fact sets it to the default basetype of whichever game it is, and when that isn't there in the next check, then the skin gets removed.
This patch adds a check at if (remove) to finish parsing the individual skin by checking for '}' and will continue if hit, otherwise breaking when out of strings. When S_SKIN it should just break, since S_SKIN generally only holds one skin unless it's converted into a SKININFO
|
|
|
|
|
|
Can confirm that the most recently uploaded patch corrects the issues. |
|
|
|
|
|
|
|
|
|
Tested the skin files with the latest 3.2 beta there was no crash and the skin count for the 2nd class was 2. |
|