[~March 13?] drawmugshot ignores specification

Forum rules
Please don't bump threads here if you have a problem - it will often be forgotten about if you do. Instead, make a new thread here.

Post a reply

Smilies
:D :) :( :o :shock: :? 8-) :lol: :x :P :oops: :cry: :evil: :twisted: :roll: :wink: :geek: :ugeek: :!: :?: :idea: :arrow: :| :mrgreen: :3: :wub: >:( :blergh:
View more smilies
BBCode is OFF
Smilies are ON
Topic review
   

Expand view Topic review: [~March 13?] drawmugshot ignores specification

Re: [~March 13?] drawmugshot ignores specification

by Matt » Sat Apr 30, 2016 7:30 pm

Works like a charm, thanks!

Re: [~March 13?] drawmugshot ignores specification

by Blzut3 » Wed Apr 27, 2016 9:16 pm

Which is pretty much what I suggested. :P
Blzut3 wrote:My natural inclination at the moment is to just outright ignore that parameter and add a new one that is specifically for forcing a prefix, but I could use more opinions there.
Probably would have went with the syntax of custom("XYZ") myself, print a warning with the old syntax that was previously deprecated, but this works fine too.

Edit: Previous wording of that sentence overstated some stuff.

Re: [~March 13?] drawmugshot ignores specification

by Graf Zahl » Wed Apr 27, 2016 5:14 am

Well, since nobody bothered to even comment on this, I decided to address the problem myself.
Added a new flag for the 'drawmugshot' command, named "custom". If this is specified, the skin's mugshot is ignored and the one given in the command always used.

Re: [~March 13?] drawmugshot ignores specification

by Graf Zahl » Fri Apr 08, 2016 1:54 am

There's no need for it anymore.
As I said, you were exploiting a bug to achieve something the function was not supposed to do.
What I think this really needs is an alternative DrawMugshot function that draws the specified graphic no matter what but I'd prefer to get some input from Blzut3 first.

Re: [~March 13?] drawmugshot ignores specification

by Matt » Thu Apr 07, 2016 11:55 pm

This should replicate all the necessary logic that was in the HD implementation.
1. 2 items to be checked, both inheriting from an "InventoryFlag"
2. Looping ACS enter script that constantly checks the player's gender, then gives or takes items depending
3. SBARINFO that draws one or the other depending on the inventory item
Attachments
fmdmtest.7z
(7.44 KiB) Downloaded 47 times

Re: [~March 13?] drawmugshot ignores specification

by Graf Zahl » Thu Apr 07, 2016 6:38 am

Actually yes, it changed something very important:

Before the scripting branch, the Face property was metadata. Afterward it became an FString in PClassPlayerPawn.
The problem is: The DECORATE parser complained about empty mugshots but assigned the value anyway, so the classes actually did have a name, albeit an invalid one.

And the skin init code only checked for NULL, not empty strings.

With the transition to metaclasses and this turning into an FString these two states became identical.
So this only ever worked because a bug was exploited - one that got a message in the console btw.!

That the feature as such is pretty much broken right now is another story.

Re: [~March 13?] drawmugshot ignores specification

by randi » Mon Apr 04, 2016 7:16 pm

The July 20, 2015 version of Hideous Destructor works as intended with 2.8.1, but fmdmtest does not, which means that they don't work identically. After the scripting branch got merged in, it was pretty broken all around, but the first commit I found where a face was displayed at all showed the marine's. So apparently something happened in the scripting branch that changed it, but since Hideous Destructor is broken for a huge chunk of that, I can't really use it for any sort of testing. So maybe it was exploiting undefined behavior, or perhaps something important changed that needs to be repaired. I can't tell.

If you could redo fmdmtest and ensure that it works on 2.8.1, that would probably help.

Re: [~March 13?] drawmugshot ignores specification

by Blzut3 » Mon Apr 04, 2016 6:37 pm

So the default face parameter has been deprecated for a long long time (basically since 2008) because it has been in a state of flux of working and not working ever since the beginning. (Between skins and later player classes and morphs. Pretty much every time that code was touched it broke something using that parameter.) Which wouldn't be too horrible except I'm not sure that between release versions of ZDoom the semantics of that parameter has been consistent and I'm certain there are mods out there that specify the parameter even though they don't want it forced. My natural inclination at the moment is to just outright ignore that parameter and add a new one that is specifically for forcing a prefix, but I could use more opinions there.

Re: [~March 13?] drawmugshot ignores specification

by Matt » Mon Apr 04, 2016 5:30 pm

Thanks for the help in tracking this. It's looking more like "Hack broke" at this point given Graf Zahl's description of how the current setup works, though it's a mystery how it could have ever worked for so long to begin with. Looking at the changelog for anything involving SBARINFO, mugshots or inventory doesn't bring up anything that could reasonably have affected this. [EDIT: could this be it?]


The video is from a version that is definitely later than 2013. The mugshot was first implemented in October 2013, so it's after that change.


It seems that my web host caches old files, so...

I recall updating (G)ZDoom to the most recent SVN in August 2015, and it worked then, so July 20, 2015 should be old enough.

That said, here's October 28, 2013 when the feature was first added.

The ACS script should automatically give the relevant item once you set the player's gender to male or female.

Re: [~March 13?] drawmugshot ignores specification

by randi » Mon Apr 04, 2016 2:35 pm

Vaecrius wrote:In principle this attached file should work identically.
Do you still have any old versions of Hideous Destructor that can be loaded with old ZDooms?

Re: [~March 13?] drawmugshot ignores specification

by Graf Zahl » Mon Apr 04, 2016 1:09 pm

That said, I think the implementation of the drawmugshot command is utterly idiotic. If the mod specifies a name that should take precedence over some skin setting.


To sum it up, the following seems to happen:

Mugshot code prefers skin over everything else, unconditionally.
Every player got a skin, and if it's the default one.
Every skin got a mugshot assigned, it's normally STF.

So whatever you specify, it never gets through. Looks broken by design to me, like a large portion of the player class and skin code. It's a hopelessly messed up affair.

The only relevant thing I could find was this commit:

* - Fixed: Skin mugshots didn't load. (This adds a texture usetype for skin graphics.)

If that is indeed the cause here some serious refactoring of a broken feature is in order.


from 2013.

Re: [~March 13?] drawmugshot ignores specification

by Graf Zahl » Mon Apr 04, 2016 12:45 pm

So, is this "Hack broke" or "SBARINFO broke"?
Just looking at the mechanism this uses to decide gender makes be go "Ugh..."

As for "it always worked", has it ever occured to you that this information is essential in tracking down the cause?
All I can do now is pick around in the messed up commit tree to find something. Git bisect won't work here.

Re: [~March 13?] drawmugshot ignores specification

by Matt » Mon Apr 04, 2016 12:02 pm

Problem is that it had always worked until some time within the last few months when someone finally noticed it wasn't working in the most recent GZDoom release. (I have no reason to think this would be a GZDoom-specific bug but it's not impossible now that I think about it. I can find nothing on a GZDoom forum search for mugshot though.)

Fortunately there happens to be video evidence so at least I can prove on this thread I'm not talking out of my ass. I've sent Slydir a PM asking if he could recall which version this was.

Re: [~March 13?] drawmugshot ignores specification

by Graf Zahl » Sat Apr 02, 2016 4:25 am

I tested this all versions down to 2.2.0 and always got the Doomguy mugshot. Since this thing has no distinctive marks I cannot say what is wrong here, but this test appears to be useless.
Next time please verify that you got it working AS INTENDED on an older version and then name that version.

Re: [~March 13?] drawmugshot ignores specification

by _mental_ » Sat Apr 02, 2016 2:04 am

I tried your test file with ZDoom 2.7.1 and STF mugshot was displayed.
From what I see in the code, skin's face have precedence over a face from status bar info.
So the question from my previous post still persists: in which version it worked?

Top