dhewm3 icon indicating copy to clipboard operation
dhewm3 copied to clipboard

Feature: Porting MFC tools

Open ashalkhakov opened this issue 11 months ago • 51 comments

This adds editors ported from MFC and builds on top of imgui-docking.

Currently ported tools:

  • PDA editor

image

  • Particle editor (WIP, mostly done)

image

  • Decl browser (WIP)

image

  • Script Editor (with working autosuggestions and tooltips)

image

ashalkhakov avatar Jan 31 '25 16:01 ashalkhakov

Thank you very much! The screenshot looks pretty good :)

I'll look at the code later; are the questions from https://github.com/dhewm/dhewm3/pull/647#issuecomment-2626646805 regarding modal dialogs and layout still open or have you figured that out already? Modals should be pretty easy (ImGui::BeginPopupModal()), for layout I'd have to figure that out myself, depending on what you want to do. Is it about having basically three columns and have that scale when resizing the window?

DanielGibson avatar Jan 31 '25 16:01 DanielGibson

I'll look at the code later; are the questions from #647 (comment) regarding modal dialogs and layout still open or have you figured that out already? Modals should be pretty easy (ImGui::BeginPopupModal()), for layout I'd have to figure that out myself, depending on what you want to do. Is it about having basically three columns and have that scale when resizing the window?

I think I figured modals out. The code:

	CDialogPDAAdd dlg;
	if ( dlg.DoModal() == IDOK ) {
/* modal dialog accepted */
	}

translates to the opening of the dialog:

	addPDADlg.Reset();
	ImGui::OpenPopup( "PDAAdd" );

and it's drawing & output handling routine:

		if ( ImGui::BeginPopupModal( "PDAAdd", nullptr, ImGuiWindowFlags_AlwaysAutoResize ) ) {
			bool accepted = addPDADlg.Draw();

			if ( accepted ) {
/* modal dialog accepted */
			}

			ImGui::EndPopup();
		}

for layout I'd have to figure that out myself, depending on what you want to do. Is it about having basically three columns and have that scale when resizing the window?

Yes, I wanted to have a 3-column layout like in the original MFC dialog and have it scale or change according to the available space. For this dialog, I think the fixed layout will suffice, for other dialogs we may have to get creative.

There is also the issue that the window position is reset when PDA is selected.

ashalkhakov avatar Jan 31 '25 16:01 ashalkhakov

Ok, I'll have a look!

By the way, did you check out the ImGui Demo (open Dhewm3Settings menu with F10 -> Other Options -> [Show ImGui Demo])? Maybe it could somehow be done with groups (Layout & Scrolling -> Groups) and/or Tables (Tables & Columns -> Resizable, stretch)?

DanielGibson avatar Jan 31 '25 16:01 DanielGibson

Ok, I'll have a look!

By the way, did you check out the ImGui Demo (open Dhewm3Settings menu with F10 -> Other Options -> [Show ImGui Demo])? Maybe it could somehow be done with groups (Layout & Scrolling -> Groups) and/or Tables (Tables & Columns -> Resizable, stretch)?

Thank you, I'll check it out!

ashalkhakov avatar Jan 31 '25 17:01 ashalkhakov

@DanielGibson Table layout works nicely! Also, fixed the reappearing window bug: the name of the window passed to ImGui must be stable.

image

ashalkhakov avatar Jan 31 '25 17:01 ashalkhakov

That's awesome!

Regarding the stable title: You can use "Whatever changing text###PDA Editor" as title (or generally as name in ImGui widgets) - then only "PDA Editor" (or its hash) is used as identifier and only "Whatever changing text" is displayed.

