Jump to content
The Dark Mod Forums

DarkRadiant crashes when trying to change the name of a newly created skin in the skin editor


Recommended Posts

  • 2 weeks later...
  • 1 year later...

Sorry for the double report, ran into this myself and quickly posted about it so the issue wasn't missed. Currently the skin editor is unusable: Editing the skin name field instantly crashes DR, so does exiting the menu after making any changes, and any changes you do manage to make are immediately reverted and don't stick. The skin editor is a very useful tool and I hope it can be fixed.

Link to comment
Share on other sites

I think the devs just haven't had much time to put into it the past year (myself included).  I took care of the mouse issue finally, since it became a 100% showstopper for me to use DarkRadiant at all on Linux.  I've only ever made my skins using a text editor, but I tried reproducing the issue and it crashes with a backtrace for me too.  This means debugging it should be straightforward, I'll look at it this weekend.

 

  • Thanks 1
Link to comment
Share on other sites

I also take it the skin editor isn't among the most used parts of DarkRadiant, making this less easy to spot. Hopefully now the team will notice the issue, and it's ideally nothing hard to fix since DR has a history of GUI issues like this that stabilized.

Link to comment
Share on other sites

Reproduced with 3.9.0 on Ubuntu. This is the backtrace:
 

ASSERT INFO:
../src/unix/threadpsx.cpp(1678): assert "This() == this" failed in TestDestroy(): wxThread::TestDestroy() can only be called in the context of the same thread

