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

RuntimeMaterial: Take RandomDungeonTextures setting into account #2026

Merged
merged 5 commits into from
Mar 12, 2021
Merged

RuntimeMaterial: Take RandomDungeonTextures setting into account #2026

merged 5 commits into from
Mar 12, 2021

Conversation

XJDHDR
Copy link
Contributor

@XJDHDR XJDHDR commented Feb 9, 2021

RuntimeMaterials previously didn't take the game's RandomDungeonTextures setting into account when it decided to apply climate variations to dungeon models. This meant that dungeon models using RuntimeMaterials looked noticeably different next to vanilla models. This PR integrates the same logic that DaggerfallDungeon uses to decide what textures vanilla models use.

The texture assignment abilities of RuntimeMaterials now almost matches that of the vanilla logic. The only thing I still need to sort out is that when the RandomDungeonTextures setting was set to "Random", I found a dungeon (Castle Buckington - Ilessan Hills in this case) where the vanilla models were using textures from the "Swamp" climate set whereas RuntimeMaterials used the "Temperate" set. Edit: This no longer applies.

In particular, I would like @TheLacus to take a look at this since RuntimeMaterials was his work.

If merged, this PR will mostly fix the third problem described in issue #1990.

RuntimeMaterials - Temperate textures:
RunMat - Temperate climate
Vanilla model - Swamp textures:
DagMesh - Swamp climate

@rossipaolo rossipaolo self-assigned this Feb 9, 2021
@rossipaolo
Copy link
Collaborator

I don't know how RandomDungeonTextures option works, i'll investigate and then test your addition.

This change breaks assignment via public API, but i can fix it during merge. Please tell me when you've finished working on it and i can review and push to master.

@XJDHDR
Copy link
Contributor Author

XJDHDR commented Feb 10, 2021

I don't know how RandomDungeonTextures option works, i'll investigate and then test your addition.

From what I can see, it's used in the UseLocationDungeonTextureTable function on line 173 of DaggerfallDungeon.

This change breaks assignment via public API, but i can fix it during merge.

I didn't have a solution at the time for the compile error and didn't know that function was being used. No worries though, I've fixed it myself.

Please tell me when you've finished working on it

Pretty much just trying to track down what the game does when set to use Random textures and how to integrate it into RM. From what I can see, the RandomTextureTableAlternate function in DungeonTextureTables is used to construct a texture table by picking five completely random archives from the entire set.

@XJDHDR
Copy link
Contributor Author

XJDHDR commented Feb 11, 2021

@TheLacus
I've finished working on the PR so you can review it whenever you like. Just one caveat though is that this PR as it stands will not work without the changes in PR #2028.

@Interkarma
Copy link
Owner

I'll leave this one to @TheLacus to review as RuntimeMaterials are his. But if I'm needed for anything, please don't hesitate to ping me and I'll do what I can to help.

@rossipaolo rossipaolo self-requested a review March 11, 2021 18:10
@rossipaolo
Copy link
Collaborator

rossipaolo commented Mar 11, 2021

I've finished working on the PR so you can review it whenever you like. Just one caveat though is that this PR as it stands will not work without the changes in PR #2028.

Thank you for working on this! I changed username, apologise for any possible confusion.

RuntimeMaterials.ApplyMaterials() receives dungeonTextureTable parameter from event (as part of automatic setup inside dungeon) or from public method (which is another way to setup material if gameobject is instantiated by mod after dungeon is created). Your previous approach of using dungeon received by event was fine, just need a fallback when texure table is set by public method.

I'd suggest to change method signature to private void ApplyMaterials(bool force, int[] dungeonTextureTable = null, int dungeonID = -1) and pass dungeon id from event. If set by pubic method (i.e. id is -1), retrieve dungeon id from PlayerEnterExit (we can assume it's available at that point). Just use a good fail-safe (i.e. climate = ClimateBases.Desert; season = ClimateSeason.Summer;) if property is null for some reason.

RuntimeMaterials previously didn't take the game's RandomDungeonTextures setting into account when it decided to apply climate variations to dungeon models. This meant that dungeon models using RuntimeMaterials looked noticeably different next to vanilla models. This commit integrates the same logic that DaggerfallDungeon uses to decide what textures vanilla models use. The texture assignment abilities of RuntimeMaterials now almost match that of the vanilla logic.
This commit completes my work in making RuntimeMaterials compatible with DFU's RandomDungeonTextures setting by adding support for it's "Random" and "RandomOnly" options.
@XJDHDR
Copy link
Contributor Author

XJDHDR commented Mar 12, 2021

@rossipaolo
No worries, I noticed the change a while ago.

I've made the changes you requested.

XJDHDR and others added 3 commits March 12, 2021 18:42
Added a way for the OnSetDungeon event to pass the Texture Table and Dungeon ID to Runtime Materials rather than having each one retrieve it from DaggerfallDungeon every time.
@rossipaolo
Copy link
Collaborator

No need to change event signature, i only said to get dungeon id directly from event. In any case thank you for improvements, i'll merge to a branch for further changes if needed.

@rossipaolo rossipaolo changed the base branch from master to runtime-materials March 12, 2021 20:43
@rossipaolo rossipaolo merged commit 73f14ae into Interkarma:runtime-materials Mar 12, 2021
@rossipaolo
Copy link
Collaborator

After further review of how dungeon texture tables work, i believe they already contain any information about climate and don't need additional climate step with ClimateSwaps.ApplyClimate(). @Interkarma can you confirm this so we can be sure?

I tested your mod with #2062 and i couldn't replicate any issue. If you also confirm that unsetting ApplyClimate solve the issue for you, i'll merge that simpler fix instead of this one. Otherways, please provide save and texture settings to replicate issue.

@XJDHDR
Copy link
Contributor Author

XJDHDR commented Mar 13, 2021

Just tried disabling the ApplyClimate setting on my models and things seem to be working as they should. The models still receive their climate specific textures when textures are set to Climate.

@Interkarma
Copy link
Owner

@Interkarma can you confirm this so we can be sure?

I'll need to confirm to be sure (it has been a few years now since I last looked at this) but my memory is that dungeon texture tables work on a different process to overland textures. I don't believe they need to run ApplyClimate() and removing this call is OK.

@rossipaolo
Copy link
Collaborator

Thank you both for your take, i'll merge that PR then.

@rossipaolo rossipaolo linked an issue Mar 16, 2021 that may be closed by this pull request
@XJDHDR XJDHDR deleted the runtime-materials-dungeon-climate-fix branch March 17, 2021 22:02
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.

3 participants