Simlarly, you could use for example "Add...##Email" and "Add...##video" (this time 2 # instead of 3) - then the ID is calculated from the whole string, but still only "Add..." is displayed. But of course pushing IDs around them is also a solution for that.

DanielGibson avatar Jan 31 '25 17:01 DanielGibson

I know this was already the case in the original PDA editor, but I wonder why the buttons in the left pane use "Add" and "Del" while the others use "Add..." and "Delete..."

Also, I just tried that "Add" (PDA) button and it asked me for a name and I entered it and clicked ok, but the modal didn't close (neither when clicking cancel or pressing escape or anything else). The .pda file was written, though.

DanielGibson avatar Jan 31 '25 19:01 DanielGibson

In https://github.com/DanielGibson/dhewm3/tree/imgui-tools I have (based on your branch) integrated the AfEditor and also modified Com_EditPDAs_f() in the way I suggested above

DanielGibson avatar Feb 01 '25 03:02 DanielGibson

I know this was already the case in the original PDA editor, but I wonder why the buttons in the left pane use "Add" and "Del" while the others use "Add..." and "Delete..."

I just tried to implement deletion, cannot get it to work. The Decl manager keeps a copy of PDA decl in memory, so even if the file is deleted, it will still get re-created.

Also, I just tried that "Add" (PDA) button and it asked me for a name and I entered it and clicked ok, but the modal didn't close (neither when clicking cancel or pressing escape or anything else). The .pda file was written, though.

Fixed that. I'll add the missing dialogs for audio & video and include this fix.

ashalkhakov avatar Feb 01 '25 17:02 ashalkhakov

The Decl manager keeps a copy of PDA decl in memory, so even if the file is deleted, it will still get re-created.

Interesting. Looks like the idDeclManager doesn't support removing decls at all - that's weird. I guess the Delete button has to remain a dummy then, for now (or maybe even comment it out, what's the point in buttons that don't do anything...)

DanielGibson avatar Feb 01 '25 17:02 DanielGibson

@DanielGibson Should I do a PR to your branch or to the main repo? (I've just updated imgui-tools on my fork.)

ashalkhakov avatar Feb 01 '25 18:02 ashalkhakov

You could update your tools-porting branch with my changes (either merge or rebase, I don't care) and continue to work on that. I don't think we need a new PR for them, let's just keep this here

DanielGibson avatar Feb 01 '25 18:02 DanielGibson

Great to see you working on the particle editor!

I've toyed a bit with the AF Editor, and it seems to be in a pretty rough shape. Using it triggers assertions both in dhewm3 code and in ImGui, some things (like the "New Articulated Figure" dialog) are kinda glitchy - not even sure if that thing is "complete" (as in at least tries to implement all features).

As I didn't find any bugreports in RBDoom3BFG about that, I guess no one has ever really used it?

Also, not sure if it's easier/faster to fix that thing or to rewrite it (based on the MFC code).

DanielGibson avatar Feb 06 '25 05:02 DanielGibson

I just updated my imgui-tools branch again. It now contains your latest changes and also some new changes from me - most importantly, now if an ImGui window is open, input events (like keys pressed or mouse moved or clicked) are not sent to the game but only to ImGui, UNLESS you click and hold the right mouse button somewhere outside an ImGui window (but in the game window, of course). This means if for example the light editor is open you can look and move around the level without closing (or even just unfocusing) the light editor window, as long as you hold the right mouse button. And once you stop holding it, the ImGui cursor is back and you can click in the light editor (or other ImGui window) again.

DanielGibson avatar Feb 06 '25 05:02 DanielGibson

Thanks for support and the suggestions, I will fix them!

As for the progress so far. Regarding dialogs: with classical GUIs it's just a call a dialog as a subroutine, but with ImGui you have to add some extra variables to the state of your app to keep track of which "control state" you're in, i.e. waiting for input from the user or acting on the inputs provided. Not a big deal, it's probably best for the users when there's less modal stuff going on.

More concretely, I'm looking for an autosuggest widget (listbox where you can either select or type the chosen option). I've ported the file dialog code from AF Editor and it works fine.

I've tested the Particle Editor a bit, and it seems to work fine (but need to set r_useSoftParticles 0 -- we break it when we modify stages in the editor). A few things I want to add:

  • sorting of files in the file picker & material picker
  • autosuggestion for the file picker & material picker
  • sorting of particle systems in the combo box
  • use tabs for Particle Editor (I've reproduced the original layout, but the one in The Dark Mod makes more sense and will help cut down the size of the editor window)
  • find why requesting all decls is so costly when we populate the particle system combo box. I think it's probably all the parsing that's done. perhaps we can fix this? we could add a method to retrieve just the "header" info about decls

The next steps I'm planning are:

  • Decl Browser (simple enough)
  • Script editor
  • Audio Shader editor (probably as complex as the Particle Editor)

ashalkhakov avatar Feb 06 '25 10:02 ashalkhakov

More concretely, I'm looking for an autosuggest widget (listbox where you can either select or type the chosen option).

I think here you want to use an ImGuiTextFilter and some kind of ListBox or Table (table makes alternating background colors for the rows easier, if you want that) with Selectables as items. See ExampleAppPropertyEditor in imgui_demo.cpp and also Widgets -> Text Filter and also the "listbox 1" ImGui::ListBox example under Widgets -> ListBoxes that shows how to use it with ImGui::Selectable

DanielGibson avatar Feb 06 '25 10:02 DanielGibson

Great work so far!

but need to set r_useSoftParticles 0 -- we break it when we modify stages in the editor

I couldn't reproduce this, what particle did you use when you had issues with r_useSoftParticles 1?

find why requesting all decls is so costly when we populate the particle system combo box. I think it's probably all the parsing that's done. perhaps we can fix this? we could add a method to retrieve just the "header" info about decls

You could try setting the forceParse argument of declManager->DeclByIndex() to false.

By the way: I think it would be nice if those lists of materials etc were sorted alphabetically. I think it should work to just call list.Sort(); - there's an overload for idList<idStr> aka idStrList that implements a string sort.

And I think it'd be cool if the material picker could show a preview image for a selected material. This code code from the light editor may come in handy:

static idImage* GetLightEditorImage( const idMaterial* mat )
{
	// this is similar to what the original light editor does; rbd3bfg iterates through all stages to find a non-null image
	idImage* ret = (mat->GetNumStages() > 0) ? mat->GetStage(0)->texture.image : NULL;
	return ret ? ret : mat->GetEditorImage();
}

void DrawTexture() {
	if( currentTexture != nullptr )
	{
		ImVec2 size( currentTexture->uploadWidth, currentTexture->uploadHeight );

		ImGui::Image( currentTexture->texnum, size, ImVec2( 0, 0 ), ImVec2( 1, 1 ),
			          ImColor( 255, 255, 255, 255 ), ImColor( 255, 255, 255, 128 ) );
	}
}

(also, at some point check if currentTexture->texnum != idImage::TEXTURE_NOT_LOADED - the light editor just doesn't list any materials without a texnum)

DanielGibson avatar Feb 09 '25 01:02 DanielGibson

but need to set r_useSoftParticles 0 -- we break it when we modify stages in the editor

I couldn't reproduce this, what particle did you use when you had issues with r_useSoftParticles 1?

You could try setting the forceParse argument of declManager->DeclByIndex() to false.

Thanks! This works great!

By the way: I think it would be nice if those lists of materials etc were sorted alphabetically. I think it should work to just call list.Sort(); - there's an overload for idList<idStr> aka idStrList that implements a string sort.

Should be working now, also, on-the-fly filtering should be working too. ^_^

And I think it'd be cool if the material picker could show a preview image for a selected material.

Noted! I'll get back to this before finishing up the PR.

ashalkhakov avatar Feb 09 '25 17:02 ashalkhakov

@DanielGibson I have a question two questions.

ScriptEditor

I'm choosing between the following editors to embed:

  • https://github.com/BalazsJako/ImGuiColorTextEdit (or rather, https://github.com/goossens/ImGuiColorTextEdit since it seems more up-to-date)
  • https://github.com/Rezonality/zep/

You mentioned ImGuiColorTextEdit earlier, but it's not been updated in a while. There are also some tiny Emacs clones floating around that we could embed:

To name a few. We could probably just 're-target' one of these to use ImGui output instead of CLI output since they are so tiny.

Anyways, just looking for feedback. Could you get me up to speed on which one to embed, and what's the process of including it into the codebase? Could I just copy it over? Should I use CMake to pull it from its source repo? Etc.

Memory management in PathTreeCtrl

I'm using idBlockAlloc for creating tree nodes, IIUC it doesn't call constructors/destructors for reused nodes? Should I worry about this? Doom's own codebase doesn't seem to use constructors much.

ashalkhakov avatar Feb 09 '25 18:02 ashalkhakov

Regarding editors: zep also looks good! There may even be other ImGui textediting widgets with syntax highlighting; just use the one you think works best and is easiest to integrate and has the least (ideally no) additional dependencies.

We could probably just 're-target' one of these to use ImGui output instead of CLI output

I guess it still would be non-trivial to get ImGui to render this, which includes cursor and text selection, properly (and with good performance).

Not sure of those emacs clones offer that, but it's important that the editor can be used without being familiar with emacs (or vi) shortcuts. Having such a mode as an option is fine, but by default it should behave like a standard GUI text editor (think notepad++, geany, kate, ...).

what's the process of including it into the codebase? Could I just copy it over?

Yes, just copy it over, like I do with ImGui.

Regarding the memory management, I'll have to take a closer look, will reply later.

DanielGibson avatar Feb 09 '25 18:02 DanielGibson

I'm using idBlockAlloc for creating tree nodes, IIUC it doesn't call constructors/destructors for reused nodes? Should I worry about this?

This depends on what types you allocate with it. If it's just POD (plain-old-data) types that don't need a destructor (like idVec3), for example to free dynamically allocated memory (and that don't have any members that need a destructor!), it's fine. Using it for types that have for example idList<Foo> or idStr members is bad, because they allocate memory on the heap that gets freed in their destructors, which have to be called.

If your types need idStr or similar, just use regular new and delete for now. If it turns out that the default allocators are too slow, we can still try to improve idBlockAlloc or write something else that reuses memory blocks.

(I first thought that hacking idBlockAlloc to call the constructors and destructors would be easy enough, by providing T* AllocNew() and FreeDestruct( T* obj ) methods that do that. But I think idBlockAlloc is also expected to free all memory in its own destructor, including objects that had been allocated but not explicitly freed yet - that's harder.)

DanielGibson avatar Feb 09 '25 19:02 DanielGibson

Correction: idBlockAlloc does call the proper constructors and destructors after all, but not in Alloc() and Free(), but when fresh a block_t (that contains an array of element_t that has a type member) is allocated, or in Shutdown() where everything is deleted.

Still not quite the behavior one would (probably) expect - but maybe good enough for your usecase?

DanielGibson avatar Feb 09 '25 20:02 DanielGibson

@DanielGibson Got ColorTextEditor working in the Script Editor, and finished up Decl Browser.

image

I would like to try using Zep next, to see if it's worth the cost. Next I'll need to make use of the new editor in Decl Editor dialog, and reimplement tooltips, autosuggestions and find/replace, bringing Script Editor and Decl Editor at feature parity with MFC.

ashalkhakov avatar Feb 11 '25 15:02 ashalkhakov

This looks awesome!

One thing that just came to my mind: It would be cool if the text editing widget could also be used for the Script Debugger. There it should be read-only (only used to render the text with syntax highlighting), but should still allow (right?-)clicking on line numbers to set breakpoints or to tell the debugger to "go on executing until this line" (basically temporary breakpoint).

So if any of the text editing widgets supports "read-only mode that still allows figuring out the line one just clicked in and render something in front of the line number to signify a breakpoint", that one would be preferable :)

DanielGibson avatar Feb 11 '25 16:02 DanielGibson

@DanielGibson My thoughts exactly! In fact it's the ability to debug things what's drawn me initially to dhewm3. I then found a whole host of other great things, like fully-maintained source code for mods that I can build on. I'll keep an eye on those features.

ashalkhakov avatar Feb 11 '25 16:02 ashalkhakov

Great! :)

I took a quick look and ImGuiColorTextEdit seems to directly support these features (TextEditor::SetReadOnly(), TextEditor::SetBreakpoints()). Not sure with zep, it has so many headers that figuring it out wasn't exactly straightforward.. but I guess you'll figure it out when integrating it :)

DanielGibson avatar Feb 11 '25 16:02 DanielGibson

The ImGuiColorTextEdit code needed some small changes to make it build with GCC (and C++11 instead of C++14): texteditor.patch.txt

(Haven't actually tested the code yet, just made sure it builds because the testbuilds are failing)

DanielGibson avatar Feb 12 '25 04:02 DanielGibson

The ImGuiColorTextEdit code needed some small changes to make it build with GCC (and C++11 instead of C++14): texteditor.patch.txt

(Haven't actually tested the code yet, just made sure it builds because the testbuilds are failing)

Thanks for the fix! That's something I didn't tell you about. Looks like we'll have to either figure out how to embed Zep (or some other similarly-featured editor), or we'll have to add features to our own fork of ImGuiColorTextEdit. Those forks that I found have the following shortcomings:

  • the original fork has not been updated for a while and there's a scrolling bug in it out of the box (although it still works! and there's a PR fixing that bug)
  • the fork by santaclose uses C++14 (as you found) and has a dependency on boost::regex (we don't need regexes, we already have the code that would be simple enough to port I think) -- I had to remove regex support in the vendored file. it also doesn't support breakpoints and most of the cursor stuff is not public anymore, e.g. one cannot get the selected text.
  • the fork by goosens uses C++17 and I couldn't figure out how to back port it to C++11 (BTW, is this the standard we're using in the codebase?)

None of these have support for:

  • go to line (need to implement a custom dialog)
  • find/replace (need to implement a custom dialog)
  • support for object member completion (custom reaction to keystrokes)
  • support for function parameter tooltips (custom reaction to keystrokes)

I've tried to use the single-file inclusion of Zep but it uses C++17 so it doesn't compile, that's a bummer. So I'll probably just go forth and implement those features in our own fork, shouldn't be too difficult, since we already have it implemented in MFC.

ashalkhakov avatar Feb 12 '25 08:02 ashalkhakov

Regarding the forks: I definitely do not want a dependency on boost. Otherwise, use whichever fork works best, and patching it is also ok :)

Regarding the C++ standard: Most of dhewm3 still uses C++98, and unless there's a good reason I'd like to keep it that way so people can build dhewm3 on obscure old platforms like PowerPC Macs. The ImGui tools currently require C++11, because ImGui itself does. This shouldn't affect old platforms too much, as ImGui (incl. ImGui tools) can be disabled for them. If you need C++14 or C++17 for ImGui-tool-specific code, that'd also be fine with me (in my patch I made it build with C++11 instead of configuring C++14 in CMakeLists.txt because it was just one tiny change). You can bump the C++ standard version in CMakeLists.txt line 236 (set (CMAKE_CXX_STANDARD 11)).

DanielGibson avatar Feb 12 '25 17:02 DanielGibson

I've been tweaking the ParticleEditor a bit, so it's narrower, more like the MFC version. One little thing I noticed was that you use InputFloat() together with SliderFloat(), like this: image That's not necessary, or actually redundant - you can just Ctrl+Click into the slider to input the value as text :) image

Together with my other tweaks it now looks like this: image

The patch implementing these changes: parteditor.diff.txt (You don't have to apply it, or all of it, especially if you have better ideas how to improve the particle editor, like using tabs, as you mentioned. That's also why I didn't push these changes into the imgui-tools branch)

DanielGibson avatar Feb 13 '25 04:02 DanielGibson