Merging Mods, Collaborating

[Interkarma]

Let me know if you'd prefer me to make that small change after accepting pull request.

of course I am happy with everything that works and that makes our life easier

That line of code is basically injecting your terrain sampler whether addon is supposed to be active or not.

Yeah I had this already before, but commented it out while fixing all the problems - so this is easy to do again

For now, the quickest way to solve this is simply to check DaggerfallUnity.Settings.Nystul_* flags during Awake() and Start(). My game startup code can then disable addon objects in the scene if user has indicated they should not be active.

Where can one define the values of these DaggerfallUnity.Settings.Nystul_* flags? - I ask because I would like to test if it is working

Cheers mate. I'll just accept the pull request as-is and we can tidy up those minor items afterwards.

DaggerfallUnity.Settings are the config options exposed by INI file. Just do something like:

Code: Select all

if (DaggerfallUnity.Settings.Nystul_IncreasedTerrainDistance)
{
    // Do setup here
}

In editor mode the INI used is Resources/fallback.ini. During play it's Application.dataPath/settings.ini. If a setting cannot be read from settings.ini then fallback.ini will be used instead. You don't need to worry about that however, the SettingsManager handles all the logic for INI file options.

[Interkarma]

DaggerfallUnity.Settings are the config options exposed by INI file. Just do something like:

Code: Select all

if (DaggerfallUnity.Settings.Nystul_IncreasedTerrainDistance)
{
    // Do setup here
}

Something that just occurred to me is that you might want the mod to be operational outside of Daggerfall Unity (say for other DFTFU projects). Maybe a better idea would a toggle for increased terrain like the one Lypyl has on enhanced sky. That way it can be turned on and off as needed by whatever project uses it. What do you think?

[Nystul]

edited my post meanwhile you were answering sry for any confusion that caused

Something that just occurred to me is that you might want the mod to be operational outside of Daggerfall Unity (say for other DFTFU projects). Maybe a better idea would a toggle for increased terrain like the one Lypyl has on enhanced sky. That way it can be turned on and off as needed by whatever project uses it. What do you think?

hmm I will think about this. I doubt I will manage to get my mods to a state were they are toggle-able while running. but I could think of a wrapper that enables/disables the gameobject with the main script. DFTFU compatibility would just mean that no wrapper is present. Daggerfall-Unity will use the wrapper which basically would enable/disable the gameobject dependent on the boolean flags in the Settings

[Interkarma]

one more question from my side:
Both my mods use external files, all files are in a corresponding Resources folder inside the mod subfolder. png-files are loaded with Resources.Load which seems to load the files always correctly.
But there are other files where Resources.Load didn't work (e.g. the ini file for the texture injection config):
I am not sure if the relative filepath is ok in terms of building the final executable - I use these relative paths:
ReflectionsMod:

Code: Select all

const string filepathConfigInjectionTextures = "Assets/daggerfall-unity/Game/Addons/ReflectionsMod/Resources
/configInjectionTextures.ini";

IncreasedTerrainDistanceMod:

Code: Select all

        const string filepathInTreeCoverageMap = "./Assets/daggerfall-unity/Game/Addons/IncreasedTerrainDistanceMod/Resources/mapTreeCoverage_in.bin";

        const string filepathOutTreeCoverageMap = "./Assets/daggerfall-unity/Game/Addons/IncreasedTerrainDistanceMod/Resources/mapTreeCoverage_out.bin";

        const string filepathMapLocationRangeX = "./Assets/daggerfall-unity/Game/Addons/IncreasedTerrainDistanceMod/Resources/mapLocationRangeX.bin";
        const string filepathMapLocationRangeY = "./Assets/daggerfall-unity/Game/Addons/IncreasedTerrainDistanceMod/Resources/mapLocationRangeY.bin";

just want to find out if this will be a problem

The way to think about Resources is that all disparate Resources folders are combined together at runtime/build. So Resources.Load() will find files in any Resources folder no matter where it is in the Project hierarchy because ultimately they all get merged into the one and only Resources folder.

