3.7.1 and morph actors

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: 3.7.1 and morph actors

Re: 3.7.1 and morph actors

by Vostyok » Thu Jan 17, 2019 9:18 am

Thank you for your time and attention. Much appreciated.

Re: 3.7.1 and morph actors

by _mental_ » Thu Jan 17, 2019 7:08 am

Fixed in fc5420b.

Re: 3.7.1 and morph actors

by Graf Zahl » Mon Jan 14, 2019 1:15 am

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

by _mental_ » Sun Jan 13, 2019 3:39 am

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.

Re: 3.7.1 and morph actors

by Graf Zahl » Sat Jan 05, 2019 7:53 am

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.

Re: 3.7.1 and morph actors

by Enjay » Sat Jan 05, 2019 6:59 am

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.

Re: 3.7.1 and morph actors

by _mental_ » Sat Jan 05, 2019 6:52 am

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.

Re: 3.7.1 and morph actors

by Graf Zahl » Sat Jan 05, 2019 6:21 am

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.

Re: 3.7.1 and morph actors

by _mental_ » Sat Jan 05, 2019 5:03 am

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.

Re: 3.7.1 and morph actors

by _mental_ » Sat Jan 05, 2019 2:05 am

Is it about this commit? Anyway, this isn't fixed in the current master, 95995e4.

Re: 3.7.1 and morph actors

by Graf Zahl » Fri Jan 04, 2019 11:57 am

If that was indeed the cause it should be fixed now. I can't test right now, can someone confirm?

Re: 3.7.1 and morph actors

by Major Cooke » Fri Jan 04, 2019 11:43 am

Hmmm, strange. I must've been doing something weird then.

Re: 3.7.1 and morph actors

by Graf Zahl » Fri Jan 04, 2019 11:28 am

That should never happen. The item would remove itself from the owner before actually destroying itself.

Re: 3.7.1 and morph actors

by Major Cooke » Fri Jan 04, 2019 11:14 am

Couldn't agree more. I could see inventory being much easier to maintain inside of a dynamic array, simply because one can delete nulls from the inventory and move on until it hits the size. Huh. Guess I might look into that for my own mods.

Whereas when using probes...

Code: Select all

for (let probe = owner.mo.Inv; probe != null; probe = probe.Inv)
The moment that item is null, the probe quits. And it goes null quite quickly when Destroy() is called, or so it would seem.

Re: 3.7.1 and morph actors

by Graf Zahl » Fri Jan 04, 2019 10:53 am

It accessed the address of the owner's first inventory pointer. I hate this linked list crap, especially if written like here. In my experience, linked lists just do not work in any conceivable situation and I would have redone the inventory storage as an array if it hadn't been exposed to ZScript already.

Top