Closed twostars closed 5 months ago
Hi ! Thanks for the very detailed commit, it's certainly a big problem that slipped through, I've shared that with other devs, I personally think it needs to be fixed before the next release!
As a little aside, I think having a battle FPS of 60 also tends to cause characters to teleport a bit for some reason in battle, but that's a little anecdotal and I didn't really test or investigate that before I lowered it to 30 (in hindsight, that may also just be related to the PSX movement setting). I just point it out because I noticed its default was changed in this commit too.
Next update is coming with a big overhaul of 60/120fps by SamsamTS, https://github.com/Albeoris/Memoria/pull/509 Every aspect of the game is reworked on that aspect, so 60fps will really feel better than 30 in that update. No more teleporting, camera going too far, blocky animation...
Before I continue with the playthrough with the setting disabled, I figured I should also just get a backup of my saves in case it's of any use for getting to this point for testing. I attached it to the original post; the autosave takes you straight there.
Before I continue with the playthrough with the setting disabled, I figured I should also just get a backup of my saves in case it's of any use for getting to this point for testing. I attached it to the original post; the autosave takes you straight there.
This is very nice, but we have a save file with every save of the game : )
Yep I can reproduce it, we might have an hypothesis
Seems like adding an exception for this field where you found the code works
Okay, so I was very curious, and decided to look into it a bit more.
I observe that the position is applied down here:
if (this.gMode == 1)
{
actorController = actor.fieldMapActorController;
if (actorController == null)
{
...
}
else
{
actorController.curPos += moveVec; // <--
actorController.SyncPosToTransform();
}
}
When it's broken, its original position is (213.9, 729.6, 3305.0)
moveVec
is (3.5, 0.0, -52.8)
This modifies it to: (217.4, 729.6, 3252.2)
Which is, as far we're concerned here, "fine".
The next movement tick, however, we find that actorController.curPos
is (213.9, 729.6, 3305.0)
.
... it's been reverted. Which is why it retries it again on the next tick and can't make progress.
Naively I assumed this should be easy to figure out what's reverting it in between; just set a data breakpoint on it and see what hits. But no, this is C# and apparently we don't have that luxury here.
What resets it is, funnily enough, FieldMapActorController.ResetPos()
:
public void ResetPos()
{
this.curPos = this.lastPos;
this.activeFloor = this.lastFloor;
this.activeTri = this.lastTri;
}
From:
> Void FieldMapActorController:ResetPos ()+0x1 at F:\git\Memoria\Assembly-CSharp\Global\Field\Map\Actor\FieldMapActorController.cs:583 C#
Void FieldMapActorController:UpdateActiveTri ()+0x1ce at F:\git\Memoria\Assembly-CSharp\Global\Field\Map\Actor\FieldMapActorController.cs:952 C#
Void FieldMapActorController:UpdateMovement (Boolean)+0x351 at F:\git\Memoria\Assembly-CSharp\Global\Field\Map\Actor\FieldMapActorController.cs:366 C#
Void FieldMapActorController:HonoUpdate ()+0x203 at F:\git\Memoria\Assembly-CSharp\Global\Field\Map\Actor\FieldMapActorController.cs:208 C#
Void HonoBehaviorSystem:Update ()+0x129 at F:\git\Memoria\Assembly-CSharp\Global\Hono\Behavior\HonoBehaviorSystem.cs:118 C#
Which is triggered because this fails on the new position in FieldMapActorController.UpdateActiveTri()
:
Int32 activeTriIdxAtPos = this.GetActiveTriIdxAtPos(this.curPos);
if (activeTriIdxAtPos != -1)
So to my understanding, the predicted new position is outside of the defined walk mesh (for the field map?). It isn't aware of it here, and only gets reset later, causing it to keep trying the same movement and never actually get anywhere.
Offhandedly, I feel like you could just test the new position to verify it's still valid, and consider the movement "done" after that, but in this case it would still be wrong because it's still a few ticks off hitting its destination, based on the working pathing. You could at least detect it getting stuck this way, but what purpose that would serve beyond more hacks, I don't really know.
I have to ask though - what is this movement behaviour intended for, exactly? I've never played this game before, let alone the original PSX version. Was it even intended to apply to scripted actors in the first place? I see that it's enforced elsewhere, so I wonder if there's even a reason for this to apply here in the first place, because my concern is there's other issues with the walk meshes.
For reference, the end of the working pathing:
actor.sid=5, x=222, y=0, z=3000, curX=204, curY=809.5492, curZ=3453.001
actor.sid=5, x=222, y=0, z=3000, curX=208, curY=777.1299, curZ=3393.002
actor.sid=5, x=222, y=0, z=3000, curX=212, curY=744.7105, curZ=3333.001
actor.sid=5, x=222, y=0, z=3000, curX=216, curY=712.2911, curZ=3273.001
actor.sid=5, x=222, y=0, z=3000, curX=220, curY=680.8035, curZ=3213.001
actor.sid=5, x=222, y=0, z=3000, curX=224, curY=650.381, curZ=3153.001
actor.sid=5, x=222, y=0, z=3000, curX=228, curY=619.9585, curZ=3093.001
actor.sid=5, x=222, y=0, z=3000, curX=232, curY=589.536, curZ=3033.001
vs the end of the broken pathing:
actor.sid=5, x=222, y=0, z=3000, curX=203.3101, curY=815.1412, curZ=3463.351
actor.sid=5, x=222, y=0, z=3000, curX=206.8292, curY=786.6191, curZ=3410.564
actor.sid=5, x=222, y=0, z=3000, curX=210.3484, curY=758.0971, curZ=3357.777
actor.sid=5, x=222, y=0, z=3000, curX=213.8676, curY=729.575, curZ=3304.99
actor.sid=5, x=222, y=0, z=3000, curX=213.8676, curY=729.575, curZ=3304.99
...
Back to the mesh itself to verify what's actually valid, we have 24 triangles (which I wish were represented better in the debugger, but here we go):
0:
+ [0] "(618.0, -395.0, 1119.0)" UnityEngine.Vector3
+ [1] "(-609.0, -395.0, 1119.0)" UnityEngine.Vector3
+ [2] "(315.0, 87.0, 2055.0)" UnityEngine.Vector3
1:
+ [0] "(315.0, 87.0, 2055.0)" UnityEngine.Vector3
+ [1] "(-609.0, -395.0, 1119.0)" UnityEngine.Vector3
+ [2] "(-295.0, 87.0, 2055.0)" UnityEngine.Vector3
2:
+ [0] "(315.0, 87.0, 2055.0)" UnityEngine.Vector3
+ [1] "(-295.0, 87.0, 2055.0)" UnityEngine.Vector3
+ [2] "(241.0, 239.0, 2348.0)" UnityEngine.Vector3
3:
+ [0] "(241.0, 239.0, 2348.0)" UnityEngine.Vector3
+ [1] "(-295.0, 87.0, 2055.0)" UnityEngine.Vector3
+ [2] "(-216.0, 239.0, 2348.0)" UnityEngine.Vector3
4:
+ [0] "(241.0, 239.0, 2348.0)" UnityEngine.Vector3
+ [1] "(-216.0, 239.0, 2348.0)" UnityEngine.Vector3
+ [2] "(400.0, 443.0, 2744.0)" UnityEngine.Vector3
5:
+ [0] "(400.0, 443.0, 2744.0)" UnityEngine.Vector3
+ [1] "(-216.0, 239.0, 2348.0)" UnityEngine.Vector3
+ [2] "(-389.0, 443.0, 2744.0)" UnityEngine.Vector3
6:
+ [0] "(400.0, 443.0, 2744.0)" UnityEngine.Vector3
+ [1] "(-389.0, 443.0, 2744.0)" UnityEngine.Vector3
+ [2] "(213.0, 695.0, 3241.0)" UnityEngine.Vector3
7:
+ [0] "(213.0, 695.0, 3241.0)" UnityEngine.Vector3
+ [1] "(-389.0, 443.0, 2744.0)" UnityEngine.Vector3
+ [2] "(-230.0, 695.0, 3241.0)" UnityEngine.Vector3
8:
+ [0] "(213.0, 695.0, 3241.0)" UnityEngine.Vector3
+ [1] "(-230.0, 695.0, 3241.0)" UnityEngine.Vector3
+ [2] "(389.0, 963.0, 3737.0)" UnityEngine.Vector3
9:
+ [0] "(389.0, 963.0, 3737.0)" UnityEngine.Vector3
+ [1] "(-230.0, 695.0, 3241.0)" UnityEngine.Vector3
+ [2] "(-428.0, 963.0, 3737.0)" UnityEngine.Vector3
10:
+ [0] "(236.0, 2331.0, 6957.0)" UnityEngine.Vector3
+ [1] "(-202.0, 2331.0, 6957.0)" UnityEngine.Vector3
+ [2] "(-224.0, 2421.0, 8323.0)" UnityEngine.Vector3
11:
+ [0] "(-224.0, 2421.0, 8323.0)" UnityEngine.Vector3
+ [1] "(236.0, 2331.0, 6957.0)" UnityEngine.Vector3
+ [2] "(226.0, 2421.0, 8323.0)" UnityEngine.Vector3
12:
+ [0] "(318.0, 1463.0, 4703.0)" UnityEngine.Vector3
+ [1] "(-329.0, 1462.0, 4703.0)" UnityEngine.Vector3
+ [2] "(286.0, 1607.0, 4981.0)" UnityEngine.Vector3
13:
+ [0] "(286.0, 1607.0, 4981.0)" UnityEngine.Vector3
+ [1] "(-329.0, 1462.0, 4703.0)" UnityEngine.Vector3
+ [2] "(-307.0, 1607.0, 4981.0)" UnityEngine.Vector3
14:
+ [0] "(-307.0, 1607.0, 4981.0)" UnityEngine.Vector3
+ [1] "(286.0, 1607.0, 4981.0)" UnityEngine.Vector3
+ [2] "(203.0, 1883.0, 5448.0)" UnityEngine.Vector3
15:
+ [0] "(203.0, 1883.0, 5448.0)" UnityEngine.Vector3
+ [1] "(-307.0, 1607.0, 4981.0)" UnityEngine.Vector3
+ [2] "(-212.0, 1883.0, 5448.0)" UnityEngine.Vector3
16:
+ [0] "(203.0, 1883.0, 5448.0)" UnityEngine.Vector3
+ [1] "(-212.0, 1883.0, 5448.0)" UnityEngine.Vector3
+ [2] "(-202.0, 2331.0, 6957.0)" UnityEngine.Vector3
17:
+ [0] "(-202.0, 2331.0, 6957.0)" UnityEngine.Vector3
+ [1] "(203.0, 1883.0, 5448.0)" UnityEngine.Vector3
+ [2] "(236.0, 2331.0, 6957.0)" UnityEngine.Vector3
18:
+ [0] "(283.0, 1315.0, 4418.0)" UnityEngine.Vector3
+ [1] "(-306.0, 1169.0, 4140.0)" UnityEngine.Vector3
+ [2] "(-295.0, 1315.0, 4418.0)" UnityEngine.Vector3
19:
+ [0] "(-329.0, 1462.0, 4703.0)" UnityEngine.Vector3
+ [1] "(-295.0, 1315.0, 4418.0)" UnityEngine.Vector3
+ [2] "(318.0, 1463.0, 4703.0)" UnityEngine.Vector3
20:
+ [0] "(318.0, 1463.0, 4703.0)" UnityEngine.Vector3
+ [1] "(-295.0, 1315.0, 4418.0)" UnityEngine.Vector3
+ [2] "(283.0, 1315.0, 4418.0)" UnityEngine.Vector3
21:
+ [0] "(389.0, 963.0, 3737.0)" UnityEngine.Vector3
+ [1] "(-428.0, 963.0, 3737.0)" UnityEngine.Vector3
+ [2] "(283.0, 1169.0, 4140.0)" UnityEngine.Vector3
22:
+ [0] "(283.0, 1169.0, 4140.0)" UnityEngine.Vector3
+ [1] "(-428.0, 963.0, 3737.0)" UnityEngine.Vector3
+ [2] "(-306.0, 1169.0, 4140.0)" UnityEngine.Vector3
23:
+ [0] "(283.0, 1169.0, 4140.0)" UnityEngine.Vector3
+ [1] "(-306.0, 1169.0, 4140.0)" UnityEngine.Vector3
+ [2] "(283.0, 1315.0, 4418.0)" UnityEngine.Vector3
(triFlags is 1 for all of them, so they're all eligible for the point test)
So at (213.9, 729.6, 3305.0) we are intended to be roughly ~8:
+ [0] "(213.0, 695.0, 3241.0)" UnityEngine.Vector3
+ [1] "(-230.0, 695.0, 3241.0)" UnityEngine.Vector3
+ [2] "(389.0, 963.0, 3737.0)" UnityEngine.Vector3
Which would be the case for its original pathing at:
actor.sid=5, x=222, y=0, z=3000, curX=212, curY=744.7105, curZ=3333.001
But we're a liiiittle bit outside it.
In Hades Workshop we can see the applicable triangle here (I'm not too sure on the walkpath distinctions in this tool though, since it seems to check all of its triangles):
Which generally just brings me back to: is this even intended for scripted actors, or was this just intended for normal player movement?
The config's page simply says this, which is a little bit vague on the subject:
[0/1] When enabled, the movement algorithm is adjusted to fit more the PSX behaviour, which is slower than the Steam version on sloping paths.
Edit: This is also a thing I didn't see, but still doesn't necessarily indicate (to me at least) if it's intended for just players:
/*
The PSX movement algorithm is different than the remaster's movement algorithm
In remaster, characters move at a given speed (eg. 60/tick for running with player character) in the horizontal plane, then the height is adjusted to launch on a valid walkmesh triangle
In PSX, it seems that characters move at that speed in the plane containing the current triangle (and then the height is adjusted just like remaster)
If that is true, it results in characters moving faster on steep slopes for the remaster version, and in characters moving either a bit slower or a bit faster on irregular walkmeshes ("irregular" = triangles are not aligned on the same plane)
*/
public static Boolean PSXMovementMethod => Instance._control.PSXMovementMethod;
So as a result of the slower speed it just happens to land outside a triangle in the walk mesh here due to the walk mesh being a bit jagged and it moving in a smaller increment than it does normally.
I think for now I'll just continue with the setting off entirely, as I don't really trust that it won't be an issue with other areas, but it's certainly an interesting issue.
To answer your question: the PSX movement method is intended to apply on NPCs as well, as there's no reason a priori they would move differently than the player character. I must have done something wrong somewhere because there are other cutscenes in which it triggers a bug like this one (#345). It is true though that the difference was spotted for the player character, in particular in some places (typically, at the end of the game, on the field right before the last saving point of the game).
It might be that NPCs should just use the remaster movement method, either because they already used that method in the PSX version for some reason, or because it just causes too much troubles for something that was mainly meant for the player character in the first place.
On the technical side, the movement shouldn't reset a character to its last position in that kind of situation but rather find the nearest valid position which is approximately in the same direction. That should be done in FieldMapActorController.ServiceChar
. I don't know why that doesn't work there or why the nearest valid position would be the same as the previous position (the geometric configuration would rather suggest that the soldier should be pushed a bit to his right when walking down the stairs). So I don't know.
Thanks a bunch for your extensive research on that issue.
Thank you for the very detailed breakdown and analysis!
To add from what @Tirlititi said, just putting in there that's you're very welcome if you feel some things could be improved and you want to work on them. Enjoy playing for the first time! You're exactly at the end of CD1/4
@Tirlititi maybe the solution is to only apply the setting to Player, that would ensure there's no more bugs with NPCs, no?
I disabled the script for non-player character, it should fix every potential instance. I pushed the hotfix in the current release.
He's required to finish walking before the dialogue will continue, which causes the game to get stuck and not display the next line: "Who are you!?"
Instead, he ends up stuck in place, not reaching his destination:
This is my first playthrough of the game so it was a little tricky to find since I didn't really know where this even was in the game (besides being "before Burmecia somewhere"), but ultimately managed to track it down to "Burmecia/Pathway" (758), and Burmecian_SoldierA's script, specifically:
So it forces a synchronous walk to 222, 3000 before it's able to show the next line, which breaks in
EventEngine.MoveToward_mixed_ex()
as a result of:Beyond this point I haven't investigated too much so I don't know the exact reasoning for this, but it gets stuck with
distanceHasIncreased
remainingfalse
(because it doesn't move,moveDistance > actor.lastdist
remainsfalse
because both distances are the same), ultimately continuing to returntrue
(becausesyncMove
stays true) which causes the script to not advance.I was confused as to why the issue hadn't been observed more; I was, supposedly, using the repository's public release build. It turns out that I was unintentionally using the repository's latest config file because I'd run it once first from the repo before I swapped it to the last public release (to be on the safe side for my playthrough). Oops.
The default for this setting was changed in 91309cb5d24a34c46789da45852f60331605e0f8 which was after the last public release.
As a little aside, I think having a battle FPS of 60 also tends to cause characters to teleport a bit for some reason in battle, but that's a little anecdotal and I didn't really test or investigate that before I lowered it to 30 (in hindsight, that may also just be related to the PSX movement setting). I just point it out because I noticed its default was changed in this commit too.
Edit: Here are my saves -- the autosave takes you straight to the problematic area: FF9-BurmeciaPathway-autosave.zip