Jump to content
The Dark Mod Forums

mohij for Dark Radiant coding


New Horizon

Recommended Posts

Okay, I already repaired this. OrbWeaver already said what had to be done. I guess that was to easy...

So a bigger task would perhaps be better.

 

ps. What about SVN access vs. sending patches to one of you?

 

How about this: Add "Player Start here" option to Orthocontext Menu?

 

OrbWeaver is the project lead, he'll have to decide about SVN access. :)

Link to comment
Share on other sites

  • Replies 69
  • Created
  • Last Reply

Top Posters In This Topic

Thanks for the patch, I already committed it to SVN. :)

 

I saw you're using spaces as indentation, which is probably not a problem, I just wanted to point out that I used tabs (that are displayed 4 spaces wide) for all my refactoring.

Link to comment
Share on other sites

Patches is certainly the way to go to start with, yes.

 

Regarding the coding style, there are some guidelines at http://www.thirdfilms.com/darkwiki/index.p...oding_standards. I'm not going to beat anyone up over tabs versus spaces (I'm not even sure which I use myself, it depends how the editor converts things), however please don't use 2 spaces since that is harder to read and inconsistent with the 4 that we are using.

 

Similarly with brace styles, line breaks etc -- there are certain conventions I use but these are not formal rules; the most important thing is to make it look readable.

Link to comment
Share on other sites

@mohij: Thanks for the patch, this does work fine. And it was quick. :)

 

I know that I haven't mentioned this before, but I think it should be expanded to take an existing info_player_start into account. Otherwise the map would end up with several info_player_start entities (but only one would be considered by Doom3).

 

There is a class available named EntityFindByClassNameWalker (in entitylib.h) which can be used to search for a specific entity class. If such an entity is found, it should be moved to the specified location instead of creating a new entity.

 

The ideal solution would be to perform this check before the context menu is actually shown, such that you can rename the Menu item caption to "Move player start here" instead of "Add player start here". This could be more complicated, but I guess you should be challenged a bit. ;)

Link to comment
Share on other sites

I finished programming this feature. My problem is that radiant segfaults on any operation on entities. As this also happens also without using my new button, this is probably not related to my changes. So I can't test my button (and remove all those bugs that are surely in there ;-).

 

Sometimes it just segfaults other times it shows:

/plugins/entity/eclassmodel/../modelskinkey.h:114
assertion failure: pointer "instantiable" is null
----------------
----------------
Please report this error to the developers

I guess it is related to greebos recent changes.

I have no idea what I should do...

Link to comment
Share on other sites

I will look into this and hopefully be able to fix it this evening.

 

edit: If you have time, perhaps you could check if this occurs in revision 1760 as well? This was before I started refactoring the typecast system.

Link to comment
Share on other sites

I checked for a missing implementation on the relevant classes as OrbWeaver suggested, but all scene::Nodes are deriving from scene::Instantiable as well (apart from BasicContainer, which I think is not used in this case).

 

I added a NULL pointer guard around the critical part in InstanceSubGraphWalker that emits an error message and some information about that node to std::cout, if the node cast to Instantiable fails.

 

Could you check if the HEAD revision is still crashing or which error message is emitted when creating entities? I will setup a dual-boot Ubuntu today, so maybe I can get to it earlier than you, maybe not. :)

Link to comment
Share on other sites

As you can't see the internal developer forums, I thought I'd drop a note here as well: the problem/crash in Linux is due to the dynamic_cast operator not working when used on classes created in the shared libraries like the entity plugin. The cast returns NULL where it is expected to return something useful. This works in Windows, but not in Linux as it seems.

 

OrbWeaver is currently investigating the linker options to find a way around it - if you're feeling adventurous, feel free to play around with the compiler flags as well. :)

Link to comment
Share on other sites

MinGW has gcc 3, Linux generally version 4.

 

That seems to be the root of the problem -- GCC 4 uses address-based type identification wherease GCC 3.3 (presumably) used name-based. This means that without careful usage of linker options, dynamic_cast and other RTTI operations do not work across different DLLs in the same executable.

 

Fixing the linker options is of course easy, it is the knock-on effects in the form of random crashes during initialisation which need further investigation.

Link to comment
Share on other sites

It seems to be fixed by the -fPIC switch I had to introduce to make it compile-able on my AMD64 system in Ubuntu. However, it still crashes as soon as I try to view the Media Browser / Shader Selectors and during shutdown. And the splash image is missing somehow, which may even be related to the shutdown crash, but that's a wild guess currently.

Link to comment
Share on other sites

mohij, sorry for letting you wait this long, we're still struggling with some issues after the scenegraph refactors (it was a bit of an unfortunate moment, the months before we didn't have such show-stopping issues).

 

I promise that I will look into your contribution in the afternoon, when I can test it using my Windows build environment again. :)

Link to comment
Share on other sites

Ok, I could finally take a look at the contribution, it works fine so far! :)

 

I found a few things I corrected before committing it to SVN:

 

You have an include there pointing into the plugins folder:

#include "../../../plugins/entity/origin.h" // write_origin()

which is certainly not allowed, because everything under the plugins/ tree should be more or less self-contained and cross-references from radiant/ to plugins/ or vice versa are not allowed. It's of course safe to include something from the libs/ and includes/ stuff from both radiant/ and plugins/.

 

I replaced the write_origin() call with this:

Entity* playerStart = walker.getEntity();

if (playerStart != NULL) {
playerStart->setKeyValue("origin", self->_lastPoint);
}

The setKeyValue() and getKeyValue() methods are the direct and convenient way to alter spawnargs on entities. In case you wonder: if you need to delete a spawnarg, just call it with an empty string setKeyValue("origin", ""), for instance.

 

In callbackMovePlayerStart(): I added a NULL pointer check before using the result of EntityFindByClassnameWalker. It's unlikely that it returns NULL, because you made sure that the according menu item is only sensitive when there is an info_player_start entity existing, but I consider it cleaner this way. You never know how if some routines are used by something else you didn't design it for, so they should be capable of catching such cases.

 

Apart from that it all works fine and the header file is properly commented, which is good. You also made the widget member variables more consistent, which is fine as well.

 

I also noticed that the sensitivity of the "Move Playerstart Here" item is disabled if the info_player_start entity itself is selected, due to this

gtk_widget_set_sensitive(_movePlayerStart, info.entityCount == 0 ? TRUE : FALSE);

That's of course not an issue in itself, it is just possible that mappers find that confusing. We can change that later on, if we, the testers or mappers consider this impractical. :)

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

    • 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!
      · 1 reply
    • 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
    • JackFarmer

      @TheUnbeholden
      You cannot receive PMs. Could you please be so kind and check your mailbox if it is full (or maybe you switched off the function)?
      · 1 reply
×
×
  • Create New...