View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0003034||The Dark Mod||Coding||public||27.02.2012 13:00||16.06.2012 18:57|
|Product Version||TDM 1.08|
|Target Version||TDM 1.08||Fixed in Version||TDM 1.08|
|Summary||0003034: DoomConfig.cfg read/write locations|
|Description||The paths for reading and writing DoomConfig.cfg are no longer in sync after retiring tdmlauncher. The game reads the config from doom3/FMNAME/DoomConfig.cfg, but writes to doom3/darkmod/fms/FMNAME/DoomConfig.cfg, so any changes made to doom3/darkmod/fms/FMNAME/DoomConfig.cfg get overwritten when loading the TDM.|
Furthermore, if no FM is set, doom3/darkmod/fms/darkmod/DoomConfig.cfg is used instead of doom3/darkmod/DoomConfig.cfg.
http://forums.thedarkmod.com/topic/13314-getting-rid-of-tdmlauncher/page__view__findpost__p__279696 posts 48-51
|Tags||No tags attached.|
* Fix the config path to <basepath>/darkmod/ regardless of the currently installed FM.
* Rename DoomConfig.cfg to DarkModConfig.cfg (another step towards shedding Doom3)
* As an interim, read <basepath>/darkmod/DoomConfig.cfg on startup only if <basepath>/darkmod/DarkModConfig.cfg doesn't exist
--> DoomConfig.cfg will be ignored after the initial load as DarkModConfig.cfg would now exist
--> This might still cause issues with the render settings that don't port well between 1.07 and 1.08
Okay, so I have patch that addresses these items:
1.Fix the config path to <basepath>/darkmod/ regardless of the currently installed FM.
2.Rename DoomConfig.cfg to DarkModConfig.cfg (another step towards shedding Doom3)
I don't want to commit these changes to SVN just yet since it could surprise people. Patch can be found here: http://taaaki.za.net/configpath-bug3034.patch
Tels/anyone else interested: if you wouldn't mind testing the patch a bit to make sure I haven't broken anything (I had to modify one of the FileSystem functions).
I have not tackled point 3 yet (if we even want to do something like that). I think I have a solution, but I don't really know enough about the changes between 1.07 and 1.08 to know which cvars cause issues when loading a 1.07 config in 1.08. It is fairly easy to add a conditional to the game init code that will load DoomConfig.cfg if it does not find DarkModConfig.cfg, but this brings across the bad cvars. If we know exactly which cvars need to be reset, I can make it load an extra file with defaults for those cvars after it has loaded DoomConfig.cfg. The "fixed" config will then be saved as DarkModConfig.cfg and it should be smooth sailing from there.
|Have not tested it yet, but in related news, the write location of screenshots is also "fms/darkmod/screenshots/" when you have no mission installed. That sounds like a related issue.|
Yeah, I noticed that fms/darkmod was being added as a search path when I enabled filesystem debugging. That is actually related to 0002997 though, so I will tackle that when I'm happy with this change. Just haven't had time to look at it this week.
If you don't have a mission loaded, should it just put screenshots in <base>/darkmod/screenshots?
"If you don't have a mission loaded, should it just put screenshots in <base>/darkmod/screenshots?"
As with the config, having the screenshots stored with the current mission has the advantage that you get them sorted by mission - but it does have the disadvantage that if you need a screenshot, you'd need to remember under which mission you took that screenshot (whcih can be a pain if you try to load it in an externan program 2 weeks later...).
I am not sure if we shouldn't just put all screenshots under "<base>/darkmod/screenshots"?
I am asking greebo what he thinks.
Ok, here are some comments on your patch:
* DarkModConfig.cfg" should be just "Darkmod.cfg" (no need to repeat words, and no need for camelcase)
* you have a #define #define CONFIG_FILE, but there are still code places with a hardcoded "DoomConfig.cfg" in it - they should all just use the define unless they are of the "fallback" case (haven't checked them all :)
Could you please change these things and make a new patch?
All right, greebo has replied with basically "I think one directory for screenshots".
So storing them all under "<base>/darkmod/screenshots" would be the way to go.
I can make a new tracker entry for the screenshot issue if you want.
Another remark for your patch, gcc did not like this construct:
OSpath = BuildOSPath( path, gamedir ? gamedir : gameFolder, relativePath );
because "gamedir" is const char* and gameFolder is idStr. I am trying to fix that and will post what the correct syntax is.
I like the idea of centralising all the screenshots. We can probably prefix the filename with the name of the mission or something if people want to remember what mission the screenshot was from.
I have modified the config file name to "Darkmod.cfg"
On the second point, all the code where there was a hardcoded "DoomConfig.cfg" aren't compiled anymore as I placed them in a #if 0 ... #endif
I will outright remove that code as it is no longer necessary once we use a single config file location. I just wanted to keep it in while I tested so that I wouldn't have to dig into SVN to get the removed code. The patch I have uploaded doesn't remove that code just yet, but I have removed the hardcoded filename so that it won't come up in searches.
New patch (might need to revert the old patch): http://taaaki.za.net/darkmodcfg-bug3034.patch
Regarding the ternary statement, could you please post the gcc warning. In the meanwhile, I've changed the line to:
OSpath = BuildOSPath( path, gamedir ? gamedir : gameFolder.c_str(), relativePath );
I also thought about adding the fm name somehow into the screenshot file name. Probably needs any non "a-z" replaced by "_" (and made lowercase), but sounds like a sensible idea. However, it might be a bit complicated because the code that searches for the next free screenshot slot might get confused (maybe this code needs simply to be changed to "read all directory entries, then pick the one with the highest number" instead of doing a "blind" search.
ternary: I made the same modification locally and now it compiles, so this seems to be the proper way :)
|I created a new tracker entry for the screenshots (http://bugs.angua.at/view.php?id=3054), so any further comments regarding the screenshots should go there. I've also just copied over your last comment, so I don't forget to incorporate it into my changes.|
taaaki, I think it's best apply your patch and then proceed to sort out the issues with the locations.
One change I think we also might do is to try to avoid that D3 (TDM) loads files from "<base>/FMNAME" altogether.
For instance, if you have:
files in the latter can interfere (it might be an old config, an old DLL, or PK4s with old content). So if you are sorting this out, please see if you can find where it adds that path into the search path and disable it.
Committed my changes:
On the topic of searchpaths, I don't think we should go ahead with removing the fm from <basepath>/ just yet, as it might have an effect on mappers using DarkRadiant. See greebo's post here: http://forums.thedarkmod.com/topic/13314-getting-rid-of-tdmlauncher/page__view__findpost__p__274707
However, I do agree that we need to make sure that old DLLs etc don't get loaded from <basepath>/<fm>. This shouldn't be a problem for configs any longer, but <basepath>/<fm> appears ahead of <basepath>/darkmod in the searchpaths, so this is an issue. I can see two easy-ish ways to fix this:
1) Reverse the order so that files are loaded from <basepath>/darkmod before <basepath>/<fm> (I don't know if there are any use-cases for custom game dlls?)
2) Make the loading of files from <basepath>/<fm> conditional on the developer Cvar or something to that effect.
In the meanwhile, I have taken "darkmod" out of the <basepath>/darkmod/fms/ searchpath (there shouldn't be any need for it) and changed some hardcoded directory names to use the appropriate defines and cvars.
|Should be resolved in Revision 5479.|
|27.02.2012 13:00||taaaki||New Issue|
|27.02.2012 13:00||taaaki||Status||new => assigned|
|27.02.2012 13:00||taaaki||Assigned To||=> taaaki|
|29.02.2012 15:33||taaaki||Note Added: 0004361|
|29.02.2012 18:00||taaaki||Note Added: 0004362|
|03.03.2012 12:01||tels||Relationship added||related to 0001975|
|06.03.2012 22:51||tels||Note Added: 0004377|
|07.03.2012 04:05||taaaki||Note Added: 0004378|
|10.03.2012 13:54||tels||Note Added: 0004383|
|10.03.2012 13:59||tels||Note Added: 0004384|
|10.03.2012 16:09||tels||Relationship added||related to 0002997|
|10.03.2012 16:12||tels||Note Added: 0004385|
|10.03.2012 16:50||taaaki||Note Added: 0004386|
|10.03.2012 17:20||tels||Note Added: 0004387|
|10.03.2012 19:28||taaaki||Relationship added||related to 0003054|
|10.03.2012 19:33||taaaki||Note Added: 0004390|
|17.03.2012 12:49||tels||Note Added: 0004411|
|18.03.2012 11:26||taaaki||Note Added: 0004413|
|18.03.2012 11:37||taaaki||Note Added: 0004414|
|16.06.2012 18:57||taaaki||Note Added: 0004663|
|16.06.2012 18:57||taaaki||Status||assigned => resolved|
|16.06.2012 18:57||taaaki||Fixed in Version||=> TDM 1.08|
|16.06.2012 18:57||taaaki||Resolution||open => fixed|