3.7.1 and morph actors

Bugs that have been investigated and resolved somehow.

Moderator: GZDoom Developers

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.
_mental_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: 3.7.1 and morph actors

Post by _mental_ »

Is it about this commit? Anyway, this isn't fixed in the current master, 95995e4.
_mental_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: 3.7.1 and morph actors

Post by _mental_ »

Cooke is right, the problem is in rewritten loops.

To fix this particular issue it's enough to replace Actor invp = self; with Actor invp = item.Owner; here.
Item's owner in needed because we are trying to remove it from normal player pawn, not morphed one, and this breaks inventory linked list.
Without this, for loop does nothing, and the inventory list is cut after the removed item.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: 3.7.1 and morph actors

Post by Graf Zahl »

So in other words, the function gets called from the wrong actor. This means the call needs to get fixed, not the function. All the function needs is to do nothing if the item doesn't belong to the calling actor.

Seriously though, the unmorphing process needs some serious rethinking. It cannot be that it fucks up the self pointer of the calling functions.
Actual unmorphing needs to be handled OUTSIDE PlayerThink or these problems will always come back haunting us - even if it breaks a small number of hacky mods that try to abuse the system.
_mental_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: 3.7.1 and morph actors

Post by _mental_ »

OK but we should fix this bug for upcoming release I think without significant and risky changes.

Here is the problem. toucher is equal to players[consoleplayer].mo for AddInventory() call.
This is not the case for RemoveInventory() because Use() does morphing changing player pawn actor.
For this reason item's owner was important for that RemoveInventory() call.
The value of toucher remains the same while player pawn is a different actor.
User avatar
Enjay
 
 
Posts: 26517
Joined: Tue Jul 15, 2003 4:58 pm
Location: Scotland
Contact:

Re: 3.7.1 and morph actors

Post by Enjay »

Graf Zahl wrote:Seriously though, the unmorphing process needs some serious rethinking.
Indeed. Although I'm not au fait with the code itself, the mere fact that it breaks so often (I can't count the number of times an unmorph bug has appeared after an update) indicates that there is something fundamentally unstable about the setup.
_mental_ wrote:OK but we should fix this bug for upcoming release I think without significant and risky changes.
But that also makes a lot of sense.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: 3.7.1 and morph actors

Post by Graf Zahl »

For a point release, the fix should obviously be kept as simple as possible.

But the fix should still not be in RemoveInventory but in the calling code. This actually needs to set toucher to the new owner before calling RemoveInventory so that the code calling CallTryPickup doesn't continue to operate on the invalid player. The weird setup of this function was only done to accomodate morphing, but missed this path because it wasn't an issue when the code was written.
_mental_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: 3.7.1 and morph actors

Post by _mental_ »

To fix the provided sample it's enough to add toucher = self.owner; after Inventory.Use() call.
I guess this should be done conditionally, i.e. it must depend on value of usegood at least. Probably the owner can be null too.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: 3.7.1 and morph actors

Post by Graf Zahl »

Yes, that's what I would have done, too. And surely be cautious about the owner being null.
_mental_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: 3.7.1 and morph actors

Post by _mental_ »

Fixed in fc5420b.
User avatar
Vostyok
Posts: 1666
Joined: Sat Jan 17, 2015 8:54 am
Preferred Pronouns: No Preference
Location: Discord: Vostyok#3164
Contact:

Re: 3.7.1 and morph actors

Post by Vostyok »

Thank you for your time and attention. Much appreciated.
Post Reply

Return to “Closed Bugs [GZDoom]”