Interkarma / daggerfall-unity

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

WorldData mess when changing building types #2601

Open KABoissonneault opened 9 months ago

KABoissonneault commented 9 months ago

Describe the issue

WorldData mods change locations, blocks (RMBs), and buildings. When it comes to a building's type, it is specified in all three files. For this issue, we're using Alik'ra (in Alik'r Desert) as an example. In classic data, this location has two temples and one guild hall: the "Fighter Trainers" temple (FG), the Temple of Stendarr temple, and the Mages Guild guildhall (MG).

Let's focus specifically on the FG building in classic data.

location-0-165.json:

...
"BuildingCount": 254,
"Buildings": [
...

            {
                "NameSeed": 27890,
                "FactionId": 41,
                "Sector": 440,
                "LocationId": 27865,
                "BuildingType": "Temple",
                "Quality": 10
            },
...
],

FIGHBL00.RMB.json:

...
            "BuildingDataList": [
                {
                    "NameSeed": 0,
                    "FactionId": 41,
                    "Sector": 0,
                    "LocationId": 0,
                    "BuildingType": "Temple",
                    "Quality": 7
                },

FIGHBL00.RMB-677-building0.json:

{
    "FactionId": 0,
    "BuildingType": 14,
...

(14 is Temple, Guildhall would be 11)

We have the mod "Unofficial Blocks, Locations, and Models Fixes" (UBLaMF) that overrides location-0-165.json to change FG's building type to "Guildhall", and replaces FIGHBL00.RMB-677-building0.json with BuildingType as 11 (Guildhall). It does not replace FIGHBL00.RMB to change the building type from Temple to Guildhall. Note: in BlocksFile.ReadRmbBlockData, a building replacement will cause the RMB's building type to be updated, but not the location's.

We have another mod, Fixed Desert Architecture (FDA). It also overrides location-0-165.json (to change the blocks to the new architecture), so it "conflicts" with UBLaMF on that file. However, it does not override FIGHBL00.RMB or FIGHBL00.RMB-677.building0.json. We tried two different things in two different versions, and both have an issue when it comes to mod compatibility.

In FDA 1.3, the FG building type is set to Guildhall in the location, like in UBLaMF. However, due to not providing the building override, the building data's building type depends on whether the user has UBLaMF active or not. If the user does not use UBLaMF, the building data's building type remains to the classic 14 (temple). If the user does use UBLaMF, then we have a matching 11 (guildhall).

In FDA 1.31 (sorry for Cliff's versioning, read this as 1.3.1), the FG building type is set to Temple in the location, like in classic. Again, without proving the building override, the building data's building type will depend on UBLaMF's presence. Therefore, if the user does have UBLaMF, we have a mismatch, due to the building replacement's building type as 11 "Guildhall".

The issue we're getting at is that in the case where the location's building type for a building mismatches the building data's building type, the entire location's guild halls break in the Map Discovery. The FG becomes a Mages Guild, the Mages Guild becomes nothing, and the temple becomes a Fighter's Guild. Sometimes it's the FG who becomes a Temple of Stendarr, and the temple becomes nothing.

To Reproduce

Steps to reproduce the behavior:

  1. Load a playthrough with either FDA 1.3 and no UBLaMF, or FDA 1.31 and UBLaMF
  2. Go to Alik'ra (Alik'r Desert)
  3. Check the discovery data of the FG, MG, and Temple buildings

Expected behavior

DFU should be more robust to such mismatches, or it should design how it interprets data in a way to prevent mismatches. When it comes to mod compatibility, a mod like FDA has to choose whether to apply the same changes as UBLaMF when it comes to location, block, or building overrides. UBLaMF is in a special position in terms of modding standards - mods generally want to duplicate the same fixes as UBLaMF when it comes to models and other minor fixes. The case in this issue stands out as a case where a mod cannot simply copy UBLaMF's location changes without also providing UBLaMF's building override, even if the mod does not otherwise touch that building.

We suspect that in this case, we have a case of DFU reading the building type from the building data. If it finds the building to be a guild hall, it will search for a guild hall in the location's "buildings" list. If the location has one guild hall but two buildings claiming claiming to be guild halls (FG and MG), one will win over and get that guild hall's name in the discovery data, and the other will get nothing. If the location has one temple but two buildings claiming to be temples (Fighter Trainers and Temple of Stendarr), then one will win over and get that temple's name in the discovery data, and the other will get nothing.

This is a difficult problem. I see a few notable facts:

Point 3 is where I'm thinking we have a problem, because modders will not be able to coordinate such efforts the more the community scales.

DFU should be able to either make the location's building type match the building data's building type, or it should not rely on the location's building type for discovery data. This way, no matter what the location replacement specifies, we take the building replacement as the "source of truth".

Screenshots and Logs

image

image (no info is provided despite clicking while in Info mode)

image (no discovery on the actual temple of stendarr, despite having been clicked on)

Conclusion

This is a big mess, and I don't expect a quick solution. For this specific case, Cliffworms can provide a building override that matches UBLaMF, and the mod will now work whether the user has UBLaMF or not. This issue is about the future, and the new conflicts that will emerge, and the pains modders will face identifying those issues.

I'm hoping a DFU solution will be better than whatever modders will have to do to prevent those issues.

Investigation

I suspect that the offending code is RMBLayout.GetLocationBuildingData. This is where the location's buildings are made into a "named building pool", and assigned from RMB building to pool item based on building type match, in order. If the building types mismatch between the pool and the actual contents of the RMBs, the links will be invalid.

KABoissonneault commented 9 months ago

To make the point about the conclusion, here's another mod that touches this area: Soldier's Luxury Overhaul. It overrides FIGHBL00.RMB-677-building0.json with 14 (Temple) as the building type. Soldier's Luxury wants to change the building only, but this will break if users have UBLaMF, because UBLaMF changes all the locations with this building to Guildhall. In order to fix the problem, SL has to provide a location override for all locations using this building. Otherwise, the mod has to warn users to not use UBLaMF. Both solutions are disappointing.

Moving forward, this will split mods into UBLaMF-required and UBLaMF-forbidden categories. And we risk finding new ways to create categories further down the line.

XJDHDR commented 9 months ago

I actually discovered this same problem a bit over 3 years ago while converting some of my mod's Map Blocks to use Building Overrides: https://forums.dfworkshop.net/viewtopic.php?t=4606 Hazelnut's reply was pretty much that this is just the way it works.

I think the first step towards solving this is figuring out if DFU is currently mapping between a buiding's location and block data the same way as vanilla? If this is not the case, would doing so improve things? Three years ago, I also attempted to determine the order that a location's buildings appear in relative to their placement in the location's blocks. I gave up in the end after I failed to figure it out.

KABoissonneault commented 9 months ago

Thanks for the info. I do think this issue will be complicated and require further investigation before we tackle this. In the meantime, I will recommend that every mod doing WorldData replacements does so using UBLaMF, to minimize compatibility issues, and ensure that basic fixes are properly applied to all mods.