Fix for intended precedence of building overrides when RMB block overrides also present #2714
Conversation
…rides also present If an RMB block was overridden, the block reading was shortcut and building overrides never applied correctly. This addition applies any applicable building overrides after loading the RMB override data.
|
Wait on cara to test this, to know if it properly addresses his issue |
|
I did lots of testing and by and large this PR successfully does everything it's supposed to do. However, it still needs fixes for two issues: building index outside the bounds of the RMB array and misplaced buildings. Building index outside the bounds of the RMB array Interestingly, when this occurs it breaks not only the DFLocation data of the affected village but that of all currently loaded locations in the map pixels surrounding the village. No buildings can be entered, and all buildings return an error (with World Tooltips) consistent with not finding any building data in the location: As we discussed on Discord, my recommendation for this edge case is to simply check if the building file's index is outside the bounds of the modded RMB's BuildingDataList array and, if so, skip the replacement. Misplaced buildings With Beautiful Cities only: With Beautiful Cities + Lively Cities: I am almost certain this is because replaced buildings are taking their XPos, YPos, ZPos, and YRotation from the building file instead of from the modded RMB. So for example, this particular building file (TVRNAL01.RMB-559-building15.json in Lively Cities) begins: This building has the following position in Beautiful Cities' TVRNAL01.RMB: Because RMBs (sometimes dramatically) change block layouts, the RMBs should always supply the building's position (ie, XPos, YPos, ZPos, and YRotation) while the building file replaces everything else. This will allow building files to be compatible with any RMB replacer, which is the main goal of this PR. (Please note that modded buildings often have YPos, not just XPos and ZPos, as they sometimes are placed on hills) |
|
I generally view these issues as mod incompatibilities, but I'll add some
protection to avoid the exception as you suggested.
Regarding the position of buildings, like everything else the building file
takes precedence if it's present. If this is altered then building
replacements will be unable to move the building. I could look at making a
special case if the RMB is overridden, but this does add complexity that
will need explaining somewhere.
…On Sat, 4 Jan 2025 at 00:33, drcarademono ***@***.***> wrote:
I did lots of testing and by and large this PR successfully does
everything it's supposed to do. However, it still needs fixes for two
issues: *building index outside the bounds of the RMB array* and *misplaced
buildings*.
*Building index outside the bounds of the RMB array*
This is an edge case we discussed on Discord earlier: a modded RMB can
sometimes have fewer buildings in its buildings array than a building
file's index. This issue crops up when Beautiful Villages and Lively Cities
are combined, since Beautiful Villages removes lots of buildings (to make
villages seem more rural) and Lively Cities then tries to modify buildings
that are missing. The result is this error:
DaggerfallWorkshop.Utility.AssetInjection.WorldDataReplacement.ReplaceRmbBlockBuildingData (System.String blockName, System.Int32 blockIndex, DaggerfallConnect.DFBlock& dfBlock) (at Assets/Scripts/Utility/AssetInjection/WorldDataReplacement.cs:408)
DaggerfallWorkshop.Utility.AssetInjection.WorldDataReplacement.GetDFBlockReplacementData (System.Int32 block, System.String blockName, DaggerfallConnect.DFBlock& dfBlock) (at Assets/Scripts/Utility/AssetInjection/WorldDataReplacement.cs:381)
DaggerfallConnect.Arena2.BlocksFile.GetBlock (System.Int32 block) (at Assets/Scripts/API/BlocksFile.cs:385)
DaggerfallConnect.Arena2.BlocksFile.GetBlock (System.String name) (at Assets/Scripts/API/BlocksFile.cs:416)
DaggerfallWorkshop.Utility.ContentReader.GetBlock (System.String name, DaggerfallConnect.DFBlock& blockOut) (at Assets/Scripts/Utility/ContentReader.cs:116)
DaggerfallWorkshop.TerrainHelper.SetLocationTiles (DaggerfallWorkshop.MapPixelData& mapPixel) (at Assets/Scripts/Terrain/TerrainHelper.cs:149)
DaggerfallWorkshop.DaggerfallTerrain.BeginMapPixelDataUpdate (DaggerfallWorkshop.ITerrainTexturing terrainTexturing) (at Assets/Scripts/Terrain/DaggerfallTerrain.cs:180)
DaggerfallWorkshop.StreamingWorld.UpdateTerrainData (DaggerfallWorkshop.StreamingWorld+TerrainDesc terrainDesc) (at Assets/Scripts/Terrain/StreamingWorld.cs:1216)
DaggerfallWorkshop.StreamingWorld+<UpdateTerrains>d__79.MoveNext () (at Assets/Scripts/Terrain/StreamingWorld.cs:662)
UnityEngine.SetupCoroutine.InvokeMoveNext (System.Collections.IEnumerator enumerator, System.IntPtr returnValueAddress) (at /home/bokken/buildslave/unity/build/Runtime/Export/Scripting/Coroutines.cs:17)
UnityEngine.MonoBehaviour:StartCoroutine(IEnumerator)
DaggerfallWorkshop.StreamingWorld:Update() (at Assets/Scripts/Terrain/StreamingWorld.cs:253)```
Interestingly, when this occurs it breaks not only the DFLocation data of the affected village but that of all currently loaded locations in the map pixels surrounding the village. No buildings can be entered, and all buildings return an error (with World Tooltips) consistent with not finding any building data in the location:
As we discussed on Discord, my recommendation for this edge case is to simply check if the building file's index is outside the bounds of the modded RMB's BuildingDataList array and, if so, skip the replacement.
**Misplaced buildings**
I also noticed that some buildings with building replacements are misplaced. Here's an example of one (from a TVRNAL01.RMB block in the north of Gothcroft in Northmoor) where the house moves positions depending on whether or not Lively Cities is enabled.
With Beautiful Cities only:
With Beautiful Cities + Lively Cities:
I am almost certain this is because replaced buildings are taking their XPos, YPos, ZPos, and YRotation from the building file instead of from the modded RMB. So for example, this particular building file (TVRNAL01.RMB-559-building15.json in Lively Cities) begins:
```{
"FactionId": 0,
"BuildingType": 18,
"Quality": 5,
"NameSeed": 0,
"RmbSubRecord": {
"XPos": 1280,
"ZPos": 2176,
"YRotation": 1536,
"Exterior": {```
This building has the following position in Beautiful Cities' TVRNAL01.RMB:
``` "XPos": 1280,
"ZPos": 2432,
"YRotation": -512,```
Because RMBs (sometimes dramatically) change block layouts, the RMBs should always supply the building's position (ie, XPos, YPos, ZPos, and YRotation) while the building file replaces everything else. This will allow building files to be compatible with any RMB replacer, which is the consistent with the goal of this PR.
(Please note that modded buildings often have YPos, not just XPos and ZPos, as they sometimes are placed on hills)
—
Reply to this email directly, view it on GitHub
<#2714 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AARMLLWLL7DHSVLO4EJLL7D2I4T6TAVCNFSM6AAAAABUDOC6RKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNRZHE3DINJWGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Making a special case if the RMB is overridden seems like a great "best of both worlds" solution here. Building files will still be able to change the building's position in a vanilla RMB, and RMB files won't have their layouts overridden by building files. That makes different types of mods as compatible with each other as possible. |
…g building coordinates and rotation as specified by the override RMB This allows RMB overrides to move buildings that are overridden by other mods.
|
@drcarademono I think I've corrected both your reported issues... over to you to test so we can get this merged. The first was caused by my using a count from the original data which we don't maintain in the overrides (we could, but my docs said no need to) when the actual array length can be easily used. So no errors should occur now. The second I have made it so building coords and rotation is not replaced for an overidden RMB when a building is replaced. |
|
I tested it and can confirm that it fixes both issues discussed above. I didn't see any new issues. Everything appears to work as intended. Terrific job, Hazel! |
When an RMB block is overridden, the block reading is shortcut and building overrides should be applied correctly. This was not happening, and this addition applies any applicable building overrides after loading the RMB override data.
This PR replaces #2699 submitted by carademono. That PR was a very good effort but it would only fixe it for a specific scenario and also breaks the block construction encapsulation.
This should be rigorously tested that it doesn't break any existing mods.
One issue that has never been addressed is the automap data which is at the RMB block level but can be overridden by a building override. I intend to take a look at a way to have building overrides map data to be merged.