Dragon Fogel
Level: Smitemaster
Rank Points: 2434
Registered: 06-21-2014
IP: Logged

D. Craven One posted this in a different thread and was asked to make a new thread about it, but never did. So I'm doing that.

Basically, this hold has issues:

http://forum.caravelgames.com/getattachment.php?id=39306

The issues I experienced when trying it:
-on my first attempt, I hit Wait a few times, there was lag, then the citizen was a few steps ahead and then DROD crashed.
-significant lag on the turn when the citizen tried to step on the platform
-beeps after the citizen made a few steps
-after the citizen reached the end of its path, I restarted the room. When the citizen was stepping on the raft, there was lag. I hit the wait button again and the game crashed, I'm guessing it was because the turn hadn't fully processed yet for some reason.

Since this is D. Craven One's sample hold, I don't know the specifics of the scripting. I just know that it causes lag, beeps, and crashes for some reason. This is in 5.1.0.6462.

skell
Level: Legendary Smitemaster

Rank Points: 3734
Registered: 12-28-2004
IP: Logged

Mike, could you please investigate this bug?

It starts in the CMonster::FindOptimalPath2, call to CMonster::OpenMove() which requests a position 2 tiles away from the player. This is already problematic, because that in turn calls CCharacter:oesSquareContainObstacle() which only takes two arguments, X and Y of the tile to check. If you scroll to the bottom of the method you'll see this code:

const int nFirstO = nGetO((int)wCol - (int)this->wX, (int)wRow - (int)this->wY);
if (room.CanMovePlatform(this->wX, this->wY, nFirstO))
    break;

This code breaks because if the distance to the target tile is greater than one nGetO() will often return gibberish (like orientation=9 which does not correspond to any orientation).

I don't have the deep knowledge of how both CMonster::FindOptimalPath2() and CDbRoom::CanMovePlatform() work so for me it would be a lot of experimentation and analysis and I don't really have time for such a commitment now .

I think it would be cool if we could fix this for 5.1, but if worse comes to worst we could probably dirty patch it by not calling CDbRoom::CanMovePlatform() if distance between Character position and the target tile is greater than 1. That's probably going to break behavior but will prevent crashes (I tested this by changing the if to:

if (nDist(wCol, wRow, this->wX, this->wY) == 1 && room.CanMovePlatform(this->wX, this->wY, nFirstO))

____________________________
My website | Facebook | Twitter

mrimer
Level: Legendary Smitemaster

Rank Points: 5056
Registered: 02-04-2003
IP: Logged

skell wrote:
It starts in the CMonster::FindOptimalPath2, call to CMonster::OpenMove() which requests a position 2 tiles away from the player.

When you say "player" here, do you mean monster (citizen)? And OpenMove is IsOpenMove, correct?

Assuming this is true, then it looks like the culprit is that DoesSquareContainObstacle() has had orientation-based logic added to it in some 5.1 fixes, but it was not enhanced to consider that checks would be made from a position other than the monster's current location.

The fix is to augment declarations of DoesSquareContainObstacle() with a source X+Y, just like how DoesArrowPreventMovement has a variant that does so. Careful here -- DoesSquareContainObstacle is polymorphic, so we need to ensure all method signatures are updated consistently.

To reduce the footprint of the change, it might be feasible to maintain an overloaded wrapper function that fills in (this->wX, this->wY) as the source for most calls (please see how DoesArrowPreventMovement has two versions that do this). If taking this route, please be certain that the 2-argument version is only called from locations that can correctly assume these default source locations, and that the polymorphic declarations are all the 4-argument variant.

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

---

[Last edited by mrimer at 02-19-2016 02:34 PM]

skell
Level: Legendary Smitemaster

Rank Points: 3734
Registered: 12-28-2004
IP: Logged

I'd like to defer it to after 5.1 is released since that's a pretty big change.

____________________________
My website | Facebook | Twitter

mrimer
Level: Legendary Smitemaster

Rank Points: 5056
Registered: 02-04-2003
IP: Logged

Okay, sure.

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

skell
Level: Legendary Smitemaster

Rank Points: 3734
Registered: 12-28-2004
IP: Logged

I have temporarily fixed this (in an ugly way) so it won't crash or throw assertion errors, but after 5.1 we'll need to do it properly.

____________________________
My website | Facebook | Twitter

Dragon Fogel
Level: Smitemaster
Rank Points: 2434
Registered: 06-21-2014
IP: Logged

Can confirm that the sloppy quick fix prevents problems with the hold in the initial post in 5.1.0.6478. There's no lag or crashes when the citizen moves on the platform.

mrimer
Level: Legendary Smitemaster

Rank Points: 5056
Registered: 02-04-2003
IP: Logged

Thank you both! I'll tackle applying the "correct" fix after 5.1 is released.

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

---

[Last edited by mrimer at 02-27-2016 07:29 PM]

mrimer
Level: Legendary Smitemaster

Rank Points: 5056
Registered: 02-04-2003
IP: Logged

Skell, I need to check with you whether we still feel there's something here that needs fixing. I can't recall whether there's a logic bug to address or we're just wanting to make the code look nicer in some way at this point, or whether this issue has already been resolved.

____________________________
Gandalf? Yes... That's what they used to call me.
Gandalf the Grey. That was my name.
I am Gandalf the White.
And I come back to you now at the turn of the tide.

---

[Last edited by mrimer at 10-21-2020 11:24 PM]

skell
Level: Legendary Smitemaster

Rank Points: 3734
Registered: 12-28-2004
IP: Logged

mrimer wrote:
Skell, I need to check with you whether we still feel there's something here that needs fixing. I can't recall whether there's a logic bug to address or we're just wanting to make the code look nicer in some way at this point, or whether this issue has already been resolved.

I don't remember anything about this issue anymore. Reading the whole thread it's clear there is a hack somewhere. Would be best to track it from the first post.

____________________________
My website | Facebook | Twitter