3.7.1 and morph actors
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.
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.
Re: 3.7.1 and morph actors
Is it about this commit? Anyway, this isn't fixed in the current master, 95995e4.
Re: 3.7.1 and morph actors
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.
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.
- Graf Zahl
- Lead GZDoom+Raze Developer
- Posts: 49073
- Joined: Sat Jul 19, 2003 10:19 am
- Location: Germany
Re: 3.7.1 and morph actors
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.
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.
Re: 3.7.1 and morph actors
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.
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.
Re: 3.7.1 and morph actors
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.Graf Zahl wrote:Seriously though, the unmorphing process needs some serious rethinking.
But that also makes a lot of sense._mental_ wrote:OK but we should fix this bug for upcoming release I think without significant and risky changes.
- Graf Zahl
- Lead GZDoom+Raze Developer
- Posts: 49073
- Joined: Sat Jul 19, 2003 10:19 am
- Location: Germany
Re: 3.7.1 and morph actors
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.
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.
Re: 3.7.1 and morph actors
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.
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.
- Graf Zahl
- Lead GZDoom+Raze Developer
- Posts: 49073
- Joined: Sat Jul 19, 2003 10:19 am
- Location: Germany
Re: 3.7.1 and morph actors
Yes, that's what I would have done, too. And surely be cautious about the owner being null.
Re: 3.7.1 and morph actors
Fixed in fc5420b.
- 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
Thank you for your time and attention. Much appreciated.