Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NetImgui branch update #1

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

sammyfreg
Copy link

  • Upgraded NetImgui branch with latest master branch content
  • Upgraded NetImgui library to latest v1.9 version
  • Pulled Unreal Server support from TDeslandes

Seeing as the NetImgui branch is now up to date, I suggest that it get also get pulled into master branch.

This will keep it up to date at all time and people that do not wish to use it can set kUseNetImgui in NetImguiLibrary.Build.cs to false, it will remove it from compilation.

sammyfreg and others added 9 commits September 13, 2020 19:33
Merging initial integration of NetImgui by Sammyfreg:
- Integration of the NetImgui.
- Build as part of the ImGui module.
- Implemented UE4 sockets.
- Added mirroring and dual context support.
- Menu bar to switch between contexts.
Merging Bugfix for NetImGui: Making sure sockets are all closed before exiting netImgui shutdown.
* Added support for netImgui.
TODO: fix issue with editor not drawing until PIE activated once
TODO: Implement UE4 sockets (instead of using Winsock directly)
TODO: Figure out how to expose options to user, and netImgui server exe

* netImgui: Fix compile and running error in game mode

* NetImgui: Added mirroring option

* Fix for Texture format detection and disconnect request

* Added : Dual Draw support
Added: IsRemoteDrawing() and IsRemoteConnected() to FImGuiModuleProperties
Removed : Intrusive code in FProxyContext
Fixed: NoUnityBuild

* Added support of Unreal Sockets instead of using Windows code. Every platform supported by Unreal should now work with remote connection.

* Changed context index mapping to fix issues with multi-PIE debugging in 4.25.

* Status update with information about NetImgui.

* Bugfix: making sure sockets are all closed before exiting netImgui shutdown

* Fixed Linux crash caused by wrong mapping of key codes.

* Update DearImgui 1.80, NetImgui 1.3

Updating DearImgui and NetImgui libraries to latest version.

* Missing ImGui context release

* Prevent crash while cooking

* Case sensitive include fix

Co-authored-by: Sebastian <[email protected]>
# Conflicts:
#	CHANGES.md
#	ImGui.uplugin
#	README.md
#	Source/ImGui/ImGui.Build.cs
#	Source/ImGui/Private/ImGuiContextManager.cpp
#	Source/ImGui/Private/ImGuiContextProxy.cpp
#	Source/ImGui/Private/ImGuiDrawData.cpp
#	Source/ThirdParty/ImGuiLibrary/Include/imconfig.h
#	Source/ThirdParty/ImGuiLibrary/Include/imgui.h
#	Source/ThirdParty/ImGuiLibrary/LICENSE.txt
#	Source/ThirdParty/ImGuiLibrary/Private/imgui.cpp
#	Source/ThirdParty/ImGuiLibrary/Private/imgui_demo.cpp
#	Source/ThirdParty/ImGuiLibrary/Private/imgui_draw.cpp
#	Source/ThirdParty/ImGuiLibrary/Private/imgui_internal.h
#	Source/ThirdParty/ImGuiLibrary/Private/imgui_tables.cpp
#	Source/ThirdParty/ImGuiLibrary/Private/imgui_widgets.cpp
#	Source/ThirdParty/ImGuiLibrary/Private/imstb_truetype.h
@jonpas
Copy link
Member

jonpas commented Jun 4, 2023

Awesome!

Should NetImgui be disabled by default on master branch and be changed to opt-in you reckon?

@sammyfreg
Copy link
Author

sammyfreg commented Jun 4, 2023

Personnally, I would leave it active by default. When not in use, it only use a sleeping thread to listen for connection on a tcp socket. If people are really bothered by that, a simple value change disable all of the NetImgui integration, which can be made clear in the documention and the main header file / build.cs of the plugin.

Aditionally, this branch gives acces to extra built-in fonts .

@jonpas jonpas changed the base branch from net_imgui to master July 11, 2023 13:14
@jonpas jonpas added the enhancement New feature or request label Jul 11, 2023
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't like this, including binaries in the repository. This should be a separate download, ideally from NetImgui itself?

Copy link
Author

