Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make building replacements compatible with RMB replacements #2699

Closed

Conversation

drcarademono
Copy link
Contributor

Currently modded building replacements and modded RMB replacements are incompatible. Whenever a mod tries to change a building in a modded RMB, the building's metadata is changed but the exterior and interior remains unchanged. This is an especially big problem for unmaintained building replacement mods like Tavern's Redone and Soldier's Luxury, which are incompatible with newer RMB replacement mods like Cities Overhauled and Beautiful Cities.

This PR fixes these incompatibilities by applying building replacements in RMBLayout, after applying RMB replacements. It modifies the method GetLocationBuildingData, in which building replacements change metadata, so that it also changes RMBSubRecords.

Everything appears to be working as intended upon testing. Buildings are replaced in modded RMBs exactly as they would be were the building replacement affecting a vanilla block. Both interior and exterior data (as with Lively Cities) works correctly. Factions (like Archaeologists Guild) work correctly and appear in blue on the automap. Named buildings like taverns still get their metadata from the location data after the building replacements are applied, so their nameseeds and quality etc are correct. This PR also appears to be robust to modded RMBs with many buildings deleted (ie Beautiful Villages); no NullReferences or Index out of range occurred.

@drcarademono
Copy link
Contributor Author

I have some growing concerns about my PR here. First I'm worried that it might not be well-structured (I'm hardly a coding wunderkind). Second, having spent a lot more time with both RMB and building replacements, I think it's important to incorporate mod load order into the replacement logic. Ie:

  • If a modded RMB is higher in the load order than a modded building file, the RMB should be applied and the building file should be ignored.
  • If a modded building file is higher in the load order than a modded RMB, then the RMB should be applied and then the building file should be applied (modifying the already modded RMB).

I have a vague idea of how to code this, but since this is at the edge of my capabilities and we're talking about a PR to DFU's core systems, I thought it would be better to defer to experts.

@ajrb
Copy link
Collaborator

ajrb commented Dec 22, 2024

That is not the way it's been written to work, load order only affects which one of 2 identical files will be used. Once the files are identified, then the world replacement mechanism should apply the RMB and then override any buildings in that RMB that have a building override file since they take precedance. (as they're more specific)

The intent here is to allow mods to be compatible as much as possible without merging files etc, which is a level of complexity that I don't think is worth the gains, and mod authors will need to cooperate if they want to achieve compatibility when using the same filenames.

I plan to take a look at the issue reported in this PR over the next 2 days, but I don't plan to have load order affecting the way different files are applied. I feel that would make mod compatability more complex for users to understand and the unintended consequence of mod order needing to be one way for the functionality and another for the world data. At the end of the day, there's a limit to how compatible mods can be if they're affecting the same parts of the world data or same parts of the code.

@drcarademono Does this make sense or do you still feel that we need it to work as you described 4 days ago? We might need to have a more interactive discussion on discord about this. After xmas I plan to be playing Stalker 2 so hopefully I can resolve this in the next couple of days.

@drcarademono
Copy link
Contributor Author

Thanks for your thoughtful response on this! I think what you wrote makes sense and withdraw the suggestion to incorporate mod load order.

Just thought of one edge case which I'm not sure is currently properly handled: what happens if there's a modded RMB with multiple buildings removed, and a modded building file whose index is higher than the remaining buildings? Ie, suppose someone mods RESIAM03.RMB to have 12 buildings instead of 16, and someone else mods RESIAM03.RMB-750-building14, which no longer exists?

I suggest simply skipping the building replacement in this case (since there's no building to replace).

Have a merry xmas / Stalker 2 marathon!

@ajrb
Copy link
Collaborator

ajrb commented Dec 23, 2024

I've investigated this and submitted #2714 to fix this issue in a way more in keeping with original design intent.

@ajrb ajrb closed this Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants