Jump to content
The Dark Mod Forums

Itches, Glitches & Anything Else...


Recommended Posts

I renamed the qerplugin.h to iradiant.h to be consistent with all the other module includes. This affected the dm.objectives folder as well, but SVN should be able to merge the change into your working version, in case you wonder what I did in your plugin. :)

Link to comment
Share on other sites

You're right, there's a horrible global CopiedString variable in that function. I suggest replacing all of the getXXXPath() functions on GlobalRadiant with a single getPath() method, which takes a single string argument detailing which path is required (e.g. "application", "bitmaps", "maps" etc). This would slim down the already-bloated IRadiant interface and allow additional paths to be added without changing the interface.

Would it be an alternative to store the relevant paths into the Registry during startup?

 

Or something property-based like GTK does for objects?

GlobalRadiant().getProperty("appPath");
GlobalRadiant().setProperty("appPath", "/usr/local/blah");

Link to comment
Share on other sites

I renamed the qerplugin.h to iradiant.h to be consistent with all the other module includes. This affected the dm.objectives folder as well, but SVN should be able to merge the change into your working version, in case you wonder what I did in your plugin. :)

 

Good, that needed doing as well. I have removed some cruft already from that interface, but it still needs further cleanup. The thing that jumps out at me is that there are four methods to just support the Clipper plugin (and are not used by anything else) and that iradiant.h includes iclipper.h as well -- this suggests that the Clipper is not/cannot be completely modularised and probably shouldn't be in a separate module. I am not sure that it makes sense for the Clipper to be a module anyway, because it is a core GUI tool which does not have any obvious "module boundaries", and doesn't meet the two tests for modularisation (neither used by any external modules, or varies from game to game).

 

Would it be an alternative to store the relevant paths into the Registry during startup?

 

That sounds fine, actually.

 

Or something property-based like GTK does for objects?

 

No, we shouldn't have two property systems, this would get confusing.

Link to comment
Share on other sites

Ok, I'll move the path information to the Registry then. This will save us a few CopiedStrings as well. :)

 

Another thing: I've locally upgraded the Win32 GTK2+ libraries to the latest version 2.10.6. Not because of pressing issues, but there appear to be some new gtk*.h header files added, and I'd like to have them ready if I need them in the future.

 

Should I...

...commit them to a new branch, since there may be compiling issues?

...commit it to the trunk right away?

...don't do such stuff, because it works in the current state? (I can of course revert everything)

Link to comment
Share on other sites

Another thing: I've locally upgraded the Win32 GTK2+ libraries to the latest version 2.10.6. Not because of pressing issues, but there appear to be some new gtk*.h header files added, and I'd like to have them ready if I need them in the future.

 

Should I...

...commit them to a new branch, since there may be compiling issues?

...commit it to the trunk right away?

...don't do such stuff, because it works in the current state? (I can of course revert everything)

 

I would put this into a separate branch for now, and then merge it back once any compiling, running or installation issues are ironed out.

Link to comment
Share on other sites

Ok, a separate branch it is then. It was relatively painless to setup, I just had to put in the cairo headers and add their path to the sconstruct and it compiled without complaining.

 

edit: new branch is created.

Link to comment
Share on other sites