BACKTRACE:
[1] wxThread::TestDestroy()
[2] wxutil::ThreadedResourceTreePopulator::ThrowIfCancellationRequested()
[3] decl::DeclarationManager::renameDeclaration(decl::Type, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
[4] skins::Doom3SkinCache::renameSkin(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
[5] wxEvtHandler::ProcessEventIfMatchesId(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&)
[6] wxEvtHandler::SearchDynamicEventTable(wxEvent&)
[7] wxEvtHandler::TryHereOnly(wxEvent&)
[8] wxEvtHandler::ProcessEventLocally(wxEvent&)
[9] wxEvtHandler::ProcessEvent(wxEvent&)
[10] wxEvtHandler::SafelyProcessEvent(wxEvent&)
[11] wxTextEntryBase::SendTextUpdatedEvent(wxWindow*)
[12..34] <internal GTK stuff> 
[35] wxGUIEventLoop::DoRun()
[36] wxEventLoopBase::Run()
[37] wxDialog::ShowModal()
[38] wxutil::DialogBase::ShowModal()
[39] cmd::CommandSystem::executeCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<cmd::Argument, std::allocator<cmd::Argument> > const&)
[40] cmd::CommandSystem::execute(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
[41] wxEvtHandler::ProcessEventIfMatchesId(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&)
[...] <Other GTK/wxWidgets stuff>

 

  • Like 1
Link to comment
Share on other sites

Looking at the backtrace and the code, I think the culprit is here: https://github.com/codereader/DarkRadiant/blob/fa4c266a6776516b59740f78f57b849097d97618/radiant/ui/skin/SkinEditorTreeView.cpp#L44

The ThreadedDeclarationTreePopulator is created on the stack and thus deallocated at the end of the function.  The EnsureStopped() function is called on deletion (https://github.com/codereader/DarkRadiant/blob/master/libs/wxutil/dataview/ThreadedResourceTreePopulator.cpp#L108) but this doesn't appear to actually wait for the deletion to finish.  By the time the deletion request is processed, the object itself is no longer valid.  Other usages I see put the object in a shared pointer instead of on the stack.

I'll have a chance to look at this properly on Sunday and figure out the best way to fix it.

 

  • Thanks 2
Link to comment
Share on other sites

14 minutes ago, datiswous said:

Why does it not crash on Windows though? Or is that code different there?

I'm honestly not sure, it's a race condition maybe?  I found and fixed 2 different segfaults (PR is here), one relating to threading and the other one relating to a variable being referenced after it was cleared out.  This stops the skin editor from crashing, but it seems there are still a lot of usability issues.  I'm not sure if they are all Linux only, but here's what I found:

  1. The rename operation happens every time you type a character, which is a little laggy.  It would be better to rename it when you are done typing and the field loses focus
  2. Typing new characters while renaming works as expected, but deleting characters causes the box to highlight so you have to reset the cursor before deleting another one.
  3. On the Material Replacements tab, click to edit is not working for me.  I spent a lot of time on this, I can get a box to show up if I replace the editbox+button with a simple edit box.  I think it's related to the compound control not being able to have focus.
  4. The skin name is showing up on top of the Close button

This will require more wxWidget-fu than I have time to do right now.

 

  • Like 1
  • Thanks 1
Link to comment
Share on other sites

2 hours ago, datiswous said:

Why does it not crash on Windows though? Or is that code different there?

It could also be that on Linux we have to compile DarkRadiant ourselves, which means it's being compiled against newer versions of packages (GTK, WxWidgets, etc) and a slightly different configuration overall.

Link to comment
Share on other sites

2 hours ago, MirceaKitsune said:

It could also be that on Linux we have to compile DarkRadiant ourselves, which means it's being compiled against newer versions of packages (GTK, WxWidgets, etc) and a slightly different configuration overall.

Is not quite that, but more likely because wx uses native Windows controls there and gtk only on Linux. It does a pretty good job of abstracting away the differences but there are always corner cases where a subtle difference turns out to be important.

Link to comment
Share on other sites

On 4/7/2024 at 9:27 PM, jonri said:
  1. The rename operation happens every time you type a character, which is a little laggy.  It would be better to rename it when you are done typing and the field loses focus
  2. Typing new characters while renaming works as expected, but deleting characters causes the box to highlight so you have to reset the cursor before deleting another one.
  3. On the Material Replacements tab, click to edit is not working for me.  I spent a lot of time on this, I can get a box to show up if I replace the editbox+button with a simple edit box.  I think it's related to the compound control not being able to have focus.
  4. The skin name is showing up on top of the Close button

I can reproduce all of those.

For (1), I propose that the Skin Name field should be non-editable, and have a separate button (usually the "pencil" icon) to show a popup entry dialog for editing the name. I doubt renaming skins is all that common, and it certainly doesn't need to happen on every keypress. But if people object to additional popups, the field could be editable but only commit the changes on ENTER or if a "tick" button was clicked.

For (3), single-click to edit in a list is rather non-standard (double-click might be more expected). I propose to have two named entry fields below the list for "From" and "To", rather like the key/value fields in the Entity Inspector, with the fields reflecting the currently-selected list item and allowing changes (committed on ENTER or button click).

Not specific to the skin editor, but I think we need an application preference for monospace font size. Reading the declarations is really difficult on my 1440p monitor.

  • Like 3
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

      Happy birthday to me!
      Yes, today's my birthday and as much as I want to play The Dark Mod, I won't until it's late Autumn, where it should be a bit chilly in my place and there's some wind too.
      For now, it's time to enjoy my birthday, if any of you want to play a great fan mission in my honour...Well that's asking too much, just play the mission because it's fun, not because of me, enjoy it yourself! 💪
      · 4 replies
    • nbohr1more

      New texture wiki article created: https://wiki.thedarkmod.com/index.php?title=Creating_textures_from_Ornate_Photos
      · 2 replies
    • Xolvix

      Took a break from TDM until I got my backlog under control. It's been months and it's still not "under control". Damnit.
      · 4 replies
    • Ansome

      I sleep well at night knowing the player will never see the absolute nightmare that is my sealing brushwork outside the playable area. Only the pointfile can judge me now.
      · 4 replies
    • JackFarmer

      Somehow I admire the fact that the material from Dune has now been filmed for the third time. Personally, I could never do much with the material, but as a child of the 80s, I of course know the David Lynch movie...and that movie was at least funny!
      · 2 replies
×
×
  • Create New...