View Issue Details

IDProjectCategoryView StatusLast Update
0003034The Dark ModCodingpublic16.06.2012 18:57
Reportertaaaki Assigned Totaaaki  
PrioritynormalSeveritynormalReproducibilityalways
Status resolvedResolutionfixed 
Product VersionTDM 1.08 
Target VersionTDM 1.08Fixed in VersionTDM 1.08 
Summary0003034: DoomConfig.cfg read/write locations
DescriptionThe 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.

See: http://forums.thedarkmod.com/topic/13314-getting-rid-of-tdmlauncher/page__view__findpost__p__275988
http://forums.thedarkmod.com/topic/13314-getting-rid-of-tdmlauncher/page__view__findpost__p__279696 posts 48-51
TagsNo tags attached.

Relationships

related to 0001975 closedtels [LINUX] - DoomConfig.cfg synchronization does not fully work. 
related to 0002997 resolvedtaaaki SVN TDM (1.08) not saving games properly 
related to 0003054 resolvedtaaaki Save screenshots to a central location 

Activities

taaaki

taaaki

29.02.2012 15:33

administrator   ~0004361

Proposed changes

* 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
taaaki

taaaki

29.02.2012 18:00

administrator   ~0004362

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.
tels

tels

06.03.2012 22:51

reporter   ~0004377

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.
taaaki

taaaki

07.03.2012 04:05

administrator   ~0004378

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?
tels

tels

10.03.2012 13:54

reporter   ~0004383

"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.
tels

tels

10.03.2012 13:59

reporter   ~0004384

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?
tels

tels

10.03.2012 16:12

reporter   ~0004385

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.
taaaki

taaaki

10.03.2012 16:50

administrator   ~0004386

Thanks Tels,

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 );
tels

tels

10.03.2012 17:20

reporter   ~0004387

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 :)
taaaki

taaaki

10.03.2012 19:33

administrator   ~0004390

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.
tels

tels

17.03.2012 12:49

reporter   ~0004411

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:

doom3/darkmod/fms/thieves

and

doom3/thieves

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.
taaaki

taaaki

18.03.2012 11:26

administrator   ~0004413

Committed my changes:

Revision 5342:
 Common.h
 Common.cpp
 FileSystem.cpp
 FileSystem.h
 Licensee.h
 SysCvar.cpp
 SysCvar.h
 Game_local.cpp
 MissionManager.cpp
taaaki

taaaki

18.03.2012 11:37

administrator   ~0004414

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.
taaaki

taaaki

16.06.2012 18:57

administrator   ~0004663

Should be resolved in Revision 5479.

Issue History

Date Modified Username Field Change
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