radiant/environment.cpp: In function ‘char* getexename(char*)’:
radiant/environment.cpp:61: error: ‘g_argv’ was not declared in this scope
radiant/environment.cpp: In member function ‘void Environment::init(int, char**)’:
radiant/environment.cpp:107: error: ‘app_path’ was not declared in this scope
radiant/environment.cpp:111: error: expected `;' before ‘}’ token
scons: *** [build/debug/radiant/environment.o] Error 1

Link to comment
Share on other sites

Yes, that works fine now.

 

If you're refactoring the environment stuff, you can look out for cruft that can be removed. That stupid initArgs() function, containing what I can only imagine was some kind of humourous obfuscated-code challenge, would be a prime candidate for termination with extreme prejudice.

Link to comment
Share on other sites

It seems that DarkRadiant is not using any command-line arguments anyway, so I agree that these can be removed.

 

I'm currently looking for a platform-independent way of determining the application and home paths. The current code is using compiler switches (which is probably inevitable), but perhaps there is some boost or other stuff that can be used here.

 

edit: actually, the args are used for the posix-specific code, when the readlink call fails to deliver the path. Don't know if that ever happens, though.

Link to comment
Share on other sites

OrbWeaver, have you ever had a look at the HomePathObserver / EnginePathObserer / VFSModuleObserver stuff? Do you know what they are for?

 

(For what it seems, each observer seems to be attached to the other observer and all those form some sort of chain that gets fired when the engine path gets changed.) It looks pretty ugly to me, but perhaps there is a deeper reason behind it, apart from the obivous of having more code just for the fun of it.

Link to comment
Share on other sites

I guess the idea was that certain modules or plugins might need to know when the respective path is changed, but it seems like a pretty useless feature to me. I am not aware of anything that is actually using it (in fact I removed the addXXXPathObserver() methods from IRadiant because they are not used by anything).

Link to comment
Share on other sites

I spent half an hour drawing a diagram what is actually going on and I think I get it now. As you said, it's meant to open a possibility of attaching observers to a global list that get notified upon engine path change. Which is not a bad idea in theory but it's immensely complicated in its implementation.

 

I traced through the code and it boils down to actually three function calls when the engine path is changed: HomePaths_realise, VFS_Init and GlobalFileSystem().initialise() (or their negative counter-parts, respectively).

 

And for that it takes nearly 200 lines of module observer classes and global variables including hardcoded realisation counters. We're talking about observer overkill here.

 

If you don't object, I'll remove that stuff and replace it by something inelegant like ordinary function calls (I know, very boring of me).

 

(I'll take care of course that the engine path is still change-able during runtime, which was the main idea behind that implementation, which is not a bad thing.)

Link to comment
Share on other sites

And for that it takes nearly 200 lines of module observer classes and global variables including hardcoded realisation counters. We're talking about observer overkill here.

 

Only 200? They must have been slimming.

 

If you don't object, I'll remove that stuff and replace it by something inelegant like ordinary function calls (I know, very boring of me).

 

By all means.

Link to comment
Share on other sites

:laugh: Everytime I read your postings about the inner workings of Radiant, I'm always glad that I work with the SDK. :)

 

Ordinary programmer's idea of a loop:

 

void doLoop() {
for (int i = 0; i < 3; i++)
	doSomething(i);
}

int main() {
doLoop();
}

 

Radiant programmer's idea of a loop:

 

template<Val>
class IntegerValue {
static int getValue() {
	return Val;
}
};

template<typename ValueType>
class DoLoop {
static void execute() {
	doSomething(ValueType::getValue());
}
}

int main() {
DoLoop<IntegerValue<0>>::execute();
DoLoop<IntegerValue<1>>::execute();
DoLoop<IntegerValue<2>>::execute();
}

 

This is NOT an exaggeration, there was some code almost identical to this which I removed.

Link to comment
Share on other sites

What actually creeps me out is asking why the heck they (or he?) did it this way. Was it sports? Was there a big picture behind it? Was it genius/insanity? And most of all why isn't there a single line of comment in that damned code? I'm 99% sure that the one who coded this actually needs the same amount of time as me re-thinking through all that crap when he needed to revisit the code.

Link to comment
Share on other sites

I did a bit of careful refactoring in the preferences stuff and factored the CGameDescription class out into a separate file (the other classes will follow).

 

I changed the behaviour of the very first startup, where the game type is queried ("Doom 3"). As the choice is based on the available *.game files, the user hasn't much to select anyway in the case of The Dark Mod. I changed it so that the doom3.game is automatically selected if it's the only .game file available. The choice is stored in the registry.

 

I also removed a bunch of CopiedStrings, g_strings and other unneeded garbage.

Link to comment
Share on other sites

All of my legacy Doom 3 models are missing, and the new "Select game" dialog shows garbage characters in the list of games. Additionally, on startup I get these

 

(darkradiant:7289): GLib-GObject-WARNING **: invalid (NULL) pointer instance

(darkradiant:7289): GLib-GObject-CRITICAL **: g_signal_connect_data: assertion `G_TYPE_CHECK_INSTANCE (instance)' failed

 

and on displaying the Select Game dialog I get these

 

(darkradiant:7298): Pango-WARNING **: Invalid UTF-8 string passed to pango_layout_set_text()

(darkradiant:7298): Pango-WARNING **: Invalid UTF-8 string passed to pango_layout_set_text()

(darkradiant:7298): Pango-WARNING **: Invalid UTF-8 string passed to pango_layout_set_text()

 

This looks a lot like uninitialised or undefined data.

 

Also note that your temporary commenting-out of code in preferences.h generates compiler warnings ("warning: no return statement in function returning non-void", "warning: control reaches end of non-void function"). As long as these return values are never used this is not a problem, but if they are the result is undefined.

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.


  • Recent Status Updates

    • The Black Arrow

      Hope everyone has the blessing of undying motivation for "The Dark Mod 15th Anniversary Contest". Can't wait to see the many magnificent missions you all may have planned. Good luck, with an Ace!
      · 0 replies
    • Ansome

      Finally got my PC back from the shop after my SSD got corrupted a week ago and damaged my motherboard. Scary stuff, but thank goodness it happened right after two months of FM development instead of wiping all my work before I could release it. New SSD, repaired Motherboard and BIOS, and we're ready to start working on my second FM with some added version control in the cloud just to be safe!
      · 2 replies
    • Petike the Taffer  »  DeTeEff

      I've updated the articles for your FMs and your author category at the wiki. Your newer nickname (DeTeEff) now comes first, and the one in parentheses is your older nickname (Fieldmedic). Just to avoid confusing people who played your FMs years ago and remember your older nickname. I've added a wiki article for your latest FM, Who Watches the Watcher?, as part of my current updating efforts. Unless I overlooked something, you have five different FMs so far.
      · 0 replies
    • Petike the Taffer

      I've finally managed to log in to The Dark Mod Wiki. I'm back in the saddle and before the holidays start in full, I'll be adding a few new FM articles and doing other updates. Written in Stone is already done.
      · 4 replies
    • nbohr1more

      TDM 15th Anniversary Contest is now active! Please declare your participation: https://forums.thedarkmod.com/index.php?/topic/22413-the-dark-mod-15th-anniversary-contest-entry-thread/
       
      · 0 replies
×
×
  • Create New...