datiswous Posted January 9, 2023 Report Share Posted January 9, 2023 If I create a new skin in the skin editor it creates a new skin with name new_skin . When I try to edit the name DR immediately crashes. Maybe I'm doing something wrong? But it should not crash at least. Using DR 3.7.0 under Linux Quote Link to comment Share on other sites More sharing options...
datiswous Posted January 17, 2023 Author Report Share Posted January 17, 2023 No responses. I created a bug report: https://bugs.thedarkmod.com/view.php?id=6239 1 Quote Link to comment Share on other sites More sharing options...
datiswous Posted April 4 Author Report Share Posted April 4 And another bug report for 3.9.0 https://bugs.thedarkmod.com/view.php?id=6514 Quote Link to comment Share on other sites More sharing options...
MirceaKitsune Posted April 4 Report Share Posted April 4 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. Quote Mods: Builder Blocks minigame | Keypad | Disguises Link to comment Share on other sites More sharing options...
datiswous Posted April 5 Author Report Share Posted April 5 (edited) I guess the devs are clueless about this bug, otherwise one would think they would have tried to fix it somewhere during this year. Edited April 5 by datiswous Quote Link to comment Share on other sites More sharing options...
jonri Posted April 5 Report Share Posted April 5 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. 1 Quote Link to comment Share on other sites More sharing options...
MirceaKitsune Posted April 5 Report Share Posted April 5 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. Quote Mods: Builder Blocks minigame | Keypad | Disguises Link to comment Share on other sites More sharing options...
OrbWeaver Posted April 5 Report Share Posted April 5 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> 1 Quote DarkRadiant homepage ⋄ DarkRadiant user guide ⋄ OrbWeaver's Dark Ambients ⋄ Blender export scripts Link to comment Share on other sites More sharing options...
jonri Posted April 6 Report Share Posted April 6 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. 2 Quote Link to comment Share on other sites More sharing options...
datiswous Posted April 7 Author Report Share Posted April 7 Why does it not crash on Windows though? Or is that code different there? Quote Link to comment Share on other sites More sharing options...
jonri Posted April 7 Report Share Posted April 7 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: 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 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. 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. 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. 1 1 Quote Link to comment Share on other sites More sharing options...
MirceaKitsune Posted April 7 Report Share Posted April 7 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. Quote Mods: Builder Blocks minigame | Keypad | Disguises Link to comment Share on other sites More sharing options...
datiswous Posted April 7 Author Report Share Posted April 7 Well I was using the Flatpack version which is always in line with a release version I think. Quote Link to comment Share on other sites More sharing options...
jonri Posted April 8 Report Share Posted April 8 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. Quote Link to comment Share on other sites More sharing options...
OrbWeaver Posted April 9 Report Share Posted April 9 On 4/7/2024 at 9:27 PM, jonri said: 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 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. 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. 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. 3 Quote DarkRadiant homepage ⋄ DarkRadiant user guide ⋄ OrbWeaver's Dark Ambients ⋄ Blender export scripts Link to comment Share on other sites More sharing options...
datiswous Posted July 4 Author Report Share Posted July 4 What is the current status? I see @OrbWeaverproposed something. Is something already implemented? Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.