Jump to content
The Dark Mod Forums

Connection to TDM with automation


Recommended Posts

  • Replies 140
  • Created
  • Last Reply

Top Posters In This Topic

Top Posters In This Topic

Popular Posts

New video:  

This is how it looks right now. Also attached the code of main class if anyone is interested.   One nuisance is that reloading a map works only after the map is saved. So when mapper moves

I have an idea of adding TDM connection management to DarkRadiant. The plan is to combine it with TDM hot reload features later. Here is a brief list of features: start/connect to TDM, sto

Posted Images

I tested it and it looks pretty good.

  • Live camera synchronisation from DarkRadiant to the game works. The game position moves in realtime as I move the DR camera around.
  • Synchronisation in the other direction works if I choose the Sync camera back now option, but not in realtime. I don't know if this is expected.
  • Hot reload works. I can change the colour of a light and see the change immediately in game if I choose Enable map update mode.
  • Something weird happened to the camera in game at one point — I could no longer rotate in place, because horizontal mouse motions suddenly turned into left/right strafing. However I can't reproduce this reliably. Maybe I just pressed something in game without realising it.

Overall this is very exciting functionality and I'm happy to merge it even in this early state. Being able to click through light shaders in the light inspector and see the lighting change immediately in another window (with full soft shadows) is amazing, and really helps to avoid the limitations of the DarkRadiant lighting renderer.

Link to post
Share on other sites
10 hours ago, OrbWeaver said:

Synchronisation in the other direction works if I choose the Sync camera back now option, but not in realtime. I don't know if this is expected.

Yes, this is how it works.
It is a bit scary to have continuous synchronization in both directions. If there is demand from mappers, it can be added in future.

Quote

Something weird happened to the camera in game at one point — I could no longer rotate in place, because horizontal mouse motions suddenly turned into left/right strafing. However I can't reproduce this reliably. Maybe I just pressed something in game without realising it.

I see the same problem in Ubuntu VM when I press Alt. Hitting it again should unblock the movements.
It is really annoying since Alt+Tab is a common combination.

 

Link to post
Share on other sites
2 hours ago, stgatilov said:

I see the same problem in Ubuntu VM when I press Alt. Hitting it again should unblock the movements.
It is really annoying since Alt+Tab is a common combination.