But there are other files where Resources.Load didn't work (e.g. the ini file for the texture injection config):

This will work, but don't forget your INI file should be named something like: mysettings.ini.txt before dragging it into Resources. The .txt on the end is important for Unity to understand this is a file it needs to load in a certain way (it doesn't understand .ini by itself). Then you can use Resources.Load() for the INI just fine. Check out how I do this in SettingsManager.cs for fallback.ini and settings.ini. This provides an example of working with both Resources and file-system from a relative path.

Using relative paths is a little trickier. The way to go about this is use the platform-agnostic Application.dataPath as root and System.IO.Path.Combine these to the final path. Don't use explicit strings into Assets like above. The difficulty here is that each platform has slightly different targets for dataPath, and sometimes this requires a rethink on why the data in a relative path in the first place. You also need to remember to drop the target files into the build folder later.

You should be able to use Resources for everything here, and it's preferred unless you absolutely must distribute to the player in a raw form (e.g. the settings.ini and the KeyBinds.txt files).

btw, there is a shader warning I am not able to get rid of easily:
Shader warning in 'Daggerfall/IncreasedTerrainTilemap': gradient-based operations must be moved out of flow control to prevent divergence. Performance may improve by using a non-gradient operation at line 163 (on d3d11)
it is caused by a tex2Dgrad command that is sampling from the different atlas textures inside if statements dependent on the climate for a tile. I do not agree with the warning though that moving it out of the if statement will improve performance since I would have to sample all the different atlas textures anyway
I will rethink the strategy if it is important to change it
If it is ok I will leave it unchanged for the moment

Sure, leave it as-is for now.

[Interkarma]

edited my post meanwhile you were answering sry for any confusion that caused

Something that just occurred to me is that you might want the mod to be operational outside of Daggerfall Unity (say for other DFTFU projects). Maybe a better idea would a toggle for increased terrain like the one Lypyl has on enhanced sky. That way it can be turned on and off as needed by whatever project uses it. What do you think?

hmm I will think about this. I doubt I will manage to get my mods to a state were they are toggle-able while running. but I could think of a wrapper that enables/disables the gameobject with the main script. DFTFU compatibility would just mean that no wrapper is present. Daggerfall-Unity will use the wrapper which basically would enable/disable the gameobject dependent on the boolean flags in the Settings

That sounds fine to me. Can always improve over time.

I've merged your addon now. Will start testing and let you know if anything jumps out.

[Interkarma]

Code: Select all

const string filepathInTreeCoverageMap = "./Assets/daggerfall-unity/Game/Addons/IncreasedTerrainDistanceMod/Resources/mapTreeCoverage_in.bin";

This will need to be fixed. It fails whenever "./Assets/daggerfall-unity" is not the relative path (as is the case for my project). I will change these to use Resources.Load() and we can go from there.

Edit: I've changed the mapLocationRangeX and mapLocationRangeY to load from Resources.Load() rather than a path. Here are the changes to make this work.

1. Rename files from mapLocationRangeX.bin to mapLocationRangeX.bin.txt and reimport to project.
2. Create a MemoryStream instead of FileStream and the rest is the same. Check commit for the deets.

I'll keep working through it.

Edit2: I'm not sure what to do with the filepathInTreeCoverageMap and filepathOutTreeCoverageMap just yet. I'll leave these for you to look at, and change to Resources.Load if possible.

Edit3: I can also trigger an exception for the below

Code: Select all

stackedNearCamera.clearFlags = CameraClearFlags.Depth;

This happens because the stackedNearCamera is null. The event TransitionToExterior() can be triggered before Setup() is run, which means that SetUpCameras() will barf. To work around this, I've moved all the setup to its own method SetupGameObjects() which can be called when needed as you're doing the null checks when creating the objects. There are other ways to manage this as well, check out the property getters in GameManager.cs for examples of a properties with a create-then-cache pattern.

