Interkarma / daggerfall-unity

Open source recreation of Daggerfall in the Unity engine
http://www.dfworkshop.net
MIT License
2.7k stars 328 forks source link

Fixed frame overflow in DaggerfallMobileUnit. #2571

Closed KABoissonneault closed 7 months ago

KABoissonneault commented 7 months ago

Fixes for #2569.

I found an issue where archive 288 has 4 frames for record 0, 1, 2, and 4, but 8 frames for record 3. Record 3 is the "back diagonal" sprites (both left and right). Normally, all mobile archives have the same number of frames on each orientation, but the Ancient Lich is an exception. When changing orientation, the currentFrame is preserved so that the animation keeps playing from where it was. However, for the Ancient Lich, if you change orientation from the back diagonal while on frame 4, 5, 6, or 7 is playing, the currentFrame will now be past the allowed frames for the new orientation (back or side). On unmodded DFU, this reaches into a "Casting spell" frame in summary.AtlasRect (because rectIndex has enough room to overflow into the next records, rather than go out of bounds of the AtlasRect). When using texture replacements from mods (ex: Vanilla Enhanced), you get the exception shown in #2569 (because we don't use the atlas anymore).

The fix is on line 371 - I simply reset currentFrame to 0 if the frame is not valid for the new orientation. A bit jarring, but better than the current behavior.

The rest of the changes are more bounds checking and error logs for these issues. I find DaggerfallMobileUnit has lots of moving parts and lots of arrays to go out of bounds, so it helps to have better logs there.

The output looks like this:

image

Hopefully this will help diagnose future issues with mobiles, if any remain. In any case, with mods like Daggerfall Enemy Expansion, it's easy to accidentally create invalid MobileEnemy data that creates random errors in DaggerfallMobileUnit, and this will help mod development.

petchema commented 7 months ago

It's hard to talk about a "best solution" without knowing the original intent (and checking what's going on in Daggerfall classic will probably be difficult), but at least for the record, since both 4 and 8 frame cycles picture a single walking cycle (left foot forward, right foot forward), another solution would be to apply cross-multiplication when orientation changes

--- a/Assets/Scripts/Internal/DaggerfallMobileUnit.cs
+++ b/Assets/Scripts/Internal/DaggerfallMobileUnit.cs
@@ -328,6 +328,8 @@ namespace DaggerfallWorkshop
             int orientation = - Mathf.RoundToInt(enemyFacingAngle / anglePerOrientation);
             // Wrap values to 0 .. numberOrientations-1
             orientation = (orientation + numberOrientations) % numberOrientations;
+            if (lastOrientation >= 0)
+                currentFrame = currentFrame * summary.StateAnims[orientation].NumFrames / summary.StateAnims[lastOrientation].NumFrames;

             // Change enemy to this orientation
             if (orientation != lastOrientation)