Right, that would make sense. Normally what I do is bring down the console in order to switch to another application, because otherwise the game window continues to capture the mouse and I cannot click anywhere outside the game (which is not ideal in this situation, because now the console is obscuring half the window and I can't see the live updates). I must have tried Alt+Tab without bringing down the console and that is what entered the strafe mode.

Link to post
Share on other sites

I've moved the game connection code into its own plugin, which is now built when --enable-darkmod-plugins is set (on Linux). This was more work than I originally anticipated since I had to move quite a few classes from the radiant/map tree into libs/scene instead, so they could be used both by the plugin and the main Radiant module. I also had to expose certain methods on pure virtual interfaces which were previously only defined in implementing subclasses, which was relatively straightforward.

Hopefully I've done all the necessary legwork at the code level, so that when @greebo eventually merges this it will be a simple matter of updating the Windows build scripts to match the new code layout.

Link to post
Share on other sites

I still haven't got around to look into that, been always busy with either work or my ongoing DR work. I try to keep focused on one thing at a time, but I'll do it eventually.

Talking about moving Map code to scenelib, I'll have to check whether these changes will conflict with the UI/logic separation efforts in my master branch, but I guess everything can be resolved in a good way.

Link to post
Share on other sites
1 hour ago, greebo said:

I still haven't got around to look into that, been always busy with either work or my ongoing DR work. I try to keep focused on one thing at a time, but I'll do it eventually.

Talking about moving Map code to scenelib, I'll have to check whether these changes will conflict with the UI/logic separation efforts in my master branch, but I guess everything can be resolved in a good way.

No problem — I'm not trying to pressure you into merging it right away, just getting the code into a reasonable state so that when you do get time to merge it there shouldn't be too many obstacles.

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

I merged everything into my master branch now. It was quite the ride, since the architecture has changed quite a bit in the meantime. I re-played some of the commits to orbweaver/master, but the huge merge commit I was unable to integrate, it was just not matching up. I also had loads of conflicts, mostly because of the architecture changes, but also due to tons of whitespace adjustments. (If you guys want to make my life easier for the next merge, please don't change the whitespace of an entire file, it makes it really hard for me to spot the relevant code lines in a git conflict.)

The plugin is compiling in Windows now, but I haven't even tested it. I have to download the correct mod version first, and I'm out of time for today. I'll also do some refactoring in the plugin itself.

Link to post
Share on other sites
14 hours ago, greebo said:

(If you guys want to make my life easier for the next merge, please don't change the whitespace of an entire file, it makes it really hard for me to spot the relevant code lines in a git conflict.)

Sorry, that was probably my fault. Sometimes when I encounter misalignment due to mixed tabs/spaces on successive lines, I use Vim's :retab command but forget to select just the affected method body, which results in Vim converting tabs to spaces in the entire file.

Link to post
Share on other sites

No worries there. I still appreciate the fact that DR has more than one active developer these days, so thanks for doing the merge work and making it a plugin.

By the way, I didn't check the compilation in my Linux VM yet, if you could check out my master branch some time to see if it compiles, that'd be great.

And on a related note, I can confirm that the game connection plugin is working with the 2.09 dev build. 👍

Link to post
Share on other sites
5 hours ago, greebo said:

By the way, I didn't check the compilation in my Linux VM yet, if you could check out my master branch some time to see if it compiles, that'd be great.

No problem. The attached patch fixes all Linux compilation issues that I encountered.

I wasn't sure what to do about this line in PassiveSocket.cpp:

    SOCKET         socket = UINT_PTR(CSimpleSocket::SocketError);

UINT_PTR isn't defined anywhere, so I assume it must be a Windows-specific type, but CSocketError is just an enum not a pointer, so I'm not sure of the intent here. Does it work on Windows because SOCKET and UINT_PTR are both typedefs of the same type (e.g. unsigned int*)? The original clsocket code looks very odd: I'm not sure why the author used the SOCKET variable to store an error code which is a completely different type of data.

In any case, I had to remove the UINT_PTR because it does not compile on Linux, and I replaced it with C++11 construction:

    SOCKET         socket{CSimpleSocket::SocketError};

But I don't know if this is just going to reintroduce whatever problem you were fixing by adding the UINT_PTR cast.

Also, be aware that you accidentally (I presume) added a compiled radiant/darkradiant binary in this commit:

commit 0270caa70ede357888a4ec23fc43520424c54e0f
Author: codereader <greebo@angua.at>
Date:   Sun May 3 15:22:40 2020 +0200

    #5231: Compilation in gcc working, this will likely fail when at runtime pretty fast.

 create mode 100755 radiant/darkradiant

 

0001-Linux-compilation-fixes.patch

Link to post
Share on other sites

Oh my, I definitely didn't want to check in the darkradiant binary. I probably screwed up using git stage. edit: I just checked, luckily it's not a binary, it's a libtool-generated file, phew. It would have been several MB, bloating the repo needlessly. Still, that file should be removed.

And thanks for the patch, I'll apply it. The UINT_PTR type-cast was to fix a conversion warning, yes. I didn't check beforehand and failed to see it wasn't a portable type, must've confused it with something else. Maybe I'll check it out another time, but I can live with that warning just fine, so I'm happy to apply the fix that makes it compile again.

Link to post
Share on other sites
21 minutes ago, greebo said:

Had to change that initialisation another time, since it didn't compile in Windows. I replaced it with a static_cast<SOCKET>, now it's compiling without warnings.

That doesn't surprise me all that much — I have a feeling that brace initialisation is actually more strict than normal construction and won't allow certain implicit conversions of parameter types.

Using static_cast seems like the ideal solution here in any case, since casting one type to another is indeed what the code is doing.

Link to post
Share on other sites
  • 2 months later...

Added wiki page about it: https://wiki.thedarkmod.com/index.php?title=Hot_Reload

 

On 8/20/2020 at 11:57 PM, stgatilov said:

Profiler shows that MapExporter::prepareScene takes half of the second(the other half is taken by its destructor).
It is clear that the called functions run over the whole scene regardless of the "traverse" filter.

By the way, was this performance issue fixed?
As far as I understand, now there are saving/loading tests, so it should be possible to do.

 

Another question is about GUI.
As I probably told, the way all these stuff is packed into menu was due to laziness, not because it is intended 🙄

The main problem is that this feature needs constant visual presence on the screen. Because there are many modes which can be enabled/disabled, and the user should always be able to see easily what is on/off. Either a persistent window or a toolbar should work. And this is a big problem, because screen space is not endless and it already pretty much filled with stuff, plus I'm not sure there is any architecture for persistent windows.

Anyway, I have sketched the GUI I was thinking about.
The idea was to use LEDs (which I hope can be found for wxWidgets) to indicate various states of the modes, i.e. small circles of some color (red/green/yellow), which are dark when they are off and bright when they are on. LEDs should also be clickable to switch between states.

 

hotreload_dr_gui.png

Link to post
Share on other sites
58 minutes ago, stgatilov said:
On 8/20/2020 at 6:57 PM, stgatilov said:

Profiler shows that MapExporter::prepareScene takes half of the second(the other half is taken by its destructor).
It is clear that the called functions run over the whole scene regardless of the "traverse" filter.

By the way, was this performance issue fixed?

No. I can't just remove that code, it is the workaround that has the func_static brushes written properly to the map format, since the engine map loader expects them to be saved relative to its parent entity. I recall placing that hack more than a decade ago, since DR at that time wasn't able to deal with these entity-grouped brushes, they were not rotating correctly. I wasn't able to fix it back then, so I put in the workaround to move the brushes around before and after map saving such that they are saved as the engine expects them. I agree that it's a hack, but fixing it properly isn't exactly trivial.

1 hour ago, stgatilov said:

As far as I understand, now there are saving/loading tests, so it should be possible to do.

There are a few saving and loading tests, but there is no coverage of saving and loading func_static brushes yet.

I can't comment the GUI part, since I don't know what half of these options are doing. But it's certainly possible to pack it into a floating tool window like the AAS Area Viewer or the Surface/Light/Patch Inspectors.

Link to post
Share on other sites

As a user advocate, I'd say this looks needlessly complex (both wiki entry, DR Connection menu, etc.). Target audience doesn't need to know how it works under the hood, they need to know what to click to do what.

obraz.png.8a2cb5e1a1c07ec2941236f2f7b7f4bd.png

^ This is really bad, UI-wise. Way too many options and confusing naming. And first off, enable/disable should be a toggle, checkbox, picklist in a floating menu, anything but this.

IMO Hot reload is the most groundbreaking feature here, and this should be the MVP. Ideally, mapper would click one button, like an icon somewhere around the 3d viewport icons in DR, and the map would start in a new window, with hot reload already enabled and always active. Mapper shouldn't have to think about what's synced or updated or not, it should work out of the box. Reload map on save in DR is a nice-to-have addition, but it's not really a main focus. It should be like one tick box in the Preferences menu.

Edited by peter_spy
Link to post
Share on other sites
1 hour ago, peter_spy said:

As a user advocate, I'd say this looks needlessly complex (both wiki entry, DR Connection menu, etc.). Target audience doesn't need to know how it works under the hood, they need to know what to click to do what.

Yes, the wiki page is quite complicated. An additional mapper-friendly tutorial would be welcome, but I can't write the one: I'm 1) not a mapper and 2) a person who created this thing and thus knows too much about it.

Quote

Mapper shouldn't have to think about what's synced or updated or not, it should work out of the box.

Sounds like you have not used it much yet 😄
It does not "just work" all the time. And when it does not work as you expect (note that different people can expect different things in some cases), knowing the technical details will help you a lot. But unless you have problems, yes, you can surely skip all that garbage, just look at the how-to-enable sequence and click on buttons by intuition.

Quote

^ This is really bad, UI-wise. Way too many options and confusing naming. And first off, enable/disable should be a toggle, checkbox, picklist in a floating menu, anything but this.

Yes, the current GUI is bad and should be changed for sure.

Quote

Ideally, mapper would click one button, like an icon somewhere around the 3d viewport icons in DR, and the map would start in a new window, with hot reload already enabled and always active.

Maybe it would converge to something like this over time. Right now it is not clear for me whether one workflow is enough for all cases. Also note that some buttons are not about hot reload, but about other features. I'm not sure everyone would want to tie everything into one button.

For now DR does not start/close TDM process. Doing so may lead to more confusion, i.e. the game being closed by DR when player thinks it should not, or changing FMs without permission, or starting it without some extra parameter that user's desktop icon passes to TDM, starting wrong version/installation, etc.

Quote

Reload map on save in DR is a nice-to-have addition, but it's not really a main focus. It should be like one tick box in the Preferences menu.

The reloadMap thing is easier to understand, less prone to errors, and can be used for troubleshooting.

UPDATE: I just came to me: Sometimes what is user's dream is programmer's nightmare.

  • Like 1
Link to post
Share on other sites
1 hour ago, greebo said:

No. I can't just remove that code, it is the workaround that has the func_static brushes written properly to the map format, since the engine map loader expects them to be saved relative to its parent entity.

There is no need to remove it. All that I need is that it respects the traversal function.

Right now when hot reload update is done (Map::saveMapDiff), the map is saved with a traverser, which filters only a few entities that the user has actually changed. The worldspawn should never be among these entities.
However, the code we are talking about does some serious work for all the brushes, without any respect to traversal function. That's probably because this workaround is applied in constructor, where traverser is not yet available.

Quote

But it's certainly possible to pack it into a floating tool window like the AAS Area Viewer or the Surface/Light/Patch Inspectors.

Oh... sorry 🥺
Somehow I got impression that all these dialogs are modal and user cannot do anything while they are open.
Yes, it can be implemented this way then.

Link to post
Share on other sites
2 hours ago, stgatilov said:

Maybe it would converge to something like this over time. Right now it is not clear for me whether one workflow is enough for all cases. Also note that some buttons are not about hot reload, but about other features. I'm not sure everyone would want to tie everything into one button.

For now DR does not start/close TDM process. Doing so may lead to more confusion, i.e. the game being closed by DR when player thinks it should not, or changing FMs without permission, or starting it without some extra parameter that user's desktop icon passes to TDM, starting wrong version/installation, etc.

True, but that's also overcomplicating things at the very start. DR already knows your engine path and current FM folder/map path too. Launching TDM game window with that map will be enough. Maybe with some text overlay, like "Dark Radiant TDM Preview" to avoid confusion with TDM game launched manually. Then you can build on that in later revisions.

2 hours ago, stgatilov said:

UPDATE: I just came to me: Sometimes what is user's dream is programmer's nightmare.

This is obviously a simplification, but, in essence, this is why programmers alone shouldn't design features. Usually there has to be some kind of a "functional lead", who will be a user advocate and someone who can establish use cases abstracted from technology layer. Otherwise you risk putting a lot of work into something that ultimately will be mostly avoided by end users.

Edited by peter_spy
Link to post
Share on other sites

As a UI programmer I'd say that stgatlov's LED idea looks nice, but I'd rather not go down the route of implementing our own custom GUIs in the OpenGL windows.

Just rationalising the menu items into appropriate toggles (rather than separate Enable X and Disable X menu items) would be a big improvement, then perhaps pulling out a couple of major options into toolbar buttons so mappers could quickly enable or disable the sync with a single click.

  • Like 1
Link to post
Share on other sites
6 minutes ago, OrbWeaver said:

but I'd rather not go down the route of implementing our own custom GUIs in the OpenGL windows.

Never suggested anything like that 😄

LEDs are missing from core wxWidgets, and without them GUI would look much worse (like... three similar radio buttons?). The unofficial code for LEDs should not use OpenGL at all, I hope. Why would it be needed?

Quote

then perhaps pulling out a couple of major options into toolbar buttons so mappers could quickly enable or disable the sync with a single click.

I think toolbar space is precious.

It should be possible to show and hide the game connection GUI. Separate window is probably enough. It is also more flexible in terms of placement, since toolbars are either horizontal or vertical.

Hotkeys are a much better usability helper for experienced users than better-placed icon.

Link to post
Share on other sites
19 minutes ago, stgatilov said:

Never suggested anything like that 😄

LEDs are missing from core wxWidgets, and without them GUI would look much worse (like... three similar radio buttons?). The unofficial code for LEDs should not use OpenGL at all, I hope. Why would it be needed?

My mistake, I thought your idea was for some kind of floating LED widget in the ortho views. Still, even custom-drawn GUIs using wxWidgets would be an additional maintenance burden, so I wouldn't recommend this unless it was really important. A regular dialog using toggle buttons with appropriate icons would be fine, I think.

It's also important to remember that some users may be colour blind, so GUIs should never use only colour to distinguish important information. It's fine to use colour as an additional highlight provided there is some other change of shape/icon/text which doesn't rely on colour.

19 minutes ago, stgatilov said:

I think toolbar space is precious.

Indeed it is, but we already make quite poor use of toolbar space and I think there is scope for rationalising or reducing some of the existing icons. For example, I doubt we really need a permanently visible "Hollow" button when we have "Make Room" which is almost always better, while "Cubic clip the camera view" should be on the 3D view toolbar (since it is closely related to the existing "Far Clip In/Out" buttons and does not affect anything outside the 3D view).

19 minutes ago, stgatilov said:

It should be possible to show and hide the game connection GUI. Separate window is probably enough. It is also more flexible in terms of placement, since toolbars are either horizontal or vertical.

I'd be fine with an additional GUI for configuring connection options or tracking more detailed state, but I don't think it should be the only way to access the basic connection functionality. Not everyone likes persistent floating windows (even GIMP eventually gave in to user pressure and switched to a single window by default), and having to hide or show the dialog is an additional barrier between the user and this feature.

19 minutes ago, stgatilov said:

Hotkeys are a much better usability helper for experienced users than better-placed icon.

Yes they are, but they are also much less discoverable for new users, and they do not reveal current state ("Is the connection enabled or not?") which is important if a user is trying to figure out whether their game should be updating or not.

Link to post
Share on other sites

Ok, so maybe let's see how a functional user story for this could look like.

As a mapper, I'd like to launch a Game Preview from DarkRadiant, so I can see changes I make in a map, in real time. That's it. Now for a possible functional solution.

UI:

- I'd like to have an icon near the 3d preview pane in DR, so I can launch the preview window.

- The Launch Preview command should also be accessible in the Menu bar (rename Connection to something more meaningful, e.g. Game Preview), and it should also be configurable as a hotkey in DR.

- Clicking the icon or otherwise using the command should launch a TDM window with a map currently opened in DR. Otherwise throw proper errors with messages. Possible cases: editor with no map opened; started work but didn't save it, etc.

Usage:

- After being launched, the window should be resizable, movable and its position should be stored. Needs to work with multiple monitor setups, so people can resize it to full screen too.

- By default, the camera in Game preview should be in the same place as the DR camera, and moving it in DR should result in movement in Game preview window.

- All changes made in DR should be updated in Game preview window by default. Think render preview panes in other engines. Possible problems: game time vs. AI movement, etc.

- By default, any changes in Game preview window should not be updated in DR map.

- By default. Saving the map in DR should not trigger map reloading in Game preview window.

- I should be able to either close the Game preview window itself, or toggle the DR icon (use the hotkey again?).

 

And that's the functional base, everything else goes after this in subsequent iterations. It doesn't even assume that the connection works in both directions, it could be a window that you can't focus on or move around in-game, and it still would meet the initial requirements ;)

Link to post
Share on other sites
24 minutes ago, peter_spy said:

As a mapper, I'd like to launch a Game Preview from DarkRadiant, so I can see changes I make in a map, in real time. That's it. Now for a possible functional solution.

Your description basically says:

  1. DR should be able to launch TDM process and own it
  2. DR should be able to start current FM/map.
  3. Camera sync always ON.
  4. Map update always in its highest state (aka "always update after changes").

As for points 3 and 4, everyone would quickly learn these two buttons (hopefully, hotkeys for them in future) and be able to do it themselves. With the GUI I suggested it is two clicks on green LEDs.

As for point 1, it is a mess implementation-wise.
It needs coding a lot of OS-specific API, so it is some tricky code written individually for Windows and Linux. If one of the guys crash (DR/TDM), the other one will survive --- and it's very hard to avoid it. Also I bet if TDM begins loading fresh game, then it won't respond to any DR commands, so if you decide to close map or close DR at this moment, it would be pretty hard to kill TDM after it finishes loading. By the way, if one TDM has automation enabled, then the second one won't be able to use it --- one more reason for the user to care.

As for point 2, I can do it. There is already an automation command for selecting FM.

I can even run dmap, although... mappers would prefer to avoid dmapping when not necessary, and it is hard to determine reliably when it is really necessary, so it would be just one more button, instead of something that is run automatically every time. And mappers would still prefer doing it manually, because this way they can see dmap output in real time, instead of having to wait till the end.

Quote

- I should be able to either close the Game preview window itself, or toggle the DR icon (use the hotkey again?).

As an author of something like Painter's Wife, I would definitely be against closing TDM every time. Because as long as TDM is running, it has all the assets loaded. An author can decide to disable the hot reload feature but keep TDM alive, and in future just use Restart Game in main menu, probably preceded by dmapping it. If TDM is closed, then he will have to wait until all the textures and models get loaded again.

Link to post
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.


×
×
  • Create New...