0002096: Add either an actor flag or actor property to allow custom player classes to control how big their skins should be

(0011638)
Torr Samaho (administrator)
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 (reporter)
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.

(0011642)
Torr Samaho (administrator)
2015-02-08 21:29

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 (developer)
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 (reporter)
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.

(0011650)
Torr Samaho (administrator)
2015-02-10 18:48

I relaxed the restrictions on the non-walking frames. Please test if this is sufficient for your needs.

(0011651)
Cutman (reporter)
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.

(0011652)
Edward-san (developer)
2015-02-10 20:35

can you export one okay skin and all the affected skins in separate files and upload here?

(0011654)
RusselCS (reporter)
2015-02-10 22:20

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.

(0011679)
ZzZombo (reporter)
2015-02-14 05:23

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.

(0011680)
Torr Samaho (administrator)
2015-02-14 17:58

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.

(0011731)
Dark-Assassin (administrator)
2015-02-21 10:00

I'm in favour of actor properties.
player.maxheight & player.maxwidth
Default to normal height and width if not defined or <= 0

(0011732)
Dusk (developer)
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 (developer)
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.

(0011798)
RusselCS (reporter)
2015-03-09 02:00

Works perfectly, thanks a lot.

(0011800)
Dusk (developer)
2015-03-09 04:31
edited on: 2015-03-09 04:45

This should take the actor's scale value into account.

(0011804)
Torr Samaho (administrator)
2015-03-09 19:48

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 (developer)
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 (updater)
2015-03-10 21:00

Issue addressed by commit 6a905c181ee3: - added new player pawn property Player.MaxSkinSizeFactor which allows customization of how large, compared to the hitbox, skins may be. (addresses 2096) - made the warning message caused by exceeding of this limit more informative.
Committed by Teemu Piippo [Dusk] on Monday 09 March 2015 03:18:47

Changes in files:
docs/zandronum-history.txt | 1 +
src/d_player.h | 4 ++++
src/r_things.cpp | 50 ++++++++++++++++++++++++++++++++++++--------------
src/thingdef/thingdef_properties.cpp | 15 +++++++++++++++
wadsrc/static/actors/shared/player.txt | 1 +
5 files changed, 57 insertions(+), 14 deletions(-)

(0011811)
Dusk (developer)
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.

This issue is already marked as resolved. If you feel that is not the case, please reopen it and explain why.
Supporters:	Celebi Messatsu Cutman ZzZombo
Opponents:	No one explicitly opposes this issue yet.

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	View Revisions
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	View Revisions
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	View Revisions
2015-03-09 01:44	Dusk	Note Edited: 0011797	View Revisions
2015-03-09 01:45	Dusk	Note Edited: 0011797	View Revisions
2015-03-09 01:59	Dusk	Note Edited: 0011797	View Revisions
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	View Revisions
2015-03-09 04:45	Dusk	Note Edited: 0011800	View Revisions
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	View Revisions
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	View Revisions
2015-03-10 21:00	cobalt	Additional Information Updated	View Revisions
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	View Revisions
2015-03-10 23:23	Dusk	Additional Information Updated	View Revisions
2018-09-30 23:40	Blzut3	Status	resolved => closed

Notes
(0011638) Torr Samaho (administrator) 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 (reporter) 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.

(0011642) Torr Samaho (administrator) 2015-02-08 21:29	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 (developer) 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 (reporter) 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.

(0011650) Torr Samaho (administrator) 2015-02-10 18:48	I relaxed the restrictions on the non-walking frames. Please test if this is sufficient for your needs.

(0011651) Cutman (reporter) 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.

(0011652) Edward-san (developer) 2015-02-10 20:35	can you export one okay skin and all the affected skins in separate files and upload here?

(0011654) RusselCS (reporter) 2015-02-10 22:20	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.

(0011679) ZzZombo (reporter) 2015-02-14 05:23	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.

(0011680) Torr Samaho (administrator) 2015-02-14 17:58	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.

(0011731) Dark-Assassin (administrator) 2015-02-21 10:00	I'm in favour of actor properties. player.maxheight & player.maxwidth Default to normal height and width if not defined or <= 0

(0011732) Dusk (developer) 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 (developer) 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.44162 = 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.

(0011798) RusselCS (reporter) 2015-03-09 02:00	Works perfectly, thanks a lot.

(0011800) Dusk (developer) 2015-03-09 04:31 edited on: 2015-03-09 04:45	This should take the actor's scale value into account.

(0011804) Torr Samaho (administrator) 2015-03-09 19:48	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 (developer) 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 (updater) 2015-03-10 21:00	Issue addressed by commit 6a905c181ee3: - added new player pawn property Player.MaxSkinSizeFactor which allows customization of how large, compared to the hitbox, skins may be. (addresses 2096) - made the warning message caused by exceeding of this limit more informative. Committed by Teemu Piippo [Dusk] on Monday 09 March 2015 03:18:47 Changes in files: docs/zandronum-history.txt \| 1 + src/d_player.h \| 4 ++++ src/r_things.cpp \| 50 ++++++++++++++++++++++++++++++++++++-------------- src/thingdef/thingdef_properties.cpp \| 15 +++++++++++++++ wadsrc/static/actors/shared/player.txt \| 1 + 5 files changed, 57 insertions(+), 14 deletions(-)

(0011811) Dusk (developer) 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.

Relationships