Feature: Porting MFC tools

[ashalkhakov]

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

Currently ported tools:

• PDA editor

• Particle editor (WIP, mostly done)

• Decl browser (WIP)

[DanielGibson]

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

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?

[ashalkhakov]

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.

[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)?

[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)?

Thank you, I'll check it out!

[ashalkhakov]

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

[DanielGibson]

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]

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]

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]

maybe this could be implemented like

idStr fileName = "newpdas/";
fileName += pdaName; // TODO
fileName += ".pda";
fileSystem->RemoveFile( fileName );

[ashalkhakov]

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.

[DanielGibson]

remember to also move the lines for editParticles and editScripts above the #ifdef ID_ALLOW_TOOLS :)

[DanielGibson]

.. and Com_EditScripts_f() and Com_EditDecls_f() as well ;)

[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]

probably to be expected at this early point of the script editor, but remember that all those FULLCAPS types are windows-only, so this currently breaks the build on Linux and macOS

[ashalkhakov]

Sorry about that, I'm currently trying to just get something out the door. ^_^ Thanks for the feedback, much appreciated!

[ashalkhakov]

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]

@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:

• OpenEmacs
• AttoEmacs
• Med

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.

[DanielGibson]

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]

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]

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?

[ashalkhakov]

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

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.

[DanielGibson]

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 :)

[ashalkhakov]

@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.

[DanielGibson]

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]

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)

[ashalkhakov]

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.

[DanielGibson]

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]

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:

That's not necessary, or actually redundant - you can just Ctrl+Click into the slider to input the value as text :)

Together with my other tweaks it now looks like this:

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)

[ashalkhakov]

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.

Okay, that's reasonable and makes my job easier. :)

The patch implementing these changes: parteditor.diff.txt

I'll probably add those tabs some time later, in the meantime, this looks much better! So I'll apply it. Thanks!