gemrb / gemrb

GemRB is a portable open-source implementation of Bioware’s Infinity Engine.
https://gemrb.org
GNU General Public License v2.0
983 stars 184 forks source link

[iwd] Fleezum: No items dropped on death #955

Closed fizzet closed 4 years ago

fizzet commented 4 years ago

Bug description

IWD AR8007. If you kill Fleezum on this level, he doesn't drop any gear. He should, though.

He only carries equipped items, so they are supposed to get dropped in CheckOnDeath(). However, he also has this block in his script (ldfleez2.bcs, to align with his other self in ldfleez1.bcs):

IF
    !Global("FLEEZUM_DEAD","GLOBAL",0)
THEN
    RESPONSE #100
        DestroySelf()
END

So when he's killed the var is set to 1, and then the last time his script runs this block will be executed, and DestroySelf() removes the actor without dropping anything at all.

A possible fix for this might be to set death vars only in CheckOnDeath() rather than Die(). Not sure whether that would interfere with anything else, though. Thoughts?

lynxlynxlynx commented 4 years ago

I guess he needs to dump something equipped, otherwise it should be already handled.

Delaying death vars sounds less horrible than trying to somehow persist the equipping effects. Though it's also completely possible we wouldn't have trouble dumping everything in Die any more — fuzzie didn't provide a test case ("fixes crash"), so it's hard to say. All options are a bit scary, since one tick can make or break a script and effects can do practically anything.

So if dumping everything still doesn't work and isn't reasonably fixable (to be determined), how about we work around it in DestroySelf? Dump the inventory if the actor is already dead (IF_REALLYDIED). Could also have some fallout, but at least it won't be so widely systemic.

fizzet commented 4 years ago

That sounds like a last resort kind of thing, because the real problem probably isn't that DestroySelf doesn't dump items, but that a script sees its own death var (or that scripts still run after death, but there's probably a good reason for that?). So it might fix this particular instance but will leave the actual bug untouched.

lynxlynxlynx commented 4 years ago

Yeah, that's why I said it's better to investigate if we still have problems with dumping everything in Die. Scripts definitely run for deaders.

fizzet commented 4 years ago

But dumping everything in Die() wouldn't fix the script getting to see the death variable...

lynxlynxlynx commented 4 years ago

But is that truly desired? Are you saying in the original his corpse remains / that script block doesn't trigger at all?

Though looking at the iesdp docs for Die(d) triggers does corroborate a delay if you understand "last" as "previous":

0x0025 Die() Returns true if the active CRE has died in the last script round. NB. When a block returns true to this trigger, this will be the final block executed in the script, unless it is Continue'd

fizzet commented 4 years ago

I cannot try in the original engine, but I'd be surprised if the corpse was supposed to vanish. That block really only exists to remove the actor if the other Fleezum has been killed.

lynxlynxlynx commented 4 years ago

Looks like you're correct: https://youtu.be/rS9zpDFCrWA?t=204

b8fa49c2a1 did the opposite, however it was likely done when reversing bg2. Perhaps iwd is different?

fizzet commented 4 years ago

Possible, but I think it's more likely that our implementation differs from the original engine on other counts (e.g. how death is implemented), and the straight RE reimplementation doesn't account for that. That's pure speculation, however.

Any ideas where that sort of timing matters in BG?

lynxlynxlynx commented 4 years ago

No, nothing comes to mind. @bubb13: any thoughts?

The start of this thread is a bit of a goose chase, since I focused on the inventory, while clearly the problem is wider. We used to set the death vars in the tick after the actual death, but then it was moved to be done immediately. How does it work in the ees?

Bubb13 commented 4 years ago

A bit of a long-winded explanation, as this stuff is tricky enough to warrant it.

Die() and the death vars use the engine's message system: 1) Opcode 13 instantly drops the character's inventory when the corresponding death type allows it. 2) Die() and the death var are sent as messages by op13. 3) All messages are processed every game pass before normal game logic is executed.

When these messages are run depends on how op13 was executed. If op13 itself was applied via message, (most normal game actions do it this way), then Die() and the death var are set immediately, and are able to be detected on the same pass op13 was received. In rare instances the engine may apply an effect directly, (without using a message), but those are extreme edge cases which shouldn't affect this issue.

The "next tick" convention comes from the fact that if some game state is set by a message, and that message was sent as part of the normal AI processing, these messages have to wait until the next game pass to be executed. For example, a script casting a spell via Spell(), (assuming an instant effect application with no projectile), adds all spell effects to the message queue. Op13 will be executed on the next pass, after which its state changes, (Die() and the death var), can be detected.

For all intents and purposes, Die() and death var messages are sent out instantaneously by op13 when it is executed, yet due to how the game passes work, can only be detected on subsequent ticks. Also note how I'm describing things in terms of game passes; I give no guarantees on how the engine reports what tick of "world time" it is on. Given an instantaneous spell applying op13, the engine reports everything happening in the same tick, when in fact, it took two passes to complete the spell application.

So, finally, back to the actual topic. At least in IWD:EE, the relevant block in Fleezum's script always DestroySelf()'s him, even if he's the only Fleezum. However, given that op13 drops his inventory immediately, everything is put onto the ground before his script can detect his death var and delete him.

fizzet commented 4 years ago

Thanks for the explanation. That means DestroySelf() should be a noop for dead actors, though, since it would otherwise remove Fleezum's corpse, which the original apparently doesn't do, correct?

lynxlynxlynx commented 4 years ago

Since iwdee is based on bg2, I'm reading that there's a difference from vanilla iwd. I couldn't find an iwdee Fleezum video to double check though (did one, but he got chunked there).

But it also looks like moving the death var setting back could perhaps be fine. We don't share this indirection of the messaging system, so it could've been just sloppy RE. In-engine applications are rare, so I don't expect trouble on that side of things.

Bubb13 commented 4 years ago

Fleezum's corpse is DestroySelf()\'d in IWD:EE:

GIF ![fleezum_death mp4](https://user-images.githubusercontent.com/36863623/95266681-80d23900-07e8-11eb-8b4c-67c4cf258cad.gif)

The EE solution is to drop the inventory in op13, not in Die(). I don't know how the original IWD engine works, (and don't have a copy on hand) — would be interesting if the script is the same between versions, yet is evaluated differently. Without analyzing the original I can't really suggest what solution is better.