Interkarma / daggerfall-unity

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

WorldData does not ignore disabled mods #2573

Closed KABoissonneault closed 9 months ago

KABoissonneault commented 9 months ago

Something in WorldData is not ignoring disabled mods.

If you install a .dfmod mod with packaged WorldData overrides (new locations, modified locations, RMBs, ...), like Betony Restored, you should see new locations, like this:

image image

(Betony has more locations with Betony Restored installed than the classic map on EUSP)

Normally, you expect that disabling the mod would remove all these new locations, the same thing as removing the mod. However, disabling Betony Restored while still having it installed will show the same Travel map as the first screenshot, but the new locations will be unclickable, and show no name on hover. In addition, on visiting classic Betony locations, you get errors like:

Found DFBlock override: TEMPAAC0.RMB (index: 550) 
(Filename: C:\buildslave\unity\build\Runtime/Export/Debug/Debug.bindings.h Line: 39)

Found DFBlock override: RUINAA19.RMB (index: 891) 
(Filename: C:\buildslave\unity\build\Runtime/Export/Debug/Debug.bindings.h Line: 39)

DFTFU 1.9.2: Unknown block 'FARMAA08F.RMB'. 
(Filename: C:\buildslave\unity\build\Runtime/Export/Debug/Debug.bindings.h Line: 39)

DFTFU 1.9.2: Time to update terrains: 2645ms 
(Filename: C:\buildslave\unity\build\Runtime/Export/Debug/Debug.bindings.h Line: 39)

DFTFU 1.9.2: Unknown block 'FARMAA08F.RMB'. 
(Filename: C:\buildslave\unity\build\Runtime/Export/Debug/Debug.bindings.h Line: 39)

Exception: GetCompleteBuildingData() could not read block FARMAA08F.RMB
  at DaggerfallWorkshop.Utility.RMBLayout.GetLocationBuildingData (DaggerfallConnect.DFLocation& location) [0x0012b] in <9bf28c68d79b48218c80373c9e1cf18f>:0 
  at DaggerfallWorkshop.Game.BuildingDirectory.SetLocation (DaggerfallConnect.DFLocation& location, DaggerfallConnect.DFBlock[]& blocks) [0x0000b] in <9bf28c68d79b48218c80373c9e1cf18f>:0 
  at DaggerfallWorkshop.StreamingWorld+<UpdateLocation>d__81.MoveNext () [0x000c7] in <9bf28c68d79b48218c80373c9e1cf18f>:0 
  at UnityEngine.SetupCoroutine.InvokeMoveNext (System.Collections.IEnumerator enumerator, System.IntPtr returnValueAddress) [0x00026] in <85d1d3e7744a4a47b5f51883bf40bba2>:0 
UnityEngine.MonoBehaviour:StartCoroutineManaged2(IEnumerator)
UnityEngine.MonoBehaviour:StartCoroutine(IEnumerator)
DaggerfallWorkshop.StreamingWorld:UpdateLocations()
DaggerfallWorkshop.StreamingWorld:Update()

These errors also happen as you travel around Betony.

Going manually to a location added by Betony Restored gives you a ghost town, and plenty of runtime errors.

image

If you delete the .dfmod, the mod is removed fine.

It seems clear to me that some systems in WorldData are not properly filtering out disabled mods when grabbing assets. This needs investigating.

In the meantime, .dfmod mods involving WorldData overrides can:

KABoissonneault commented 9 months ago

The issue does not occur in the Editor, but occurs in the game.

I've narrowed the issue to a form of data race, where a PlayerGPS update causes a ContentReader location read, which makes it call EnumerateMaps, which makes it read all the regions in MapsFile, which loads the WorldDataReplacement location data for all the regions. Note that this is only the MapName and MapTable being loaded, the actual location is loaded when the terrain loads that map pixel later.

The exact callstack is this:

  at DaggerfallWorkshop.Utility.AssetInjection.WorldDataReplacement.GetDFRegionAdditionalLocationData (System.Int32 regionIndex, DaggerfallConnect.DFRegion& dfRegion) [0x00000] in <f0539ea58aaf42c18271da6a083227a3>:0 
  at DaggerfallConnect.Arena2.MapsFile.ReadRegion (System.Int32 region) [0x00000] in <f0539ea58aaf42c18271da6a083227a3>:0 
  at DaggerfallConnect.Arena2.MapsFile.LoadRegion (System.Int32 region) [0x00000] in <f0539ea58aaf42c18271da6a083227a3>:0 
  at DaggerfallConnect.Arena2.MapsFile.GetRegion (System.Int32 region) [0x00000] in <f0539ea58aaf42c18271da6a083227a3>:0 
  at DaggerfallWorkshop.Utility.ContentReader.EnumerateMaps () [0x00000] in <f0539ea58aaf42c18271da6a083227a3>:0 
  at DaggerfallWorkshop.Utility.ContentReader.MapDictCheck () [0x00000] in <f0539ea58aaf42c18271da6a083227a3>:0 
  at DaggerfallWorkshop.Utility.ContentReader.HasLocation (System.Int32 mapPixelX, System.Int32 mapPixelY, DaggerfallWorkshop.Utility.ContentReader+MapSummary& summaryOut) [0x00000] in <f0539ea58aaf42c18271da6a083227a3>:0 
  at DaggerfallWorkshop.PlayerGPS.UpdateWorldInfo (System.Int32 x, System.Int32 y) [0x00000] in <f0539ea58aaf42c18271da6a083227a3>:0 
  at DaggerfallWorkshop.PlayerGPS.Update () [0x00000] in <f0539ea58aaf42c18271da6a083227a3>:0  

In the editor, this call occurs after mods have been filtered out by ModManager.Init. In the standalone game, this occurs before. Meaning that in the standalone game, WorldDataReplacement will see disabled mods at the point of loading all region locations. These location data get cached and are never re-read. Later on when we go to load the location proper, the disabled mods will be filtered out, making the location load fail, creating all the issues I've listed in the post above.

For solutions, I'm investigating a few things:

  1. Why does PlayerGPS have a world position before loading a save file? If it stayed at -1, -1 until loading a save, the world info would not be updated, with would avoid loading the entirety of the content reader
  2. If the position in PlayerGPS is necessary (or cannot be figured out), do we need to notify all the systems if the player position changes while a game is not in progress?
  3. If all else fails, we can make WorldDataReplacement not load Location Data if the game is not in progress. They would get loaded on the next region load.

For reference, I've made a modified build with a log when WDR loads location data. Player(14).log

Starting on line 1969 For the next 200 lines, we load pretty much all regions in the game Notice line 2002: "Loading region 19". That's Betony

image

The lines below are clearly loading data from Betony Restored

AFTER that, on line 2186, we have the line "removing mod at index: 3". That's where it removes Betony Restored due to it being disabled (index 3 means 4th mod)

image