@sammyfreg sammyfreg Jul 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The server app needs to match the client code included. Since the .exe is small, I always included it pre-compiled together to keep simple for users. I could store it on a github wiki, like I do for the github webpage's images. I guess it will also need to have an updated webpage documentation to include the link.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the server application and updated the documentation.

README.md Outdated

To be able to connect to the Unreal editor or application that use NetImgui, you need to run a server (netImguiServer). Please, see the [NetImgui](https://github.com/sammyfreg/netImgui) page for instructions how to get it.

After launching the server for the first time, you need to add two client configurations, for ports 8889 and 8890. After that, you can either initialise connection from the server or set it to autoconnection mode. More info will be added later.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still true if netImgui.cfg is provided?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right this is no longuer needed with provided config (the 2 basics config). Still need to add console config. It'll update this info.

@jonpas
Copy link
Member

jonpas commented Jul 11, 2023

I have some changes locally that I can't seem to push to your branch (likely because it's a fork of the original repo, not this fork). net_imgui branch on this repo has those changes though.

@sammyfreg
Copy link
Author

Unrelated to my change, I saw this comment in the documentation

... you'll need to add the following line to your [GameName].Build.cs file otherwise you'll get linking errors:

// Tell the compiler we want to import the ImPlot symbols when linking against ImGui plugin 
PrivateDefinitions.Add(string.Format("IMPLOT_API=DLLIMPORT"));

This can actually be avoided, by adding the public define in the plugin .cs file or even the imgui config .h

@jonpas
Copy link
Member

jonpas commented Jul 19, 2023

Looks like just IMPLOT_API=DLLEXPORT is enough for Unreal to figure it out as our internal project using this never defines IMPLOT_API=DLLIMPORT.

@jonpas
Copy link
Member

jonpas commented Jul 19, 2023

Is your overall plan to deprecate https://github.com/sammyfreg/UnrealNetImgui? I haven't dived too deep into the integration yet, but I am wondering if it would be better to keep NetImgui integration into Unreal separate? That would also mean less waiting for this plugin to update to latest ImGui changes when there are changes required in NetImgui. Maybe keeping compatibility between both without direct access is not that easy though.

Overall I think the decision here is to either have this plugin as a collection of ImGui plugins integrated into Unreal Engine, or a hub/core plugin that allows sub-plugins to be added. Ultimately first is probably a lot easier, and doing that with ability to enable/disable features is ultimately the better choice. I only worry about version compatibilities in this case.

As for the server executable, I would recommend attaching a build of it to each NetImgui release. That would be useful for whatever NetImgui is/gets integrated into, wiki page is a bit hidden.

@sammyfreg
Copy link
Author

Is your overall plan to deprecate https://github.com/sammyfreg/UnrealNetImgui?

No, I am still working on it. At first, it was just intended as a simple plugin for people wanting to use a simple Dear ImGui integration without local display. A lot of game company used their own version of Dear ImGui integration in Unreal, so it is also a good example on how to import NetImgui into their own plugin. Since UnrealImGui had been inactive for a while, I started work some time ago on supporting local display too, controlled from the remote server. But this is slower going since I am pretty busy with other things.

I haven't dived too deep into the integration yet, but I am wondering if it would be better to keep NetImgui integration into Unreal separate?

It cannot be separate, NetImgui needs to be compiled alognside the Dear ImGui Integration. There's the issue of replacing the ImGui::Begin()/Render() with NetImgui::Begin()/End() and knowing on which context to remotely work with.

... wiki page is a bit hidden.

As for this point, I didn't mean that the user hunt the link on another wiki. I just store the zip file there for download. In my lastest pull request, I added a link in the README.md to the zip file with the needed content.

I would recommend attaching a build of it to each NetImgui release

Each NetImgui release already contain the exe compiled (link), but it also contains the client .cpps for integration into your application, and missing the Unreal Remote configs, could be confusing for Plugin users.

@jonpas
Copy link
Member

jonpas commented Jul 19, 2023

Unreal Remote config can be in this repo, that's fine. Only binaries should not - either from a verified release or self-built.

@sammyfreg
Copy link
Author

Ok. For now I removed the NetImguiServer folder entirely, so user can download it using the link on the README.md page, and copy the application to the folder of their choice, with the client list config file included.

@jonpas jonpas mentioned this pull request Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants