MantisBT - Zandronum |
| View Issue Details |
|
| ID | Project | Category | View Status | Date Submitted | Last Update |
| 0002096 | Zandronum | [All Projects] Suggestion | public | 2015-02-08 20:38 | 2018-09-30 23:40 |
|
| Reporter | RusselCS | |
| Assigned To | Dusk | |
| Priority | high | Severity | minor | Reproducibility | N/A |
| Status | closed | Resolution | fixed | |
| Platform | | OS | | OS Version | |
| Product Version | 2.0-beta | |
| Target Version | 1.4 | Fixed in Version | 1.4 | |
|
| Summary | 0002096: Add either an actor flag or actor property to allow custom player classes to control how big their skins should be |
| Description | In mods that are built to have skins for their player classes, there are often some skins included with the mod itself. The skin parser updates introduced due to ticket 2090 cause certain skins to be improperly scaled, including skins intentionally included with a pwad. This change breaks compatibility with a large number of mods by scaling things down to unintended sizes. Couldn't there be some workaround introduced?
My suggestion is to either add an actor flag to allow custom player classes these skins are designed for to bypass the skin size check entirely or add an actor property to change the maximum scale by which the skin parser checks for, the value defaulting to the current scale value. |
| Steps To Reproduce | |
| Additional Information | In ticket 2090, there's a set of files that contain skins that are improperly scaled with the new Zandronum build. In the notes, Torr has also included a version with the updated skin parser.
The file included shows the compatibility issue in question, with SkinScaleTest_nobug.pk3 showing the same issue. |
| Tags | No tags attached. |
| Relationships | |
| Attached Files |  ClassScaleTest.zip (153,031) 2015-02-08 20:38
/tracker/file_download.php?file_id=1411&type=bug |
|
| Issue History |
| Date Modified | Username | Field | Change |
| 2015-02-08 20:38 | RusselCS | New Issue | |
| 2015-02-08 20:38 | RusselCS | File Added: ClassScaleTest.zip | |
| 2015-02-08 20:42 | Torr Samaho | Note Added: 0011638 | |
| 2015-02-08 20:45 | Torr Samaho | Note Edited: 0011638 | bug_revision_view_page.php?bugnote_id=11638#r6603 |
| 2015-02-08 20:45 | Torr Samaho | Note Revision Dropped: 11638: 0006602 | |
| 2015-02-08 21:08 | RusselCS | Note Added: 0011641 | |
| 2015-02-08 21:09 | RusselCS | Note Edited: 0011641 | bug_revision_view_page.php?bugnote_id=11641#r6605 |
| 2015-02-08 21:29 | Torr Samaho | Note Added: 0011642 | |
| 2015-02-08 21:57 | Dusk | Note Added: 0011643 | |
| 2015-02-08 22:24 | Celebi | Note Added: 0011644 | |
| 2015-02-10 18:48 | Torr Samaho | Note Added: 0011650 | |
| 2015-02-10 19:58 | Cutman | Note Added: 0011651 | |
| 2015-02-10 20:35 | Edward-san | Note Added: 0011652 | |
| 2015-02-10 22:20 | RusselCS | Note Added: 0011654 | |
| 2015-02-14 05:23 | ZzZombo | Note Added: 0011679 | |
| 2015-02-14 17:58 | Torr Samaho | Note Added: 0011680 | |
| 2015-02-21 10:00 | Dark-Assassin | Note Added: 0011731 | |
| 2015-02-21 11:05 | Dusk | Note Added: 0011732 | |
| 2015-03-08 20:53 | Dusk | Assigned To | => Dusk |
| 2015-03-08 20:53 | Dusk | Status | new => assigned |
| 2015-03-09 01:21 | Dusk | Note Added: 0011797 | |
| 2015-03-09 01:22 | Dusk | Status | assigned => needs review |
| 2015-03-09 01:41 | Dusk | Note Edited: 0011797 | bug_revision_view_page.php?bugnote_id=11797#r6735 |
| 2015-03-09 01:44 | Dusk | Note Edited: 0011797 | bug_revision_view_page.php?bugnote_id=11797#r6736 |
| 2015-03-09 01:45 | Dusk | Note Edited: 0011797 | bug_revision_view_page.php?bugnote_id=11797#r6737 |
| 2015-03-09 01:59 | Dusk | Note Edited: 0011797 | bug_revision_view_page.php?bugnote_id=11797#r6738 |
| 2015-03-09 02:00 | RusselCS | Note Added: 0011798 | |
| 2015-03-09 04:31 | Dusk | Note Added: 0011800 | |
| 2015-03-09 04:41 | Dusk | Note Edited: 0011800 | bug_revision_view_page.php?bugnote_id=11800#r6740 |
| 2015-03-09 04:45 | Dusk | Note Edited: 0011800 | bug_revision_view_page.php?bugnote_id=11800#r6741 |
| 2015-03-09 19:48 | Torr Samaho | Note Added: 0011804 | |
| 2015-03-10 02:55 | Dusk | Note Added: 0011808 | |
| 2015-03-10 02:58 | Dusk | Note Edited: 0011808 | bug_revision_view_page.php?bugnote_id=11808#r6755 |
| 2015-03-10 21:00 | cobalt | Status | needs review => needs testing |
| 2015-03-10 21:00 | cobalt | Target Version | => 1.4 |
| 2015-03-10 21:00 | cobalt | Description Updated | bug_revision_view_page.php?rev_id=6757#r6757 |
| 2015-03-10 21:00 | cobalt | Additional Information Updated | bug_revision_view_page.php?rev_id=6759#r6759 |
| 2015-03-10 21:00 | cobalt | Note Added: 0011809 | |
| 2015-03-10 23:23 | Dusk | Note Added: 0011811 | |
| 2015-03-10 23:23 | Dusk | Status | needs testing => resolved |
| 2015-03-10 23:23 | Dusk | Fixed in Version | => 2.0 |
| 2015-03-10 23:23 | Dusk | Resolution | open => fixed |
| 2015-03-10 23:23 | Dusk | Fixed in Version | 2.0 => 1.4 |
| 2015-03-10 23:23 | Dusk | Description Updated | bug_revision_view_page.php?rev_id=6760#r6760 |
| 2015-03-10 23:23 | Dusk | Additional Information Updated | bug_revision_view_page.php?rev_id=6761#r6761 |
| 2018-09-30 23:40 | Blzut3 | Status | resolved => closed |
|
Notes |
|
|
(0011638)
|
|
Torr Samaho
|
2015-02-08 20:42
(edited on: 2015-02-08 20:45) |
|
I'm not convinced that a sprite significantly exceeding the hitbox makes sense. If you want a large sprite, you need a large player class.
Can you post an example of a sprite that has a legitimate reason to exceed the hitbox?
|
|
|
|
(0011641)
|
|
RusselCS
|
2015-02-08 21:08
(edited on: 2015-02-08 21:09) |
|
This skin is present in Megaman 8-Bit Deathmatch. Its firing frame causes it to go outside the limits of the scaled sprite hitbox, forcing it to get scaled down.
This is one of around 45 examples present in Megaman 8-Bit Deathmatch currently and I could site a good number more that weren't intended for jokes or exploits created by the game's community.
I'm sure more issues like this exist outside of Megaman 8-Bit Deathmatch, as someone mentioned on IRC that the game resized a good number of their skins at one point.
|
|
|
|
|
|
I see. I'd say these firing frames have a legitimate reason to exceed the hitbox. Let me think if there is a way for Zandronum to automatically allow this. |
|
|
|
(0011643)
|
|
Dusk
|
|
2015-02-08 21:57
|
|
|
As discussed on IRC we figured that laxing the restrictions for the firing frames would make a good compromise. All other players appear to use E and F frames for firing though Hexen's cleric uses the G frame as well (in which case death frames start from I onwards). |
|
|
|
(0011644)
|
|
Celebi
|
|
2015-02-08 22:24
|
|
I was asked to share these screenshots in hopes to show some examples to the problem.
'http://imgur.com/a/eQ0uN [^]'
In all these screenshots, we see a the base sprite for comparison. In the first screenshot, an edit of the base sprite is for some reason scaled to microscopic size. This sprite is exactly the same size as the base sprite. In the second screenshot, we see an exceptionally large skin scaled down. (It also has a tiny base sprite between the legs for comparing.) In the third/fourth screenshot, those skins are scaled down quite a bit compared to the base skin. (The fourth screenshot is also the example Lego_CS provides.) The fifth screenshot is one of the largest core skins for MM8BDM being scaled down. The last two screenshots are the main point. That skin is scaled down quite a bit like screenshot three. The final screenshot shows a comparison of the base sprite, the skin, and a boss within MM8BDM who uses that skin. |
|
|
|
|
|
I relaxed the restrictions on the non-walking frames. Please test if this is sufficient for your needs. |
|
|
|
(0011651)
|
|
Cutman
|
|
2015-02-10 19:58
|
|
Hi
Unfortunately this doesn't quite cut it. A lot of our bigger characters (skins) are still getting caught and resized. There's about 30 of them of varying sprite sizes.
I definitely think we need an additional feature to allow bigger player classes to be allowed bigger skins, if desired by the modder. There are many situations where skins want to be larger than the player's default hitbox (such as a marine carrying a war banner or something). Whether it be a flag in the playerclass or an actor property to specify the size limit.
As it stands right now, we have a LOT of skins in MM8BDM (bundled with the game and external) that are getting broken by this. We won't be able to update to the latest Zandronum version until this gets resolved as skins are a pretty big part of MM8BDM. |
|
|
|
|
|
can you export one okay skin and all the affected skins in separate files and upload here? |
|
|
|
|
Alright, here you go.
Note that these skins were designed to function alongside the class scale test pk3 included in the ticket above.
The base skin and MegaWaterS should be fully functional while all the others included in the zip file break in the test build Torr provided above.
I'm curious as to what you need them for, aside from manually adjusting the limits of the engine to fit these skins and ensuring they work. |
|
|
|
|
|
Torr, I don't think it's really doable to do what you're trying to do. I wanted once to make a mod there the player assumes a role of Archvile. You know, Archvile's sprites are much bigger than the ones of DoomPlayer. I couldn't make it to work because if I make the player's height or radius bigger, he won't be able to pass even stock Doom levels w/o getting stuck, so I gave up the idea. If you instead make the size check optional, it could work reliably. |
|
|
|
|
You honestly want those sprites to have the same hitbox
? I can't think of any sane check that would come to the conclusion that those are valid sprites for the same player class. If it's essential for your mod to use the same hitboxes for theses sprites, then we really need a flag that turns off the check completely or a property that allows to set the leeway to a ridiculously large size.
@ZzZombo If the Archvile is too big for the Doom levels, it sounds pretty reasonably to me to downsize the sprite accordingly. |
|
|
|
|
I'm in favour of actor properties.
player.maxheight & player.maxwidth
Default to normal height and width if not defined or <= 0 |
|
|
|
(0011732)
|
|
Dusk
|
|
2015-02-21 11:05
|
|
New actor properties make sense but I'd name them Player.MaxSkinHeight and Player.MaxSkinWidth
Player.MaxHeight and Player.MaxWidth imply that the hitbox size can be changed and that those properties would define limits to how far that could go. |
|
|
|
(0011797)
|
|
Dusk
|
2015-03-09 01:21
(edited on: 2015-03-09 01:59) |
|
PHEW this one was more involved than I thought
- Diff here
- Build
This adds the actor property Player.MaxSkinSizeFactor <widthfactor>, <heightfactor>
The skin gets downsized if the maximum width of the skin is larger than (2 * radius * widthfactor) or if its maximum height is larger than (height * heightfactor).
By default this is 3.44, 1.68, which means that skins wider than 3.44*16*2 = 110px or higher than 1.68*56 = 94px are downscaled to fit. That is to say, skins wider than 3.44x and skins taller than 1.68x are downscaled.
EDIT: Hmm.. as it is currently, the max width scale factor doesn't take actor scaling into account. So the megaman's max width scale factor values need to be multiplied by 2.5. This probably shouldn't be the case.
|
|
|
|
|
|
Works perfectly, thanks a lot. |
|
|
|
(0011800)
|
|
Dusk
|
2015-03-09 04:31
(edited on: 2015-03-09 04:45) |
|
This should take the actor's scale value into account.
|
|
|
|
|
|
Isn't the scale factor of the actor volatile (at least ZDoom has A_SetScale)? If that's the case, the scale factor should also scale the skin. So I'm not sure why this needs to be factored into the skin size check. Can you elaborate? |
|
|
|
(0011808)
|
|
Dusk
|
2015-03-10 02:55
(edited on: 2015-03-10 02:58) |
|
After considering the above I agree that scalex shouldn't be factored in so it's reverted now.
With that I think this is pretty much done and has been tested, so I made a pull request. Note that 28490e7 and d9ab9f9 cancel each other out so you could just pull in c4ba853.
|
|
|
|
(0011809)
|
|
cobalt
|
|
2015-03-10 21:00
|
|
|
|
|
(0011811)
|
|
Dusk
|
|
2015-03-10 23:23
|
|
|
Was already tested before being pulled (the build had a change afterwards but that was since reverted anyway), so I'm closing this now. |
|