Edit4: The stackedNearCamera depth conflicts with the SkyRig camera for default sky (both end up 2 as SkyRig sets itself to MainCamera-1.). I will fix this up now in my code and adjust increased terrain to use a depth value set from Inspector rather than shifting around main camera on me at startup. Update: Now the camera depths can be set at startup rather than from code, I can use classic sky behind the far terrain. Now we need to adjust things like fog distance on the fly which are drastically different between stock and far terrain.

[Interkarma]

So most of the integration problems have been fixed and checked in now. I can use the increased terrain with the default sky rig no problems, and everything looks great!

Great work on this mod, you've brought this so far in the last few months.

[Nystul]

great!
have you had time to test the ReflectionsMod as well?

[Nystul]

Code: Select all

[b]Edit4:[/b] The stackedNearCamera depth conflicts with the SkyRig camera for default sky (both end up 2 as SkyRig sets itself to  MainCamera-1.). I will fix this up now in my code and adjust increased terrain to use a depth value set from Inspector rather than shifting around main camera on me at startup. Update: Now the camera depths can be set at startup rather than from code, I can use classic sky behind the far terrain. Now we need to adjust things like fog distance on the fly which are drastically different between stock and far terrain. :)[/quote]

the camera's depth values must be in this order from higher to lower values:

main Camera from PlayerAdvanced
stackedNearCamera
stackedCamera
SkyCam (from Lypyl's Enhanced Sky mod)
stackedCameraSkyboxRenderToTextureGeneric

since Lypyl's SkyCam had default depth value of -2 (before mods were integrated into the git repository, I didn't dare to modify Lypyl's EnhancedSky mod - so a adjusted all other cameras to fit in - that's why I had to change main camera's depth value as well), I decided to set the main camera's depth to a value higher than 0 and the stacked camera's depth in between.
Now since we have all the mods in git it there is a better way:
suggestion: SkyCam depth value set to -3, the stacked cameras to -2 and -1 and main camera at depth 0
I like your changes to allow to specify depth as parameter - should have done that before ;)

btw. I am uncomfortable with the stackedNearCamera:
I introduced it because I realized floating point precision issues (e.b. going up a large hill and looking down on locations caused flickering and positioning artifacts for these locations).
Unfortunately it introduces issues with globalfog script and fog (set fog to linear ranging from 0 to 2000 and you will see immediately) - unity forum post on this issue exist but the solutions involve copying the depth buffer between cameras in a kind of complicated way (if I understood it right).
Not sure if it is a worth the extra troubles

[Interkarma]

great!
have you had time to test the ReflectionsMod as well?

Not yet. I ran out of time yesterday with all the prep to go on leave. Currently up at 2am to catch a stupidly early flight ahead. Will totally get stuck into this when I get back next week though.

since Lypyl's SkyCam had default depth value of -2 (before mods were integrated into the git repository, I didn't dare to modify Lypyl's EnhancedSky mod - so a adjusted all other cameras to fit in - that's why I had to change main camera's depth value as well), I decided to set the main camera's depth to a value higher than 0 and the stacked camera's depth in between.
Now since we have all the mods in git it there is a better way:
suggestion: SkyCam depth value set to -3, the stacked cameras to -2 and -1 and main camera at depth 0
I like your changes to allow to specify depth as parameter - should have done that before

btw. I am uncomfortable with the stackedNearCamera:
I introduced it because I realized floating point precision issues (e.b. going up a large hill and looking down on locations caused flickering and positioning artifacts for these locations).
Unfortunately it introduces issues with globalfog script and fog (set fog to linear ranging from 0 to 2000 and you will see immediately) - unity forum post on this issue exist but the solutions involve copying the depth buffer between cameras in a kind of complicated way (if I understood it right).
Not sure if it is a worth the extra troubles

I noticed that with the fog and see how you worked around it for now, which is perfectly good. However else you feel about the camera setup, I think this is the best it has ever "felt" to me. The terrains feel completely part of the one whole now.

BTW, I forgot to mention that I commented out the line which reduces the main camera draw distance to 1200. I can see why you had to do this, and I'll put this back in a slightly different form shortly. When I get a bit more time, I'd love to help work on smoothing the two terrains together even more. I'm really excited for the future of this project with people like yourself building these amazing addons.